[LibOS] Misc tiny bug fixes in IPC subsystem

LibOS/shim/include/shim_ipc.h

@@ -454,7 +454,7 @@ struct shim_ipc_info* create_ipc_info_cur_process(bool is_self_ipc_info);
 int get_ipc_info_cur_process(struct shim_ipc_info** pinfo);
 
 enum {
-    LISTEN=0,   /* listening */
+    LISTEN,     /* listening */
     SERVER,     /* connect as a server */
     KEEPALIVE,  /* keep the connetion alive */
     DIRCLD,     /* direct child */
@@ -487,7 +487,7 @@ struct shim_ipc_info* create_ipc_info(IDTYPE vmid, const char* uri, size_t len);
 void get_ipc_info(struct shim_ipc_info* port);
 void put_ipc_info(struct shim_ipc_info* port);
 
-struct shim_ipc_info* create_ipc_info_in_list(IDTYPE vmid, const char* uri);
+struct shim_ipc_info* create_ipc_info_in_list(IDTYPE vmid, const char* uri, size_t len);
 void put_ipc_info_in_list(struct shim_ipc_info* info);
 struct shim_ipc_info* lookup_ipc_info(IDTYPE vmid);
 
@@ -497,6 +497,7 @@ static_always_inline size_t get_ipc_msg_size(size_t payload) {
 }
 
 static_always_inline size_t get_ipc_msg_duplex_size(size_t payload) {
+    assert(sizeof(struct shim_ipc_msg_duplex) >= sizeof(struct shim_ipc_msg));
     return get_ipc_msg_size(payload) +
         (sizeof(struct shim_ipc_msg_duplex) - sizeof(struct shim_ipc_msg));
 }

LibOS/shim/src/bookkeep/shim_thread.c

@@ -658,8 +658,6 @@ BEGIN_RS_FUNC(thread)
     struct shim_thread * thread = (void *) (base + GET_CP_FUNC_ENTRY());
     __UNUSED(offset);
 
-    thread->vmid = cur_process.vmid;
-
     CP_REBASE(thread->children);
     CP_REBASE(thread->siblings);
     CP_REBASE(thread->exited_children);

LibOS/shim/src/ipc/shim_ipc.c

@@ -17,7 +17,7 @@
 /*
  * shim_ipc.c
  *
- * This file contains codes to maintain generic bookkeeping of IPC: operations
+ * This file contains code to maintain generic bookkeeping of IPC: operations
  * on shim_ipc_msg (one-way IPC messages), shim_ipc_msg_duplex (IPC messages
  * with acknowledgement), shim_ipc_info (IPC ports of process), shim_process.
  */
@@ -45,8 +45,8 @@ struct shim_lock ipc_info_lock;
 
 struct shim_process cur_process;
 
-#define CLIENT_HASH_LEN     6
-#define CLIENT_HASH_NUM     (1 << CLIENT_HASH_LEN)
+#define CLIENT_HASH_BITLEN  6
+#define CLIENT_HASH_NUM     (1 << CLIENT_HASH_BITLEN)
 #define CLIENT_HASH_MASK    (CLIENT_HASH_NUM - 1)
 #define CLIENT_HASH(vmid)   ((vmid) & CLIENT_HASH_MASK)
 DEFINE_LISTP(shim_ipc_info);
@@ -145,15 +145,13 @@ struct shim_ipc_info* create_ipc_info(IDTYPE vmid, const char* uri, size_t len)
     return info;
 }
 
