diff mbox series

libgo patch committed: break dependence on unwind-pe.h

Message ID CAOyqgcWVhNNCRqcAyJedL0un-8RpJ7bO1R8tSTmvk7g_AkJDyA@mail.gmail.com
State New
Headers show
Series libgo patch committed: break dependence on unwind-pe.h | expand

Commit Message

Ian Lance Taylor May 2, 2018, 9:53 p.m. UTC 
This patch by Than McIntosh breaks the dependence of go-unwind.c on
unwind-pe.h, by adding the required definitions and code directly to
go-unwind.c.  go-unwind.c still depends on the public unwind.h API.
This makes it easier to build libgo separately.  Bootstrapped and ran
Go testsuite on x86_64-pc-linux-gnu.  Committed to mainline.

Ian

Comments

Rainer Orth May 3, 2018, 7:03 p.m. UTC | #1 
Hi Ian,

> This patch by Than McIntosh breaks the dependence of go-unwind.c on
> unwind-pe.h, by adding the required definitions and code directly to
> go-unwind.c.  go-unwind.c still depends on the public unwind.h API.
> This makes it easier to build libgo separately.  Bootstrapped and ran
> Go testsuite on x86_64-pc-linux-gnu.  Committed to mainline.

I strongly suspect that this patch severely broke Go on Solaris/SPARC:
between 20180502 (r259840) and 20180503 (r259897) there appeared tons of
new execution failures, both 32 and 64-bit:

+FAIL: go.go-torture/execute/printnil.go execution,  -O0 
+FAIL: go.go-torture/execute/printnil.go execution,  -O1 
+FAIL: go.go-torture/execute/printnil.go execution,  -O2 
+FAIL: go.go-torture/execute/printnil.go execution,  -O2 -fbounds-check 
+FAIL: go.go-torture/execute/printnil.go execution,  -O2 -fomit-frame-pointer -finline-functions 
+FAIL: go.go-torture/execute/printnil.go execution,  -O2 -fomit-frame-pointer -finline-functions -funroll-loops 
+FAIL: go.go-torture/execute/printnil.go execution,  -O3 -g 
+FAIL: go.go-torture/execute/printnil.go execution,  -Os 

and many more, also in libgo and gotools tests.

One example is printnil.x:

Thread 9 received signal SIGSEGV, Segmentation fault.
[Switching to Thread 4 (LWP 4)]
read_encoded_value (val=<synthetic pointer>, p=0xfebcb953 "", 
    encoding=3 '\003', context=0x0)
    at /vol/gcc/src/hg/trunk/local/libgo/runtime/go-unwind.c:250
250                 decoded = (_Unwind_Internal_Ptr)(*(const void *const *)p);
(gdb) where
#0  read_encoded_value (val=<synthetic pointer>, p=0xfebcb953 "", 
    encoding=3 '\003', context=0x0)
    at /vol/gcc/src/hg/trunk/local/libgo/runtime/go-unwind.c:250
#1  __gccgo_personality_v0 (version=<optimized out>, actions=1, 
    exception_class=<optimized out>, ue_header=0x1cc44000, context=0x105810ec)
    at /vol/gcc/src/hg/trunk/local/libgo/runtime/go-unwind.c:472
#2  0xfefbd450 in _Unwind_RaiseException (exc=0x1cc44000)
    at /builds2/ulhg/workspace/Solaris_Trunk/Userland/full-build/02b-build-sparc/components/gcc7/gcc-7.3.0/libgcc/unwind.inc:113
#3  0xfe59fb00 in runtime.throwException ()
    at /vol/gcc/src/hg/trunk/local/libgo/runtime/go-unwind.c:124
#4  0xfe9e5a90 in runtime.unwindStack ()
    at /vol/gcc/src/hg/trunk/local/libgo/go/runtime/panic.go:336
#5  runtime.gopanic (e=...)
    at /vol/gcc/src/hg/trunk/local/libgo/go/runtime/panic.go:527
#6  0xfe5a0758 in runtime_panicstring (s=0xfe3e32d8 "nil pointer dereference")
    at /vol/gcc/src/hg/trunk/local/libgo/runtime/panic.c:38
#7  0xfe59f65c in __go_runtime_error (i=<optimized out>)
    at /vol/gcc/src/hg/trunk/local/libgo/runtime/go-runtime-error.c:76
#8  0x000120b4 in main.MyType.String ()
    at /vol/gcc/src/hg/trunk/local/gcc/testsuite/go.go-torture/execute/printnil.go:11
