Fix missing usage of ADDR() and MAKE_CPTR()

(fix of commits 42c770a, 176d640)

Issue #627 (bdwgc).

* darwin_stop_world.c (StackFrame): Remove reserved, savedRTOC fields
(these are unused trailing ones).
* darwin_stop_world.c (GC_FindTopOfStack): Use MAKE_CPTR() for
stack_start and frame->savedSP; compare frame to NULL instead of
stack_start to zero.
* darwin_stop_world.c [CPPCHECK] (GC_FindTopOfStack): Call
GC_noop1(frame->savedCR) (to make a dummy use of the field).
* dyn_load.c (GC_FirstDLOpenedLinkMap): Use MAKE_CPTR() for
dp->d_un.d_ptr.
* dyn_load.c (GC_register_dynamic_libraries): Replace char* to ptr_t;
use ADDR() for e.
* dyn_load.c (GC_register_dynamic_libraries,
GC_register_dynlib_callback): Use MAKE_CPTR() for p->p_vaddr.
* dyn_load.c [AIX] (GC_register_dynamic_libraries): Replace while()
loop with a for(;;) one; cast ldi->ldinfo_dataorg to ptr_t instead of
use of MAKE_CPTR().
* dyn_load.c [DARWIN] (dyld_section_add_del): Change type of start,
end local variables from unsigned long to ptr_t; use MAKE_CPTR() for
slide+sec->addr.
* headers.c (GC_prev_block): Use MAKE_CPTR() for HBLK_ADDR(bi,j).
* include/private/gc_priv.h (HBLKDISPL): Use ADDR().
* os_dep.c [NEED_CALLINFO && SAVE_CALL_CHAIN && (NARGS!=0
|| NFRAMES%2!=0 || !GC_HAVE_BUILTIN_BACKTRACE) && (I386 || SPARC)]
(GC_save_callers): Cast frame->FR_SAVFP to ptr_t instead of long; use
MAKE_CPTR() for fp->fr_arg[i].
* os_dep.c [NEED_CALLINFO] (GC_print_callers): Use ADDR() for
info[i].ci_pc.
* pthread_support.c [GC_PTHREADS_PARAMARK && GC_ASSERTIONS
&& GC_WIN32_THREADS && !USE_PTHREAD_LOCKS] (NUMERIC_THREAD_ID): Use
ADDR() for GC_PTHREAD_PTRVAL(id); refine comment.

darwin_stop_world.c