-struct shim_ipc_info* create_ipc_info_in_list(IDTYPE vmid, const char* uri) {
+struct shim_ipc_info* create_ipc_info_in_list(IDTYPE vmid, const char* uri, size_t len) {
     assert(vmid);
 
     struct shim_ipc_info* info;
-    size_t len = strlen(uri);
-
     lock(&ipc_info_lock);
 
-    /* check if info with this vmid and uri already exists and return it */
+    /* check if info with this vmid & uri already exists and return it */
     LISTP_TYPE(shim_ipc_info)* info_bucket = &info_hlist[CLIENT_HASH(vmid)];
     LISTP_FOR_EACH_ENTRY(info, info_bucket, hlist)
         if (info->vmid == vmid && !qstrcmpstr(&info->uri, uri, len)) {
@@ -356,7 +354,7 @@ int send_ipc_message_duplex(struct shim_ipc_msg_duplex* msg, struct shim_ipc_por
     struct shim_thread * thread = get_cur_thread();
     assert(thread);
 
-    /* prepare thread which sends the message for waiting for response
+    /* prepare thread which will send the message for waiting for response
      * (this also acquires reference to the thread) */
     if (!msg->thread)
         thread_setwait(&msg->thread, thread);
@@ -377,9 +375,9 @@ int send_ipc_message_duplex(struct shim_ipc_msg_duplex* msg, struct shim_ipc_por
     if (seq)
         *seq = msg->msg.seq;
 
-    debug("Start waiting for response (seq = %lu)\n", msg->msg.seq);
+    debug("Waiting for response (seq = %lu)\n", msg->msg.seq);
 
-    /* force thread which sends the message to wait for response;
+    /* force thread which will send the message to wait for response;
      * ignore unrelated interrupts but fail on actual errors */
     do {
         ret = thread_sleep(NO_TIMEOUT);
@@ -457,14 +455,14 @@ int ipc_checkpoint_send(const char* cpdir, IDTYPE cpsession) {
     struct shim_ipc_msg* msg = __alloca(total_msg_size);
     init_ipc_msg(msg, IPC_CHECKPOINT, total_msg_size, 0);
 
-    struct shim_ipc_checkpoint* msgin = (struct shim_ipc_checkpoint *) &msg->msg;
+    struct shim_ipc_checkpoint* msgin = (struct shim_ipc_checkpoint *)&msg->msg;
     msgin->cpsession = cpsession;
     memcpy(&msgin->cpdir, cpdir, len + 1);
 
     debug("IPC broadcast to all: IPC_CHECKPOINT(%u, %s)\n", cpsession, cpdir);
 
     /* broadcast to all including myself (so I can also checkpoint) */
-    ret = broadcast_ipc(msg, IPC_PORT_DIRCLD|IPC_PORT_DIRPRT, /*exclude_port*/ NULL);
+    ret = broadcast_ipc(msg, IPC_PORT_DIRCLD|IPC_PORT_DIRPRT, /*exclude_port=*/NULL);
     SAVE_PROFILE_INTERVAL(ipc_checkpoint_send);
     return ret;
 }
@@ -476,7 +474,7 @@ int ipc_checkpoint_send(const char* cpdir, IDTYPE cpsession) {
 int ipc_checkpoint_callback(struct shim_ipc_msg* msg, struct shim_ipc_port* port) {
     BEGIN_PROFILE_INTERVAL();
     int ret = 0;
-    struct shim_ipc_checkpoint* msgin = (struct shim_ipc_checkpoint *) msg->msg;
+    struct shim_ipc_checkpoint* msgin = (struct shim_ipc_checkpoint *)msg->msg;
 
     debug("IPC callback from %u: IPC_CHECKPOINT(%u, %s)\n",
           msg->src, msgin->cpsession, msgin->cpdir);
@@ -495,7 +493,7 @@ out:
 BEGIN_CP_FUNC(ipc_info) {
     assert(size == sizeof(struct shim_ipc_info));
 
-    struct shim_ipc_info* info = (struct shim_ipc_info *) obj;
+    struct shim_ipc_info* info = (struct shim_ipc_info *)obj;
     struct shim_ipc_info* new_info = NULL;
 
     ptr_t off = GET_FROM_CP_MAP(obj);
@@ -504,7 +502,7 @@ BEGIN_CP_FUNC(ipc_info) {
         off = ADD_CP_OFFSET(sizeof(struct shim_ipc_info));
         ADD_TO_CP_MAP(obj, off);
 
-        new_info = (struct shim_ipc_info *) (base + off);
+        new_info = (struct shim_ipc_info *)(base + off);
         memcpy(new_info, info, sizeof(struct shim_ipc_info));
         REF_SET(new_info->ref_count, 0);
 
@@ -523,18 +521,18 @@ BEGIN_CP_FUNC(ipc_info) {
         }
     } else {
         /* already checkpointed */
-        new_info = (struct shim_ipc_info *) (base + off);
+        new_info = (struct shim_ipc_info *)(base + off);
     }
 
     if (new_info && objp)
-        *objp = (void *) new_info;
+        *objp = (void *)new_info;
 }
 END_CP_FUNC_NO_RS(ipc_info)
 
 BEGIN_CP_FUNC(process) {
     assert(size == sizeof(struct shim_process));
 
-    struct shim_process* process = (struct shim_process *) obj;
+    struct shim_process* process = (struct shim_process *)obj;
     struct shim_process* new_process = NULL;
 
     ptr_t off = GET_FROM_CP_MAP(obj);
@@ -543,7 +541,7 @@ BEGIN_CP_FUNC(process) {
         off = ADD_CP_OFFSET(sizeof(struct shim_process));
         ADD_TO_CP_MAP(obj, off);
 
-        new_process = (struct shim_process *) (base + off);
+        new_process = (struct shim_process *)(base + off);
         memcpy(new_process, process, sizeof(struct shim_process));
 
         /* call ipc_info-specific checkpointing functions
@@ -559,7 +557,7 @@ BEGIN_CP_FUNC(process) {
         ADD_CP_FUNC_ENTRY(off);
     } else {
         /* already checkpointed */
-        new_process = (struct shim_process *) (base + off);
+        new_process = (struct shim_process *)(base + off);
     }
 
     if (objp)
@@ -569,7 +567,7 @@ END_CP_FUNC(process)
 
 BEGIN_RS_FUNC(process) {
     __UNUSED(offset);
-    struct shim_process* process = (void *) (base + GET_CP_FUNC_ENTRY());
+    struct shim_process* process = (void *)(base + GET_CP_FUNC_ENTRY());
 
     /* process vmid  = 0: fork/clone case, forces to pick up new host-OS vmid
      * process vmid != 0: execve case, forces to re-use vmid of parent */

LibOS/shim/src/ipc/shim_ipc_child.c

@@ -121,8 +121,8 @@ void ipc_port_with_child_fini(struct shim_ipc_port* port, IDTYPE vmid, unsigned
     if ((ret = walk_simple_thread_list(&child_sthread_exit, &info)) > 0)
         exited_threads_cnt += ret;
 
-    debug("Child process %u got disconnected: assume that child exited and "
-          "force %d of its threads to exit\n", vmid & 0xFFFF, exited_threads_cnt);
+    debug("Child process %u got disconnected: assuming that child exited and "
+          "forcing %d of its threads to exit\n", vmid & 0xFFFF, exited_threads_cnt);
 }
 
 DEFINE_PROFILE_INTERVAL(ipc_cld_exit_turnaround, ipc);
@@ -153,12 +153,12 @@ int ipc_cld_exit_send(IDTYPE ppid, IDTYPE tid, unsigned int exitcode, unsigned i
     debug("IPC broadcast: IPC_CLD_EXIT(%u, %u, %d, %u)\n",
           ppid, tid, exitcode, term_signal);
 
-    int ret = broadcast_ipc(msg, IPC_PORT_DIRPRT|IPC_PORT_DIRCLD, /*exclude_port*/ NULL);
+    int ret = broadcast_ipc(msg, IPC_PORT_DIRPRT|IPC_PORT_DIRCLD, /*exclude_port=*/NULL);
     SAVE_PROFILE_INTERVAL(ipc_cld_exit_send);
     return ret;
 }
 
-/* IPC helper thread invokes this callback on an IPC_CLD_EXIT mesage received
+/* IPC helper thread invokes this callback on an IPC_CLD_EXIT message received
  * from a specific thread msgin->tid of the exiting child process with vmid
  * msg->src. The thread of the exiting child process informs about its exit
  * code in msgin->exit_code and its terminating signal in msgin->term_signal.
@@ -189,7 +189,7 @@ int ipc_cld_exit_callback(struct shim_ipc_msg* msg, struct shim_ipc_port* port)
     /* message cannot come from our own threads (from ourselves as process) */
     assert(msg->src != cur_process.vmid);
 
-    /* First try to find remote thread who sent this message among normal
+    /* First try to find remote thread which sent this message among normal
      * threads. In the common case, we (as parent process) keep remote child
      * threads in the thread list. But sometimes the message can arrive twice
      * or very late, such that the corresponding remote thread was already
@@ -206,7 +206,7 @@ int ipc_cld_exit_callback(struct shim_ipc_msg* msg, struct shim_ipc_port* port)
 
         /* Remote thread is "virtually" exited: SIGCHLD is generated for the
          * parent thread and exit events are arranged for subsequent wait4(). */
-        ret = thread_exit(thread, /*send_ipc*/ false);
+        ret = thread_exit(thread, /*send_ipc=*/false);
         put_thread(thread);
     } else {
         /* Uncommon case: remote child thread was already exited and deleted
@@ -244,7 +244,7 @@ DEFINE_PROFILE_INTERVAL(ipc_send_profile, ipc);
 #ifdef PROFILE
 int ipc_cld_profile_send(void) {
     struct shim_ipc_port* port = NULL;
-    IDTYPE dest = (IDTYPE) -1;
+    IDTYPE dest = (IDTYPE)-1;
 
     /* port and dest are initialized to parent process */
     lock(&cur_process.lock);
@@ -254,7 +254,7 @@ int ipc_cld_profile_send(void) {
     }
     unlock(&cur_process.lock);
 
-    if (!port || (dest == (IDTYPE) -1))
+    if (!port || (dest == (IDTYPE)-1))
         return -ESRCH;
 
     unsigned long time = GET_PROFILE_INTERVAL();
@@ -278,7 +278,7 @@ int ipc_cld_profile_send(void) {
     struct shim_ipc_msg* msg = __alloca(total_msg_size);
     init_ipc_msg(msg, IPC_CLD_PROFILE, total_msg_size, dest);
 
-    struct shim_ipc_cld_profile* msgin = (struct shim_ipc_cld_profile *) &msg->msg;
+    struct shim_ipc_cld_profile* msgin = (struct shim_ipc_cld_profile *)&msg->msg;
 
     size_t nsent = 0;
     for (size_t i = 0; i < N_PROFILE && nsent < nsending; i++)
@@ -323,7 +323,7 @@ int ipc_cld_profile_send(void) {
 int ipc_cld_profile_callback(struct shim_ipc_msg* msg, struct shim_ipc_port* port) {
     debug("IPC callback from %u: IPC_CLD_PROFILE\n", msg->src & 0xFFFF);
 
-    struct shim_ipc_cld_profile* msgin = (struct shim_ipc_cld_profile *) &msg->msg;
+    struct shim_ipc_cld_profile* msgin = (struct shim_ipc_cld_profile *)&msg->msg;
 
     for (int i = 0; i < msgin->nprofile; i++) {
         int idx = msgin->profile[i].idx;

LibOS/shim/src/ipc/shim_ipc_helper.c

@@ -44,8 +44,7 @@ static MEM_MGR port_mgr;
 DEFINE_LISTP(shim_ipc_port);
 static LISTP_TYPE(shim_ipc_port) port_list;
 
-/* can be read without ipc_helper_lock but always written with lock held */
-static enum {  HELPER_NOTALIVE, HELPER_ALIVE } ipc_helper_state;
+static enum { HELPER_NOTALIVE, HELPER_ALIVE } ipc_helper_state;
 
 static struct shim_thread* ipc_helper_thread;
 static struct shim_lock    ipc_helper_lock;
@@ -95,7 +94,7 @@ static int init_self_ipc_port(void) {
 
     if (!cur_process.self) {
         /* very first process or clone/fork case: create IPC port from scratch */
-        cur_process.self = create_ipc_info_cur_process(/*is_self_ipc_info*/ true);
+        cur_process.self = create_ipc_info_cur_process(/*is_self_ipc_info=*/true);
         if (!cur_process.self) {
             unlock(&cur_process.lock);
             return -EACCES;
@@ -107,7 +106,7 @@ static int init_self_ipc_port(void) {
         add_ipc_port_by_id(cur_process.self->vmid,
                            cur_process.self->pal_handle,
                            IPC_PORT_SERVER,
-                           /* fini callback */ NULL,
+                           /*fini=*/NULL,
                            &cur_process.self->port);
     }
 
@@ -135,7 +134,7 @@ static int init_parent_ipc_port(void) {
     add_ipc_port_by_id(cur_process.parent->vmid,
                        cur_process.parent->pal_handle,
                        IPC_PORT_DIRPRT | IPC_PORT_LISTEN,
-                       /* fini callback */ NULL,
+                       /*fini=*/NULL,
                        &cur_process.parent->port);
 
     unlock(&cur_process.lock);
@@ -170,7 +169,7 @@ static int init_ns_ipc_port(int ns_idx) {
     add_ipc_port_by_id(cur_process.ns[ns_idx]->vmid,
                        cur_process.ns[ns_idx]->pal_handle,
                        type | IPC_PORT_LISTEN,
-                       /* fini callback */ NULL,
+                       /*fini=*/NULL,
                        &cur_process.ns[ns_idx]->port);
 
     unlock(&cur_process.lock);
@@ -271,11 +270,14 @@ static void __add_ipc_port(struct shim_ipc_port* port, IDTYPE vmid,
 
     /* find empty slot in fini callbacks and register callback */
     if (fini) {
+        bool found_empty_slot = false;
         for (int i = 0; i < MAX_IPC_PORT_FINI_CB; i++)
             if (!port->fini[i] || port->fini[i] == fini) {
                 port->fini[i] = fini;
+                found_empty_slot = true;
                 break;
             }
+        assert(found_empty_slot);
     }
 
     /* add to port list if not there already */
@@ -305,7 +307,7 @@ static void __del_ipc_port(struct shim_ipc_port* port) {
         msg->retval = -ECONNRESET;
         if (msg->thread) {
             debug("Deleted pending message on port %p, wake up blocking thread %d\n",
-                    port, msg->thread->tid);
+                  port, msg->thread->tid);
             thread_wakeup(msg->thread);
         }
     }
@@ -384,8 +386,10 @@ void del_ipc_port_fini(struct shim_ipc_port* port, unsigned int exitcode) {
     unlock(&ipc_helper_lock);
 
     for (int i = 0; i < MAX_IPC_PORT_FINI_CB; i++)
-        if (port->fini[i])
+        if (port->fini[i]) {
             (port->fini[i])(port, port->vmid, exitcode);
+            port->fini[i] = NULL;
+        }
 
     __put_ipc_port(port);
 }
@@ -410,7 +414,7 @@ struct shim_ipc_port* lookup_ipc_port(IDTYPE vmid, IDTYPE type) {
     struct shim_ipc_port* tmp;
     LISTP_FOR_EACH_ENTRY(tmp, &port_list, list) {
         if (tmp->vmid == vmid && (tmp->type & type)) {
-            debug("found port %p (handle %p) for process %u (type %04x)\n",
+            debug("Found port %p (handle %p) for process %u (type %04x)\n",
                   tmp, tmp->pal_handle, tmp->vmid & 0xFFFF, tmp->type);
             port = tmp;
             __get_ipc_port(port);
@@ -422,34 +426,36 @@ struct shim_ipc_port* lookup_ipc_port(IDTYPE vmid, IDTYPE type) {
     return port;
 }
 
+#define PORTS_ON_STACK_CNT 32
+
 int broadcast_ipc(struct shim_ipc_msg* msg, int target_type, struct shim_ipc_port* exclude_port) {
     int ret;
     struct shim_ipc_port* port;
     struct shim_ipc_port** target_ports;
-    int target_ports_cnt = 0;
+    size_t target_ports_cnt = 0;
 
     assert(target_type);
     lock(&ipc_helper_lock);
 
     /* Collect all ports with appropriate types. In common case, stack-allocated
-     * array of 32 ports is enough. If there are more ports, we will allocate
-     * a bigger array on the heap and collect all ports again. */
-    struct shim_ipc_port* target_ports_stack[32];
+     * array of PORTS_ON_STACK_CNT ports is enough. If there are more ports, we
+     * will allocate a bigger array on the heap and collect all ports again. */
+    struct shim_ipc_port* target_ports_stack[PORTS_ON_STACK_CNT];
     LISTP_FOR_EACH_ENTRY(port, &port_list, list) {
         if (port == exclude_port)
             continue;
         if (port->type & target_type) {
-            if (target_ports_cnt < 32)
+            if (target_ports_cnt < PORTS_ON_STACK_CNT)
                 target_ports_stack[target_ports_cnt] = port;
             target_ports_cnt++;
         }
     }
     target_ports = target_ports_stack;
 
-    if (target_ports_cnt > 32) {
-        /* Rare case when there are more than 32 ports. Allocate big-enough
-         * array on the heap and collect all ports again. */
-        int cnt = 0;
+    if (target_ports_cnt > PORTS_ON_STACK_CNT) {
+        /* Rare case when there are more than PORTS_ON_STACK_CNT ports. Allocate
+         * big-enough array on the heap and collect all ports again. */
+        size_t cnt = 0;
         struct shim_ipc_port** target_ports_heap =
             malloc(sizeof(struct shim_ipc_port *) * target_ports_cnt);
 
@@ -463,13 +469,13 @@ int broadcast_ipc(struct shim_ipc_msg* msg, int target_type, struct shim_ipc_por
         assert(cnt == target_ports_cnt);
     }
 
-    unlock(&ipc_helper_lock);
+    for (size_t i = 0; i < target_ports_cnt; i++)
+        __get_ipc_port(target_ports[i]);
 
-    for (int i = 0; i < target_ports_cnt; i++)
-        get_ipc_port(target_ports[i]);
+    unlock(&ipc_helper_lock);
 
     /* send msg to each collected port (note that ports cannot be freed in meantime) */
-    for (int i = 0; i < target_ports_cnt; i++) {
+    for (size_t i = 0; i < target_ports_cnt; i++) {
         port = target_ports[i];
 
         debug("Broadcast to port %p (handle %p) for process %u (type %x, target %x)\n",
@@ -479,14 +485,14 @@ int broadcast_ipc(struct shim_ipc_msg* msg, int target_type, struct shim_ipc_por
         ret = send_ipc_message(msg, port);
         if (ret < 0) {
             debug("Broadcast to port %p (handle %p) for process %u failed (errno = %d)!\n",
-                    port, port->pal_handle, port->vmid & 0xFFFF, ret);
+                  port, port->pal_handle, port->vmid & 0xFFFF, ret);
             goto out;
         }
     }
 
     ret = 0;
 out:
-    for (int i = 0; i < target_ports_cnt; i++)
+    for (size_t i = 0; i < target_ports_cnt; i++)
         put_ipc_port(target_ports[i]);
     if (target_ports != target_ports_stack)
         free(target_ports);
@@ -503,7 +509,7 @@ static int ipc_resp_callback(struct shim_ipc_msg* msg, struct shim_ipc_port* por
     /* find a corresponding request msg for this response msg */
     struct shim_ipc_msg_duplex* req_msg = pop_ipc_msg_duplex(port, msg->seq);
 
-    /* if some thread waits for response, wake it up with response retval */
+    /* if some thread is waiting for response, wake it with response retval */
     if (req_msg) {
         req_msg->retval = resp->retval;
         if (req_msg->thread)
@@ -514,7 +520,7 @@ static int ipc_resp_callback(struct shim_ipc_msg* msg, struct shim_ipc_port* por
     return resp->retval;
 }
 
-int send_response_ipc_message(struct shim_ipc_port* port, IDTYPE dest, int ret, uint64_t seq) {
+int send_response_ipc_message(struct shim_ipc_port* port, IDTYPE dest, int ret, unsigned long seq) {
     ret = (ret == RESPONSE_CALLBACK) ? 0 : ret;
 
     /* create IPC_RESP msg to send to dest, with sequence number seq, and in-body retval ret */
@@ -523,7 +529,7 @@ int send_response_ipc_message(struct shim_ipc_port* port, IDTYPE dest, int ret,
     init_ipc_msg(resp_msg, IPC_RESP, total_msg_size, dest);
     resp_msg->seq = seq;
 
-    struct shim_ipc_resp* resp = (struct shim_ipc_resp *) resp_msg->msg;
+    struct shim_ipc_resp* resp = (struct shim_ipc_resp *)resp_msg->msg;
     resp->retval = ret;
 
     debug("IPC send to %u: IPC_RESP(%d)\n", resp_msg->dst & 0xFFFF, ret);
@@ -532,12 +538,12 @@ int send_response_ipc_message(struct shim_ipc_port* port, IDTYPE dest, int ret,
 
 static int receive_ipc_message(struct shim_ipc_port* port) {
     int ret;
-    int readahead = IPC_MSG_MINIMAL_SIZE * 2;
-    int bufsize = IPC_MSG_MINIMAL_SIZE + readahead;
+    size_t readahead = IPC_MSG_MINIMAL_SIZE * 2;
+    size_t bufsize = IPC_MSG_MINIMAL_SIZE + readahead;
 
     struct shim_ipc_msg* msg = malloc(bufsize);
-    int expected_size = IPC_MSG_MINIMAL_SIZE;
-    int bytes = 0;
+    size_t expected_size = IPC_MSG_MINIMAL_SIZE;
+    size_t bytes = 0;
 
     do {
         while (bytes < expected_size) {
@@ -551,8 +557,8 @@ static int receive_ipc_message(struct shim_ipc_port* port) {
                 msg = tmp_buf;
             }
 
-            int read = DkStreamRead(port->pal_handle, /*offset*/ 0, expected_size - bytes + readahead,
-                                     (void *) msg + bytes, NULL, 0);
+            size_t read = DkStreamRead(port->pal_handle, /*offset=*/0, expected_size - bytes + readahead,
+                                       (void *) msg + bytes, NULL, 0);
 
             if (!read) {
                 if (PAL_ERRNO == EINTR || PAL_ERRNO == EAGAIN || PAL_ERRNO == EWOULDBLOCK)
@@ -578,13 +584,13 @@ static int receive_ipc_message(struct shim_ipc_port* port) {
         if (msg->src != cur_process.vmid) {
             if (msg->code < IPC_CODE_NUM && ipc_callbacks[msg->code]) {
                 /* invoke callback to this msg */
-                ret = (*ipc_callbacks[msg->code]) (msg, port);
+                ret = (*ipc_callbacks[msg->code])(msg, port);
                 if ((ret < 0 || ret == RESPONSE_CALLBACK) && msg->seq) {
                     /* send IPC_RESP message to sender of this msg */
                     ret = send_response_ipc_message(port, msg->src, ret, msg->seq);
                     if (ret < 0) {
                         debug("Sending IPC_RESP msg on port %p (handle %p) to %u failed\n",
-                                port, port->pal_handle, msg->src & 0xFFFF);
+                              port, port->pal_handle, msg->src & 0xFFFF);
                         ret = -PAL_ERRNO;
                         goto out;
                     }
@@ -597,7 +603,7 @@ static int receive_ipc_message(struct shim_ipc_port* port) {
         if (bytes > 0) {
             /* we may have started reading the next message, move this message
              * to beginning of msg buffer and reset expected size */
-            memmove(msg, (void *) msg + expected_size, bytes);
+            memmove(msg, (void *)msg + expected_size, bytes);
             expected_size = IPC_MSG_MINIMAL_SIZE;
             if (bytes >= IPC_MSG_MINIMAL_SIZE)
                 expected_size = msg->size;
@@ -642,7 +648,7 @@ noreturn static void shim_ipc_helper(void* dummy) {
     /* Initialize two lists:
      * - object_list collects IPC port objects and is the main handled list
      * - palhandle_list collects corresponding PAL handles of IPC port objects
-     *   and is needed for DkObjectsWaitAny(.., <arrya-of-PAL-handles>, ..)
+     *   and is needed for DkObjectsWaitAny(.., <array-of-PAL-handles>, ..)
      *   interface; palhandle_list always contains at least install_new_event
      *
      * We allocate these two lists on the heap so they do not overflow the
@@ -658,7 +664,15 @@ noreturn static void shim_ipc_helper(void* dummy) {
     PAL_HANDLE install_new_event_hdl = event_handle(&install_new_event);
     palhandle_list[0] = install_new_event_hdl;
 
-    while (ipc_helper_state == HELPER_ALIVE) {
+    while (true) {
+        lock(&ipc_helper_lock);
+        if (ipc_helper_state != HELPER_ALIVE) {
+            ipc_helper_thread = NULL;
+            unlock(&ipc_helper_lock);
+            break;
+        }
+        unlock(&ipc_helper_lock);
+
         struct shim_ipc_port* polled_port = NULL;
 
         if (polled == install_new_event_hdl) {
@@ -702,22 +716,20 @@ noreturn static void shim_ipc_helper(void* dummy) {
 
                     if (attr.disconnected) {
                         debug("Port %p (handle %p) disconnected\n",
-                                polled_port, polled_port->pal_handle);
+                              polled_port, polled_port->pal_handle);
                         del_ipc_port_fini(polled_port, -ECONNRESET);
                     }
                 } else {
                     debug("Port %p (handle %p) was removed during attr querying\n",
-                            polled_port, polled_port->pal_handle);
+                          polled_port, polled_port->pal_handle);
                     del_ipc_port_fini(polled_port, -PAL_ERRNO);
                 }
             }
         }
 
         /* done handling ports; put their references so they can be freed */
-        for (size_t i = 0; i < object_list_size; i++) {
-            struct shim_ipc_port* port = object_list[i];
-            put_ipc_port(port);
-        }
+        for (size_t i = 0; i < object_list_size; i++)
+            put_ipc_port(object_list[i]);
 
         lock(&ipc_helper_lock);
 
@@ -729,14 +741,14 @@ noreturn static void shim_ipc_helper(void* dummy) {
             /* get port reference so it is not freed while we wait on/handle it */
             __get_ipc_port(port);
 
-            /* grow object_list and palhandle_list to accomodate more objects */
             if (object_list_size == object_list_maxsize) {
+                /* grow object_list and palhandle_list to accomodate more objects */
                 struct shim_ipc_port** tmp_array = malloc(
                         sizeof(struct shim_ipc_port *) * (object_list_maxsize * 2));
                 PAL_HANDLE* tmp_pal_array = malloc(
                         sizeof(PAL_HANDLE) * (1 + object_list_maxsize * 2));
                 memcpy(tmp_array, object_list,
-                        sizeof(struct shim_ipc_port *) * (object_list_maxsize));
+                        sizeof(struct shim_ipc_port *) * (object_list_size));
                 memcpy(tmp_pal_array, palhandle_list,
                         sizeof(PAL_HANDLE) * (1 + object_list_size));
                 object_list_maxsize *= 2;
@@ -759,23 +771,15 @@ noreturn static void shim_ipc_helper(void* dummy) {
 
         /* wait on collected ports' PAL handles + install_new_event_hdl */
         polled = DkObjectsWaitAny(object_list_size + 1, palhandle_list, NO_TIMEOUT);
-        /* ensure that while loop breaks on ipc_helper_state change */
-        COMPILER_BARRIER();
     }
 
     /* IPC thread exits; put acquired port references so they can be freed */
-    for (size_t i = 0; i < object_list_size; i++) {
-        struct shim_ipc_port* port = object_list[i];
-        put_ipc_port(port);
-    }
+    for (size_t i = 0; i < object_list_size; i++)
+        put_ipc_port(object_list[i]);
 
     free(object_list);
     free(palhandle_list);
 
-    lock(&ipc_helper_lock);
-    ipc_helper_state = HELPER_NOTALIVE;
-    ipc_helper_thread = NULL;
-    unlock(&ipc_helper_lock);
     put_thread(self);
     debug("IPC helper thread terminated\n");
 
@@ -799,6 +803,7 @@ static void shim_ipc_helper_prepare(void* arg) {
     void* stack = allocate_stack(IPC_HELPER_STACK_SIZE, allocsize, false);
 
     if (notme || !stack) {
+        free(stack);
         put_thread(self);
         DkThreadExit();
         return;
@@ -827,10 +832,11 @@ static int create_ipc_helper(void) {
     PAL_HANDLE handle = thread_create(shim_ipc_helper_prepare, new);
 
     if (!handle) {
+        int ret = -PAL_ERRNO;  /* put_thread() may overwrite errno */
         ipc_helper_thread = NULL;
         ipc_helper_state = HELPER_NOTALIVE;
         put_thread(new);
-        return -PAL_ERRNO;
+        return ret;
     }
 
     new->pal_handle = handle;

LibOS/shim/src/ipc/shim_ipc_nsimpl.h

@@ -266,7 +266,7 @@ static int __add_range (struct range * r, IDTYPE off, IDTYPE owner,
     r->subranges = NULL;
 
     if (owner) {
-        r->owner = create_ipc_info_in_list(owner, uri);
+        r->owner = create_ipc_info_in_list(owner, uri, strlen(uri));
         if (!r->owner)
             return -ENOMEM;
     }
@@ -367,7 +367,7 @@ int CONCAT3(add, NS, subrange) (IDTYPE idx, IDTYPE owner,
     assert(owner);
     lock(&range_map_lock);
 
-    s->owner = create_ipc_info_in_list(owner, uri);
+    s->owner = create_ipc_info_in_list(owner, uri, strlen(uri));
     if (!s->owner) {
         err = -ENOMEM;
         goto failed;

LibOS/shim/src/ipc/shim_ipc_pid.c

@@ -94,7 +94,7 @@ int ipc_pid_kill_send(IDTYPE sender, IDTYPE target, enum kill_type type, int sig
     struct shim_ipc_msg* msg = __alloca(total_msg_size);
     init_ipc_msg(msg, IPC_PID_KILL, total_msg_size, dest);
 
-    struct shim_ipc_pid_kill* msgin = (struct shim_ipc_pid_kill *) &msg->msg;
+    struct shim_ipc_pid_kill* msgin = (struct shim_ipc_pid_kill *)&msg->msg;
     msgin->sender = sender;
     msgin->type   = type;
     msgin->id     = target;
@@ -103,9 +103,8 @@ int ipc_pid_kill_send(IDTYPE sender, IDTYPE target, enum kill_type type, int sig
     if (type == KILL_ALL) {
         debug("IPC broadcast: IPC_PID_KILL(%u, %d, %u, %d)\n",
               sender, type, target, signum);
-        ret = broadcast_ipc(msg, IPC_PORT_DIRCLD|IPC_PORT_DIRPRT, /*exclude_port*/ NULL);
-    }
-    else {
+        ret = broadcast_ipc(msg, IPC_PORT_DIRCLD|IPC_PORT_DIRPRT, /*exclude_port=*/NULL);
+    } else {
         debug("IPC send to %u: IPC_PID_KILL(%u, %d, %u, %d)\n",
               dest & 0xFFFF, sender, type, target, signum);
         ret = send_ipc_message(msg, port);

LibOS/shim/src/shim_async.c

@@ -130,6 +130,10 @@ int init_async(void) {
     async_helper_state = HELPER_NOTALIVE;
     create_lock(&async_helper_lock);
     create_event(&install_new_event);
+
+    /* enable locking mechanisms since we are going in multi-threaded mode */
+    enable_locking();
+
     return 0;
 }
 

LibOS/shim/src/shim_checkpoint.c

@@ -1131,7 +1131,7 @@ int do_migrate_process (int (*migrate) (struct shim_cp_store *,
          * current process, so notify the leader regarding subleasing of TID
          * (child must create self-pipe with convention of pipe:child-vmid) */
         char new_process_self_uri[256];
-        snprintf(new_process_self_uri, 256, "pipe:%u", res.child_vmid);
+        snprintf(new_process_self_uri, sizeof(new_process_self_uri), "pipe:%u", res.child_vmid);
         ipc_pid_sublease_send(res.child_vmid, thread->tid, new_process_self_uri, NULL);
 
         /* listen on the new IPC port to the new child process */

LibOS/shim/src/shim_init.c

@@ -844,9 +844,9 @@ static int name_pipe_rand (char * uri, size_t size, void * id)
     if (ret < 0)
         return -convert_pal_errno(-ret);
     debug("creating pipe: pipe.srv:%u\n", pipeid);
-    if ((len = snprintf(uri, size, "pipe.srv:%u", pipeid)) == size)
+    if ((len = snprintf(uri, size, "pipe.srv:%u", pipeid)) >= size)
         return -ERANGE;
-    *((IDTYPE *) id) = pipeid;
+    *((IDTYPE *)id) = pipeid;
     return len;
 }
 
@@ -855,9 +855,9 @@ static int name_pipe_vmid (char * uri, size_t size, void * id)
     IDTYPE pipeid = cur_process.vmid;
     size_t len;
     debug("creating pipe: pipe.srv:%u\n", pipeid);
-    if ((len = snprintf(uri, size, "pipe.srv:%u", pipeid)) == size)
+    if ((len = snprintf(uri, size, "pipe.srv:%u", pipeid)) >= size)
         return -ERANGE;
-    *((IDTYPE *) id) = pipeid;
+    *((IDTYPE *)id) = pipeid;
     return len;
 }
 

LibOS/shim/src/sys/shim_pipe.c

@@ -41,7 +41,7 @@ int create_pipes (IDTYPE * pipeid, PAL_HANDLE * srv, PAL_HANDLE * cli,
     char uri[PIPE_URI_SIZE];
 
     if ((ret = create_pipe(pipeid, uri, PIPE_URI_SIZE, &hdl0,
-                           qstr, /*use_vmid_for_name*/ false)) < 0) {
+                           qstr, /*use_vmid_for_name=*/false)) < 0) {
         debug("pipe creation failure\n");
         return ret;
     }

LibOS/shim/src/sys/shim_semget.c

@@ -574,7 +574,7 @@ send_result:
             init_ipc_msg(resp_msg, IPC_RESP, total_msg_size, sops->client.vmid);
             resp_msg->seq = sops->client.seq;
 
-            struct shim_ipc_resp* resp = (struct shim_ipc_resp *) resp_msg->msg;
+            struct shim_ipc_resp* resp = (struct shim_ipc_resp *)resp_msg->msg;
             resp->retval = sops->stat.completed ? 0 : -EAGAIN;
 
             send_ipc_message(resp_msg, sops->client.port);
@@ -797,7 +797,7 @@ unowned:
             init_ipc_msg(resp_msg, IPC_RESP, total_msg_size, client->vmid);
             resp_msg->seq = client->seq;
 
-            struct shim_ipc_resp* resp = (struct shim_ipc_resp *) resp_msg->msg;
+            struct shim_ipc_resp* resp = (struct shim_ipc_resp *)resp_msg->msg;
             resp->retval = ret;
 
             ret = send_ipc_message(resp_msg, client->port);

LibOS/shim/src/sys/shim_sigaction.c

@@ -535,7 +535,7 @@ int shim_do_kill (pid_t pid, int sig)
     /* If pid equals -1, then sig is sent to every process for which the
        calling process has permission to send */
     else if (pid == -1) {
-        ipc_pid_kill_send(cur->tid, /*target*/ 0, KILL_ALL, sig);
+        ipc_pid_kill_send(cur->tid, /*target=*/0, KILL_ALL, sig);
         kill_all_threads(cur, cur->tid, sig);
         send_to_self = true;
     }