#9  0xfe6efa08 in fmt.pp.handleMethods (p=0x10bf4000, verb=115)
    at /vol/gcc/src/hg/trunk/local/libgo/go/fmt/print.go:603
#10 0xfe6eebdc in fmt.pp.printArg (p=0x10bf4000, arg=..., verb=115)
    at /vol/gcc/src/hg/trunk/local/libgo/go/fmt/print.go:686
#11 0xfe6f0d84 in fmt.pp.doPrintf (p=0x10bf4000, format=..., a=...)
    at /vol/gcc/src/hg/trunk/local/libgo/go/fmt/print.go:1003
#12 0xfe6f1460 in fmt.Sprintf (format=..., a=...)
    at /vol/gcc/src/hg/trunk/local/libgo/go/fmt/print.go:203
#13 0x000121e0 in main.main ()
    at /vol/gcc/src/hg/trunk/local/gcc/testsuite/go.go-torture/execute/printnil.go:16

Seems like the new code doesn't play well on strict-alignment targets.

	Rainer
Li, Pan2 via Gcc-patches May 3, 2018, 7:35 p.m. UTC | #2 
Thanks for catching that.

I don't have access to test machines, but could you possibly try the
attached patch?

Thanks, Than



On Thu, May 3, 2018 at 3:03 PM Rainer Orth <ro@cebitec.uni-bielefeld.de>
wrote:

> Hi Ian,

> > This patch by Than McIntosh breaks the dependence of go-unwind.c on
> > unwind-pe.h, by adding the required definitions and code directly to
> > go-unwind.c.  go-unwind.c still depends on the public unwind.h API.
> > This makes it easier to build libgo separately.  Bootstrapped and ran
> > Go testsuite on x86_64-pc-linux-gnu.  Committed to mainline.

> I strongly suspect that this patch severely broke Go on Solaris/SPARC:
> between 20180502 (r259840) and 20180503 (r259897) there appeared tons of
> new execution failures, both 32 and 64-bit:

> +FAIL: go.go-torture/execute/printnil.go execution,  -O0
> +FAIL: go.go-torture/execute/printnil.go execution,  -O1
> +FAIL: go.go-torture/execute/printnil.go execution,  -O2
> +FAIL: go.go-torture/execute/printnil.go execution,  -O2 -fbounds-check
> +FAIL: go.go-torture/execute/printnil.go execution,  -O2
-fomit-frame-pointer -finline-functions
> +FAIL: go.go-torture/execute/printnil.go execution,  -O2
-fomit-frame-pointer -finline-functions -funroll-loops
> +FAIL: go.go-torture/execute/printnil.go execution,  -O3 -g
> +FAIL: go.go-torture/execute/printnil.go execution,  -Os

> and many more, also in libgo and gotools tests.

> One example is printnil.x:

> Thread 9 received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 4 (LWP 4)]
> read_encoded_value (val=<synthetic pointer>, p=0xfebcb953 "",
>      encoding=3 '\003', context=0x0)
>      at /vol/gcc/src/hg/trunk/local/libgo/runtime/go-unwind.c:250
> 250                 decoded = (_Unwind_Internal_Ptr)(*(const void *const
*)p);
> (gdb) where
> #0  read_encoded_value (val=<synthetic pointer>, p=0xfebcb953 "",
>      encoding=3 '\003', context=0x0)
>      at /vol/gcc/src/hg/trunk/local/libgo/runtime/go-unwind.c:250
> #1  __gccgo_personality_v0 (version=<optimized out>, actions=1,
>      exception_class=<optimized out>, ue_header=0x1cc44000,
context=0x105810ec)
>      at /vol/gcc/src/hg/trunk/local/libgo/runtime/go-unwind.c:472
> #2  0xfefbd450 in _Unwind_RaiseException (exc=0x1cc44000)
>      at
/builds2/ulhg/workspace/Solaris_Trunk/Userland/full-build/02b-build-sparc/components/gcc7/gcc-7.3.0/libgcc/unwind.inc:113
> #3  0xfe59fb00 in runtime.throwException ()
>      at /vol/gcc/src/hg/trunk/local/libgo/runtime/go-unwind.c:124
> #4  0xfe9e5a90 in runtime.unwindStack ()
>      at /vol/gcc/src/hg/trunk/local/libgo/go/runtime/panic.go:336
> #5  runtime.gopanic (e=...)
>      at /vol/gcc/src/hg/trunk/local/libgo/go/runtime/panic.go:527
> #6  0xfe5a0758 in runtime_panicstring (s=0xfe3e32d8 "nil pointer
dereference")
>      at /vol/gcc/src/hg/trunk/local/libgo/runtime/panic.c:38
> #7  0xfe59f65c in __go_runtime_error (i=<optimized out>)
>      at /vol/gcc/src/hg/trunk/local/libgo/runtime/go-runtime-error.c:76
> #8  0x000120b4 in main.MyType.String ()
>      at
/vol/gcc/src/hg/trunk/local/gcc/testsuite/go.go-torture/execute/printnil.go:11
> #9  0xfe6efa08 in fmt.pp.handleMethods (p=0x10bf4000, verb=115)
>      at /vol/gcc/src/hg/trunk/local/libgo/go/fmt/print.go:603
> #10 0xfe6eebdc in fmt.pp.printArg (p=0x10bf4000, arg=..., verb=115)
>      at /vol/gcc/src/hg/trunk/local/libgo/go/fmt/print.go:686
> #11 0xfe6f0d84 in fmt.pp.doPrintf (p=0x10bf4000, format=..., a=...)
>      at /vol/gcc/src/hg/trunk/local/libgo/go/fmt/print.go:1003
> #12 0xfe6f1460 in fmt.Sprintf (format=..., a=...)
>      at /vol/gcc/src/hg/trunk/local/libgo/go/fmt/print.go:203
> #13 0x000121e0 in main.main ()
>      at
/vol/gcc/src/hg/trunk/local/gcc/testsuite/go.go-torture/execute/printnil.go:16

