| Message ID | CAMe9rOrAoXEE4f8k_6-e4thZMaHCowvirJNAowp666+rHuWwsg@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
H.J., Thanks for the patches. Please (re)send them to llvm-commits, otherwise I can not accept them. --kcc On Wed, May 14, 2014 at 2:31 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Tue, May 13, 2014 at 2:02 AM, Konstantin Serebryany > <konstantin.s.serebryany@gmail.com> wrote: >> New patch attached. >> It is based on r208674 which enables LeakSanitizer >> (https://code.google.com/p/address-sanitizer/wiki/LeakSanitizer) by >> default in asan mode. > > There are a couple issues for x32: > > 1. This change > > @@ -56,13 +55,6 @@ typedef signed long long sptr; // NOLINT > typedef unsigned long uptr; // NOLINT > typedef signed long sptr; // NOLINT > #endif // defined(_WIN64) > -#if defined(__x86_64__) > -// Since x32 uses ILP32 data model in 64-bit hardware mode, we must use > -// 64-bit pointer to unwind stack frame. > -typedef unsigned long long uhwptr; // NOLINT > -#else > -typedef uptr uhwptr; // NOLINT > -#endif > typedef unsigned char u8; > typedef unsigned short u16; // NOLINT > typedef unsigned int u32; > > @@ -120,46 +34,43 @@ uptr StackTrace::GetCurrentPc() { > void StackTrace::FastUnwindStack(uptr pc, uptr bp, > uptr stack_top, uptr stack_bottom, > uptr max_depth) { > - if (max_depth == 0) { > - size = 0; > - return; > - } > + CHECK_GE(max_depth, 2); > trace[0] = pc; > size = 1; > - uhwptr *frame = (uhwptr *)bp; > - uhwptr *prev_frame = frame - 1; > + uptr *frame = (uptr *)bp; > + uptr *prev_frame = frame - 1; > if (stack_top < 4096) return; // Sanity check for stack top. > // Avoid infinite loop when frame == frame[0] by using frame > prev_frame. > while (frame > prev_frame && > - frame < (uhwptr *)stack_top - 2 && > - frame > (uhwptr *)stack_bottom && > + frame < (uptr *)stack_top - 2 && > + frame > (uptr *)stack_bottom && > IsAligned((uptr)frame, sizeof(*frame)) && > size < max_depth) { > - uhwptr pc1 = frame[1]; > + uptr pc1 = frame[1]; > if (pc1 != pc) { > - trace[size++] = (uptr) pc1; > + trace[size++] = pc1; > } > prev_frame = frame; > - frame = (uhwptr *)frame[0]; > + frame = (uptr*)frame[0]; > } > } > > reverted: > > 2012-11-16 H.J. Lu <hongjiu.lu@intel.com> > > PR other/55333 > * include/sanitizer/common_interface_defs.h (uhwptr): New type > for hardware pointer. > * sanitizer_common/sanitizer_stacktrace.cc > (StackTrace::FastUnwindStack): > Replace uptr with uhwptr for stack unwind. > > 2. struct linux_dirent is incorrect for x32. We need something like > > struct linux_dirent { > #if defined(__x86_64__) && !defined(_LP64) > unsigned long long d_ino; > unsigned long long d_off; > #else > unsigned long d_ino; > unsigned long d_off; > #endif > unsigned short d_reclen; > char d_name[256]; > }; > > 3. Pointers passed to internal_syscall should be casted to (uptr). > Otherwise, they won't be properly extended to 64-bit. We need > to use (uptr) in internal_sigprocmask, like > > uptr internal_sigprocmask(int how, __sanitizer_sigset_t *set, > __sanitizer_sigset_t *oldset) { > #if SANITIZER_FREEBSD > return internal_syscall(SYSCALL(sigprocmask), how, set, oldset); > #else > __sanitizer_kernel_sigset_t *k_set = (__sanitizer_kernel_sigset_t *)set; > __sanitizer_kernel_sigset_t *k_oldset = (__sanitizer_kernel_sigset_t *)oldset; > return internal_syscall(SYSCALL(rt_sigprocmask), (uptr)how, > (uptr)&k_set->sig[0], > (uptr)&k_oldset->sig[0], sizeof(__sanitizer_kernel_sigset_t)); > #endif > } > > 4. ThreadDescriptorSize returns wrong value for x32. Size of struct > pthread should be 1728. > > I am enclosing patches for those issues. > > > -- > H.J.
On Thu, May 15, 2014 at 1:05 AM, Konstantin Serebryany <konstantin.s.serebryany@gmail.com> wrote: > H.J., > Thanks for the patches. Please (re)send them to llvm-commits, > otherwise I can not accept them. I think this is bogus reasoning. You should be able to take and post them yourself. Those patches. Thanks, Andrew Pinski > > --kcc > > On Wed, May 14, 2014 at 2:31 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Tue, May 13, 2014 at 2:02 AM, Konstantin Serebryany >> <konstantin.s.serebryany@gmail.com> wrote: >>> New patch attached. >>> It is based on r208674 which enables LeakSanitizer >>> (https://code.google.com/p/address-sanitizer/wiki/LeakSanitizer) by >>> default in asan mode. >> >> There are a couple issues for x32: >> >> 1. This change >> >> @@ -56,13 +55,6 @@ typedef signed long long sptr; // NOLINT >> typedef unsigned long uptr; // NOLINT >> typedef signed long sptr; // NOLINT >> #endif // defined(_WIN64) >> -#if defined(__x86_64__) >> -// Since x32 uses ILP32 data model in 64-bit hardware mode, we must use >> -// 64-bit pointer to unwind stack frame. >> -typedef unsigned long long uhwptr; // NOLINT >> -#else >> -typedef uptr uhwptr; // NOLINT >> -#endif >> typedef unsigned char u8; >> typedef unsigned short u16; // NOLINT >> typedef unsigned int u32; >> >> @@ -120,46 +34,43 @@ uptr StackTrace::GetCurrentPc() { >> void StackTrace::FastUnwindStack(uptr pc, uptr bp, >> uptr stack_top, uptr stack_bottom, >> uptr max_depth) { >> - if (max_depth == 0) { >> - size = 0; >> - return; >> - } >> + CHECK_GE(max_depth, 2); >> trace[0] = pc; >> size = 1; >> - uhwptr *frame = (uhwptr *)bp; >> - uhwptr *prev_frame = frame - 1; >> + uptr *frame = (uptr *)bp; >> + uptr *prev_frame = frame - 1; >> if (stack_top < 4096) return; // Sanity check for stack top. >> // Avoid infinite loop when frame == frame[0] by using frame > prev_frame. >> while (frame > prev_frame && >> - frame < (uhwptr *)stack_top - 2 && >> - frame > (uhwptr *)stack_bottom && >> + frame < (uptr *)stack_top - 2 && >> + frame > (uptr *)stack_bottom && >> IsAligned((uptr)frame, sizeof(*frame)) && >> size < max_depth) { >> - uhwptr pc1 = frame[1]; >> + uptr pc1 = frame[1]; >> if (pc1 != pc) { >> - trace[size++] = (uptr) pc1; >> + trace[size++] = pc1; >> } >> prev_frame = frame; >> - frame = (uhwptr *)frame[0]; >> + frame = (uptr*)frame[0]; >> } >> } >> >> reverted: >> >> 2012-11-16 H.J. Lu <hongjiu.lu@intel.com> >> >> PR other/55333 >> * include/sanitizer/common_interface_defs.h (uhwptr): New type >> for hardware pointer. >> * sanitizer_common/sanitizer_stacktrace.cc >> (StackTrace::FastUnwindStack): >> Replace uptr with uhwptr for stack unwind. >> >> 2. struct linux_dirent is incorrect for x32. We need something like >> >> struct linux_dirent { >> #if defined(__x86_64__) && !defined(_LP64) >> unsigned long long d_ino; >> unsigned long long d_off; >> #else >> unsigned long d_ino; >> unsigned long d_off; >> #endif >> unsigned short d_reclen; >> char d_name[256]; >> }; >> >> 3. Pointers passed to internal_syscall should be casted to (uptr). >> Otherwise, they won't be properly extended to 64-bit. We need >> to use (uptr) in internal_sigprocmask, like >> >> uptr internal_sigprocmask(int how, __sanitizer_sigset_t *set, >> __sanitizer_sigset_t *oldset) { >> #if SANITIZER_FREEBSD >> return internal_syscall(SYSCALL(sigprocmask), how, set, oldset); >> #else >> __sanitizer_kernel_sigset_t *k_set = (__sanitizer_kernel_sigset_t *)set; >> __sanitizer_kernel_sigset_t *k_oldset = (__sanitizer_kernel_sigset_t *)oldset; >> return internal_syscall(SYSCALL(rt_sigprocmask), (uptr)how, >> (uptr)&k_set->sig[0], >> (uptr)&k_oldset->sig[0], sizeof(__sanitizer_kernel_sigset_t)); >> #endif >> } >> >> 4. ThreadDescriptorSize returns wrong value for x32. Size of struct >> pthread should be 1728. >> >> I am enclosing patches for those issues. >> >> >> -- >> H.J.
On Thu, May 15, 2014 at 12:07 PM, Andrew Pinski <pinskia@gmail.com> wrote: > On Thu, May 15, 2014 at 1:05 AM, Konstantin Serebryany > <konstantin.s.serebryany@gmail.com> wrote: >> H.J., >> Thanks for the patches. Please (re)send them to llvm-commits, >> otherwise I can not accept them. > > > I think this is bogus reasoning. You should be able to take and post > them yourself. Those patches. I can't, sorry.
--- sanitizer_common/sanitizer_linux_libcdep.cc.x32 2014-05-13 08:03:33.524933988 -0700 +++ sanitizer_common/sanitizer_linux_libcdep.cc 2014-05-13 14:58:22.483884958 -0700 @@ -310,6 +310,9 @@ uptr ThreadDescriptorSize() { int minor = internal_simple_strtoll(buf + 8, &end, 10); if (end != buf + 8 && (*end == '\0' || *end == '.')) { /* sizeof(struct thread) values from various glibc versions. */ +#if defined(__x86_64__) && !defined(_LP64) + val = 1728; +#else if (minor <= 3) val = FIRST_32_SECOND_64(1104, 1696); else if (minor == 4) @@ -324,6 +327,7 @@ uptr ThreadDescriptorSize() { val = FIRST_32_SECOND_64(1168, 2288); else val = FIRST_32_SECOND_64(1216, 2304); +#endif } if (val) atomic_store(&kThreadDescriptorSize, val, memory_order_relaxed);