@@ -53,16 +53,16 @@ typedef struct StackFrame {
   unsigned long savedSP;
   unsigned long savedCR;
   unsigned long savedLR;
-  unsigned long reserved[2];
-  unsigned long savedRTOC;
+  /* unsigned long reserved[2]; */
+  /* unsigned long savedRTOC; */
 } StackFrame;
 
 GC_INNER ptr_t
 GC_FindTopOfStack(unsigned long stack_start)
 {
-  StackFrame *frame = (StackFrame *)stack_start;
+  StackFrame *frame = (StackFrame *)MAKE_CPTR(stack_start);
 
-  if (stack_start == 0) {
+  if (NULL == frame) {
 #    ifdef POWERPC
 #      if CPP_WORDSZ == 32
     __asm__ __volatile__("lwz %0,0(r1)" : "=r"(frame));
@@ -71,12 +71,14 @@ GC_FindTopOfStack(unsigned long stack_start)
 #      endif
 #    elif defined(ARM32)
     volatile ptr_t sp_reg;
+
     __asm__ __volatile__("mov %0, r7\n" : "=r"(sp_reg));
-    frame = (StackFrame *)sp_reg;
+    frame = (/* no volatile */ StackFrame *)sp_reg;
 #    elif defined(AARCH64)
     volatile ptr_t sp_reg;
+
     __asm__ __volatile__("mov %0, x29\n" : "=r"(sp_reg));
-    frame = (StackFrame *)sp_reg;
+    frame = (/* no volatile */ StackFrame *)sp_reg;
 #    else
 #      if defined(CPPCHECK)
     GC_noop1_ptr(&frame);
@@ -91,7 +93,10 @@ GC_FindTopOfStack(unsigned long stack_start)
   while (frame->savedSP != 0) { /* stop if no more stack frames */
     unsigned long maskedLR;
 
-    frame = (StackFrame *)frame->savedSP;
+#    ifdef CPPCHECK
+    GC_noop1(frame->savedCR);
+#    endif
+    frame = (StackFrame *)MAKE_CPTR(frame->savedSP);
 
     /* We do these next two checks after going to the next frame        */
     /* because the LR for the first stack frame in the loop is not set  */

dyn_load.c

@@ -337,14 +337,16 @@ GC_FirstDLOpenedLinkMap(void)
     /* _DYNAMIC symbol not resolved. */
     return NULL;
   }
-  if (cachedResult == 0) {
+  if (NULL == cachedResult) {
     int tag;
 
     for (dp = (ElfW(Dyn) *)&_DYNAMIC; (tag = dp->d_tag) != 0; dp++) {
       if (tag == DT_DEBUG) {
-        const struct r_debug *rd = (struct r_debug *)dp->d_un.d_ptr;
+        const struct r_debug *rd = (struct r_debug *)MAKE_CPTR(dp->d_un.d_ptr);
+
         if (rd != NULL) {
           const struct link_map *lm = rd->r_map;
+
           if (lm != NULL)
             cachedResult = lm->l_next; /* might be NULL */
         }
@@ -361,22 +363,22 @@ GC_register_dynamic_libraries(void)
   struct link_map *lm;
 
   GC_ASSERT(I_HOLD_LOCK());
-  for (lm = GC_FirstDLOpenedLinkMap(); lm != 0; lm = lm->l_next) {
+  for (lm = GC_FirstDLOpenedLinkMap(); lm != NULL; lm = lm->l_next) {
     ElfW(Ehdr) * e;
     ElfW(Phdr) * p;
+    ptr_t start;
     unsigned long offset;
-    char *start;
     int i;
 
     e = (ElfW(Ehdr) *)lm->l_addr;
-    p = ((ElfW(Phdr) *)(((char *)(e)) + e->e_phoff));
-    offset = (unsigned long)lm->l_addr;
+    p = (ElfW(Phdr) *)((ptr_t)e + e->e_phoff);
+    offset = (unsigned long)ADDR(e);
     for (i = 0; i < (int)e->e_phnum; i++, p++) {
       switch (p->p_type) {
       case PT_LOAD:
-        if (!(p->p_flags & PF_W))
+        if ((p->p_flags & PF_W) == 0)
           break;
-        start = (char *)p->p_vaddr + offset;
+        start = MAKE_CPTR(p->p_vaddr) + offset;
         GC_add_roots_inner(start, start + p->p_memsz, TRUE);
         break;
       default:
@@ -630,7 +632,7 @@ GC_register_dynlib_callback(struct dl_phdr_info *info, size_t size, void *ptr)
       if ((p->p_flags & PF_W) == 0)
         continue;
 
-      start = (ptr_t)p->p_vaddr + info->dlpi_addr;
+      start = MAKE_CPTR(p->p_vaddr) + info->dlpi_addr;
       end = start + p->p_memsz;
       if (callback != 0 && !callback(info->dlpi_name, start, p->p_memsz))
         continue;
@@ -674,7 +676,7 @@ GC_register_dynlib_callback(struct dl_phdr_info *info, size_t size, void *ptr)
       /* encountered "LOAD" segment, so we need to exclude it.        */
       int j;
 
-      start = (ptr_t)p->p_vaddr + info->dlpi_addr;
+      start = MAKE_CPTR(p->p_vaddr) + info->dlpi_addr;
       end = start + p->p_memsz;
       for (j = n_load_segs; --j >= 0;) {
         if (ADDR_INSIDE(start, load_segs[j].start, load_segs[j].end)) {
@@ -874,10 +876,12 @@ GC_FirstDLOpenedLinkMap(void)
 
     for (dp = _DYNAMIC; (tag = dp->d_tag) != 0; dp++) {
       if (tag == DT_DEBUG) {
-        const struct r_debug *rd = (struct r_debug *)dp->d_un.d_ptr;
-        /* d_ptr could be null if libs are linked statically. */
+        const struct r_debug *rd = (struct r_debug *)MAKE_CPTR(dp->d_un.d_ptr);
+
+        /* d_ptr could be 0 if libs are linked statically. */
         if (rd != NULL) {
           const struct link_map *lm = rd->r_map;
+
           if (lm != NULL)
             cachedResult = lm->l_next; /* might be NULL */
         }
@@ -900,26 +904,26 @@ GC_register_dynamic_libraries(void)
     return;
   }
 #      endif
-  for (lm = GC_FirstDLOpenedLinkMap(); lm != 0; lm = lm->l_next) {
+  for (lm = GC_FirstDLOpenedLinkMap(); lm != NULL; lm = lm->l_next) {
     ElfW(Ehdr) * e;
     ElfW(Phdr) * p;
+    ptr_t start;
     unsigned long offset;
-    char *start;
     int i;
 
     e = (ElfW(Ehdr) *)lm->l_addr;
 #      ifdef HOST_ANDROID
-    if (e == NULL)
+    if (NULL == e)
       continue;
 #      endif
-    p = (ElfW(Phdr) *)((char *)e + e->e_phoff);
-    offset = (unsigned long)lm->l_addr;
+    p = (ElfW(Phdr) *)((ptr_t)e + e->e_phoff);
+    offset = (unsigned long)ADDR(e);
     for (i = 0; i < (int)e->e_phnum; i++, p++) {
       switch (p->p_type) {
       case PT_LOAD:
-        if (!(p->p_flags & PF_W))
+        if ((p->p_flags & PF_W) == 0)
           break;
-        start = (char *)p->p_vaddr + offset;
+        start = MAKE_CPTR(p->p_vaddr) + offset;
         GC_add_roots_inner(start, start + p->p_memsz, TRUE);
         break;
       default:
@@ -993,7 +997,7 @@ GC_register_dynamic_libraries(void)
     current_sz = needed_sz * 2 + 1;
     addr_map
         = (prmap_t *)GC_scratch_alloc((size_t)current_sz * sizeof(prmap_t));
-    if (addr_map == NULL)
+    if (NULL == addr_map)
       ABORT("Insufficient memory for address map");
   }
   if (ioctl(fd, PIOCMAP, addr_map) < 0) {
@@ -1255,13 +1259,14 @@ GC_register_dynamic_libraries(void)
     }
 
     ldi = (struct ld_info *)ldibuf;
-    while (ldi) {
+    for (;;) {
       len = ldi->ldinfo_next;
-      GC_add_roots_inner(ldi->ldinfo_dataorg,
-                         MAKE_CPTR((unsigned long)ldi->ldinfo_dataorg)
-                             + ldi->ldinfo_datasize,
+      GC_add_roots_inner((ptr_t)ldi->ldinfo_dataorg,
+                         (ptr_t)ldi->ldinfo_dataorg + ldi->ldinfo_datasize,
                          TRUE);
-      ldi = len ? (struct ld_info *)((char *)ldi + len) : 0;
+      if (0 == len)
+        break;
+      ldi = (struct ld_info *)((ptr_t)ldi + len);
     }
     break;
   }
@@ -1353,7 +1358,8 @@ dyld_section_add_del(const struct GC_MACH_HEADER *phdr, intptr_t slide,
                      const char *dlpi_name, GC_has_static_roots_func callback,
                      const char *seg, const char *secnam, GC_bool is_add)
 {
-  unsigned long start, end, sec_size;
+  ptr_t start, end;
+  unsigned long sec_size;
 #    ifdef USE_GETSECTBYNAME
 #      if CPP_WORDSZ == 64
   const struct section_64 *sec = getsectbynamefromheader_64(phdr, seg, secnam);
@@ -1364,13 +1370,13 @@ dyld_section_add_del(const struct GC_MACH_HEADER *phdr, intptr_t slide,
   if (NULL == sec)
     return;
   sec_size = sec->size;
-  start = slide + sec->addr;
+  start = MAKE_CPTR(slide + sec->addr);
 #    else
 
   UNUSED_ARG(slide);
   sec_size = 0;
-  start = (unsigned long)getsectiondata(phdr, seg, secnam, &sec_size);
-  if (0 == start)
+  start = (ptr_t)getsectiondata(phdr, seg, secnam, &sec_size);
+  if (NULL == start)
     return;
 #    endif
   if (sec_size < sizeof(ptr_t))
@@ -1380,14 +1386,14 @@ dyld_section_add_del(const struct GC_MACH_HEADER *phdr, intptr_t slide,
     LOCK();
     /* The user callback is invoked holding the allocator lock.   */
     if (EXPECT(callback != 0, FALSE)
-        && !callback(dlpi_name, (void *)start, (size_t)sec_size)) {
+        && !callback(dlpi_name, start, (size_t)sec_size)) {
       UNLOCK();
       return; /* skip section */
     }
-    GC_add_roots_inner((ptr_t)start, (ptr_t)end, FALSE);
+    GC_add_roots_inner(start, end, FALSE);
     UNLOCK();
   } else {
-    GC_remove_roots((void *)start, (void *)end);
+    GC_remove_roots(start, end);
   }
 #    ifdef DARWIN_DEBUG
   GC_log_printf("%s section __DATA,%s at %p-%p (%lu bytes) from image %s\n",

headers.c

@@ -430,7 +430,7 @@ GC_prev_block(struct hblk *h)
         j -= (signed_word)ADDR(hhdr);
       } else {
         /* TODO: return hhdr -> hb_block instead */
-        return (struct hblk *)HBLK_ADDR(bi, j);
+        return (struct hblk *)MAKE_CPTR(HBLK_ADDR(bi, j));
       }
     }
     j = BOTTOM_SZ - 1;

include/private/gc_priv.h

@@ -964,7 +964,7 @@ EXTERN_C_BEGIN
 #define modHBLKSZ(n) ((n) & (HBLKSIZE - 1))
 
 #define HBLKPTR(objptr) ((struct hblk *)PTR_ALIGN_DOWN(objptr, HBLKSIZE))
-#define HBLKDISPL(objptr) modHBLKSZ((size_t)(objptr))
+#define HBLKDISPL(objptr) modHBLKSZ((size_t)ADDR(objptr))
 
 /* Same as HBLKPTR() but points to the first block in the page.     */
 #define HBLK_PAGE_ALIGNED(objptr) \

os_dep.c

@@ -5398,7 +5398,7 @@ GC_save_callers(struct callinfo info[NFRAMES])
   fp = frame;
 #      else /* SPARC */
   frame = (struct frame *)GC_save_regs_in_stack();
-  fp = (struct frame *)((long)frame->FR_SAVFP + BIAS);
+  fp = (struct frame *)((ptr_t)frame->FR_SAVFP + BIAS);
 #      endif
 
   for (; !HOTTER_THAN((ptr_t)fp, (ptr_t)frame)
@@ -5408,15 +5408,15 @@ GC_save_callers(struct callinfo info[NFRAMES])
          && fp != NULL
 #      endif
          && nframes < NFRAMES;
-       fp = (struct frame *)((long)fp->FR_SAVFP + BIAS), nframes++) {
+       fp = (struct frame *)((ptr_t)fp->FR_SAVFP + BIAS), nframes++) {
 #      if NARGS > 0
     int i;
 #      endif
 
     info[nframes].ci_pc = (GC_return_addr_t)fp->FR_SAVPC;
 #      if NARGS > 0
     for (i = 0; i < NARGS; i++) {
-      info[nframes].ci_arg[i] = GC_HIDE_NZ_POINTER((void *)fp->fr_arg[i]);
+      info[nframes].ci_arg[i] = GC_HIDE_NZ_POINTER(MAKE_CPTR(fp->fr_arg[i]));
     }
 #      endif
   }
@@ -5480,7 +5480,7 @@ GC_print_callers(struct callinfo info[NFRAMES])
     if (reent_cnt > 0) {
       /* We were called either concurrently or during an allocation */
       /* by backtrace_symbols() called from GC_print_callers; punt. */
-      GC_err_printf("\t\t##PC##= 0x%lx\n", (unsigned long)info[i].ci_pc);
+      GC_err_printf("\t\t##PC##= 0x%lx\n", (unsigned long)ADDR(info[i].ci_pc));
       continue;
     }
 
@@ -5496,7 +5496,7 @@ GC_print_callers(struct callinfo info[NFRAMES])
 #  endif
       /* else */ {
         (void)snprintf(buf, sizeof(buf), "##PC##= 0x%lx",
-                       (unsigned long)info[i].ci_pc);
+                       (unsigned long)ADDR(info[i].ci_pc));
         buf[sizeof(buf) - 1] = '\0';
         name = buf;
       }
@@ -5536,7 +5536,7 @@ GC_print_callers(struct callinfo info[NFRAMES])
         /* isn't time critical.                                 */
         (void)snprintf(cmd_buf, sizeof(cmd_buf),
                        "/usr/bin/addr2line -f -e %s 0x%lx", exe_name,
-                       (unsigned long)info[i].ci_pc);
+                       (unsigned long)ADDR(info[i].ci_pc));
         cmd_buf[sizeof(cmd_buf) - 1] = '\0';
         old_preload = GETENV("LD_PRELOAD");
         if (old_preload != NULL) {
@@ -5588,7 +5588,7 @@ GC_print_callers(struct callinfo info[NFRAMES])
           /* Add address in the hex format.     */
           (void)snprintf(&result_buf[result_len],
                          sizeof(result_buf) - result_len, " [0x%lx]",
-                         (unsigned long)info[i].ci_pc);
+                         (unsigned long)ADDR(info[i].ci_pc));
           result_buf[sizeof(result_buf) - 1] = '\0';
         }
 #    if defined(CPPCHECK)

pthread_support.c

@@ -2979,9 +2979,9 @@ GC_lock(void)
 
 #    if defined(GC_ASSERTIONS) && defined(GC_WIN32_THREADS) \
         && !defined(USE_PTHREAD_LOCKS)
-/* Id is not guaranteed to be unique.       */
+/* Note: result is not guaranteed to be unique. */
 #      define NUMERIC_THREAD_ID(id) \
-        (unsigned long)((word)GC_PTHREAD_PTRVAL(id))
+        ((unsigned long)ADDR(GC_PTHREAD_PTRVAL(id)))
 #    endif
 
 #    ifdef GC_ASSERTIONS