> Seems like the new code doesn't play well on strict-alignment targets.

>          Rainer

> --

-----------------------------------------------------------------------------
> Rainer Orth, Center for Biotechnology, Bielefeld University
diff --git a/libgo/runtime/go-unwind.c b/libgo/runtime/go-unwind.c
index 536a619d..6464bec7 100644
--- a/libgo/runtime/go-unwind.c
+++ b/libgo/runtime/go-unwind.c
@@ -247,7 +247,7 @@ read_encoded_value (struct _Unwind_Context *context, uint8_t encoding,
               break;
             }
           case DW_EH_PE_absptr:
-            decoded = (_Unwind_Internal_Ptr)(*(const void *const *)p);
+            __builtin_memcpy (&decoded, (const void *)p, sizeof(const void*));
             p += sizeof(void *);
             break;
           default:
Rainer Orth May 3, 2018, 9:53 p.m. UTC | #3 
Hi Than,

> Thanks for catching that.
>
> I don't have access to test machines, but could you possibly try the
> attached patch?

I just did.  However, it made no difference, while a full reversal of
the patch fixed all new go and libgo failures on sparc-sun-solaris2.11.

	Rainer
diff mbox series

Patch

Index: libgo/runtime/go-unwind.c
===================================================================
--- libgo/runtime/go-unwind.c	(revision 259805)
+++ libgo/runtime/go-unwind.c	(working copy)
@@ -10,11 +10,30 @@ 
 #include <unistd.h>
 
 #include "unwind.h"
-#define NO_SIZE_OF_ENCODED_VALUE
-#include "unwind-pe.h"
 
 #include "runtime.h"
 
+/* These constants are documented here:
+   https://refspecs.linuxfoundation.org/LSB_3.0.0/LSB-PDA/LSB-PDA/dwarfext.html
+ */
+
+#define DW_EH_PE_omit     0xff
+#define DW_EH_PE_absptr   0x00
+#define DW_EH_PE_uleb128  0x01
+#define DW_EH_PE_udata2   0x02
+#define DW_EH_PE_udata4   0x03
+#define DW_EH_PE_udata8   0x04
+#define DW_EH_PE_sleb128  0x09
+#define DW_EH_PE_sdata2   0x0A
+#define DW_EH_PE_sdata4   0x0B
+#define DW_EH_PE_sdata8   0x0C
+#define DW_EH_PE_pcrel    0x10
+#define DW_EH_PE_textrel  0x20
+#define DW_EH_PE_datarel  0x30
+#define DW_EH_PE_funcrel  0x40
+#define DW_EH_PE_aligned  0x50
+#define DW_EH_PE_indirect 0x80
+
 /* The code for a Go exception.  */
 
 #ifdef __ARM_EABI_UNWINDER__
@@ -109,6 +128,150 @@  throwException ()
   abort ();
 }
 
