Patchwork PATCH: PR other/55333: libsanitizer StackTrace::FastUnwindStack wrong x32

login
register
mail settings
Submitter H.J. Lu
Date Nov. 15, 2012, 5:14 p.m.
Message ID <20121115171423.GA17883@gmail.com>
Download mbox | patch
Permalink /patch/199366/
State New
Headers show

Comments

H.J. Lu - Nov. 15, 2012, 5:14 p.m.
On Thu, Nov 15, 2012 at 09:05:13AM -0800, Konstantin Serebryany wrote:
> +dvyukov, +glider, +samsonov
> 
> Sorry I am lagging behind e-mail, but I am sure Dmitry, Alexander or
> Alexey may submit the patch upstream.
> Please make sure to comment the reason for using a separate typedef.

Here is the patch with comments.

> 
> We need our custom unwinder based on frame pointers to remain the
> default choice on x86[_64] because this is a hotspot
> and replacing it with any library call (especially if that call does
> not use frame pointers but instead uses debug info) will slow down
> the tool significantly.
> The asan docs explicitly say that you need -fno-omit-frame-pointers to
> get reasonable bug reports.
> 
> Note that on ARM (and on Windows) we are using a library call.
> 

Thanks.

H.J.
---
2012-11-14  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 frame unwind.

Patch

diff --git a/libsanitizer/include/sanitizer/common_interface_defs.h b/libsanitizer/include/sanitizer/common_interface_defs.h
index 4ac7609..d78d280 100644
--- a/libsanitizer/include/sanitizer/common_interface_defs.h
+++ b/libsanitizer/include/sanitizer/common_interface_defs.h
@@ -46,6 +46,13 @@  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;
diff --git a/libsanitizer/sanitizer_common/sanitizer_stacktrace.cc b/libsanitizer/sanitizer_common/sanitizer_stacktrace.cc
index f6d7a09..915c4b8 100644
--- a/libsanitizer/sanitizer_common/sanitizer_stacktrace.cc
+++ b/libsanitizer/sanitizer_common/sanitizer_stacktrace.cc
@@ -120,18 +120,18 @@  void StackTrace::FastUnwindStack(uptr pc, uptr bp,
                                  uptr stack_top, uptr stack_bottom) {
   CHECK(size == 0 && trace[0] == pc);
   size = 1;
-  uptr *frame = (uptr*)bp;
-  uptr *prev_frame = frame;
+  uhwptr *frame = (uhwptr *)bp;
+  uhwptr *prev_frame = frame;
   while (frame >= prev_frame &&
-         frame < (uptr*)stack_top - 2 &&
-         frame > (uptr*)stack_bottom &&
+         frame < (uhwptr *)stack_top - 2 &&
+         frame > (uhwptr *)stack_bottom &&
          size < max_size) {
-    uptr pc1 = frame[1];
+    uhwptr pc1 = frame[1];
     if (pc1 != pc) {
-      trace[size++] = pc1;
+      trace[size++] = (uptr) pc1;
     }
     prev_frame = frame;
-    frame = (uptr*)frame[0];
+    frame = (uhwptr *)frame[0];
   }
 }