From edf607ba7e155b182e6c578891877e3a7a472eb5 Mon Sep 17 00:00:00 2001 From: Ivan Maidanski Date: Sat, 26 Oct 2024 12:22:34 +0300 Subject: [PATCH] Fix missing usage of ADDR() and MAKE_CPTR() (fix of commits 42c770aa6, 176d640ec) 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 | 19 +++++++---- dyn_load.c | 70 +++++++++++++++++++++------------------ headers.c | 2 +- include/private/gc_priv.h | 2 +- os_dep.c | 14 ++++---- pthread_support.c | 4 +-- 6 files changed, 61 insertions(+), 50 deletions(-) diff --git a/darwin_stop_world.c b/darwin_stop_world.c index 83d72caa6..97a21b8c1 100644 --- a/darwin_stop_world.c +++ b/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 */ diff --git a/dyn_load.c b/dyn_load.c index 1e8697159..37379c4f7 100644 --- a/dyn_load.c +++ b/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", diff --git a/headers.c b/headers.c index a642f9daf..ea4ded80c 100644 --- a/headers.c +++ b/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; diff --git a/include/private/gc_priv.h b/include/private/gc_priv.h index 023f20853..e21f1af8f 100644 --- a/include/private/gc_priv.h +++ b/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) \ diff --git a/os_dep.c b/os_dep.c index ba3981617..53da4f88c 100644 --- a/os_dep.c +++ b/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,7 +5408,7 @@ 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 @@ -5416,7 +5416,7 @@ GC_save_callers(struct callinfo info[NFRAMES]) 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) diff --git a/pthread_support.c b/pthread_support.c index 10e2ece9a..4dd839bdb 100644 --- a/pthread_support.c +++ b/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