+static inline _Unwind_Ptr
+encoded_value_base (uint8_t encoding, struct _Unwind_Context *context)
+{
+  if (encoding == DW_EH_PE_omit)
+    return 0;
+  switch (encoding & 0x70)
+    {
+      case DW_EH_PE_absptr:
+      case DW_EH_PE_pcrel:
+      case DW_EH_PE_aligned:
+        return 0;
+      case DW_EH_PE_textrel:
+        return _Unwind_GetTextRelBase(context);
+      case DW_EH_PE_datarel:
+        return _Unwind_GetDataRelBase(context);
+      case DW_EH_PE_funcrel:
+        return _Unwind_GetRegionStart(context);
+    }
+  abort ();
+}
+
+/* Read an unsigned leb128 value.  */
+
+static inline const uint8_t *
+read_uleb128 (const uint8_t *p, _uleb128_t *val)
+{
+  unsigned int shift = 0;
+  _uleb128_t result = 0;
+  uint8_t byte;
+
+  do
+    {
+      byte = *p++;
+      result |= ((_uleb128_t)byte & 0x7f) << shift;
+      shift += 7;
+    }
+  while (byte & 0x80);
+
+  *val = result;
+  return p;
+}
+
+/* Similar, but read a signed leb128 value.  */
+
+static inline const uint8_t *
+read_sleb128 (const uint8_t *p, _sleb128_t *val)
+{
+  unsigned int shift = 0;
+  _uleb128_t result = 0;
+  uint8_t byte;
+
+  do
+    {
+      byte = *p++;
+      result |= ((_uleb128_t)byte & 0x7f) << shift;
+      shift += 7;
+    }
+  while (byte & 0x80);
+
+  /* sign extension */
+  if (shift < (8 * sizeof(result)) && (byte & 0x40) != 0)
+    result |= (((_uleb128_t)~0) << shift);
+
+  *val = (_sleb128_t)result;
+  return p;
+}
+
+#define ROUND_UP_TO_PVB(x) (x + sizeof(void *) - 1) &- sizeof(void *)
+
+#define COPY_AND_ADVANCE(dst, ptr, typ) \
+  (dst = *((const typ*)ptr),            \
+   ptr += sizeof(typ))
+
+static inline const uint8_t *
+read_encoded_value (struct _Unwind_Context *context, uint8_t encoding,
+                    const uint8_t *p, _Unwind_Ptr *val)
+{
+  _Unwind_Ptr base = encoded_value_base (encoding, context);
+  _Unwind_Internal_Ptr decoded = 0;
+  const uint8_t *origp = p;
+
+  if (encoding == DW_EH_PE_aligned)
+    {
+      _Unwind_Internal_Ptr uip = (_Unwind_Internal_Ptr)p;
+      uip = ROUND_UP_TO_PVB (uip);
+      decoded = *(_Unwind_Internal_Ptr *)uip;
+      p = (const uint8_t *)(uip + sizeof(void *));
+    }
+  else
+    {
+      switch (encoding & 0x0f)
+        {
+          case DW_EH_PE_sdata2:
+          case DW_EH_PE_udata2:
+            COPY_AND_ADVANCE (decoded, p, uint16_t);
+            break;
+          case DW_EH_PE_sdata4:
+          case DW_EH_PE_udata4:
+            COPY_AND_ADVANCE (decoded, p, uint32_t);
+            break;
+          case DW_EH_PE_sdata8:
+          case DW_EH_PE_udata8:
+            COPY_AND_ADVANCE (decoded, p, uint64_t);
+            break;
+          case DW_EH_PE_uleb128:
+            {
+              _uleb128_t value;
+              p = read_uleb128 (p, &value);
+              decoded = (_Unwind_Internal_Ptr)value;
+              break;
+            }
+          case DW_EH_PE_sleb128:
+            {
+              _sleb128_t value;
+              p = read_sleb128 (p, &value);
+              decoded = (_Unwind_Internal_Ptr)value;
+              break;
+            }
+          case DW_EH_PE_absptr:
+            decoded = (_Unwind_Internal_Ptr)(*(const void *const *)p);
+            p += sizeof(void *);
+            break;
+          default:
+            abort ();
+        }
+
+      if (decoded == 0)
+        {
+          *val = decoded;
+          return p;
+        }
+
+      if ((encoding & 0x70) == DW_EH_PE_pcrel)
+        decoded += ((_Unwind_Internal_Ptr)origp);
+      else
+        decoded += base;
+
+      if ((encoding & DW_EH_PE_indirect) != 0)
+        decoded = *(_Unwind_Internal_Ptr *)decoded;
+    }
+  *val = decoded;
+  return p;
+}
+
 /* The rest of this code is really similar to gcc/unwind-c.c and
    libjava/exception.cc.  */