[PATCH] PR target/80556

[PATCH] PR target/80556

[Simon Wright]

[-- Attachment #1: Type: text/plain, Size: 1657 bytes --]

This PR was raised because of a bootstrap failure on Darwin. The cause
was a mishandled exception raised by Rtsfind.Load_Fail while
processing g-exptty.adb. g-exptty.adb had been updated on 2017-04-25
(but so had a lot of other parts of GNAT). Since a lot of other
compilations had been performed successfully by this point in the
build, one may assume that this was the first that actually caused an
exception.

The exception was mishandled because the default ldflags call up
-static-libgcc, and (on Darwin; not, it seems, on Debian [jessie]),
the notes on -static-libgcc/-shared-libgcc at
https://gcc.gnu.org/onlinedocs/gcc/Link-Options.html which say

   However, if a library or main executable is supposed to throw or
   catch exceptions, you must link it using the G++ driver, as
   appropriate for the languages used in the program, or using the
   option -shared-libgcc, such that it is linked with the shared
   libgcc.

do actually apply (this affects all the Ada executables, in this case
specifically gnat1).

The result of the mishandling of the exception is that gnat1 crashes
with SIGILL.

The change proposed here is to include -lSystem in such a way that
it's called in before the (static) libgcc, and thus supplies the
required exception unwinders.

Bootstrapped on Darwin 16.6.0 and on Debian Jessie.

A question: I've checked for x86_64-apple-darwin*, is this OK or
should it be more restrictive?

Changelog:

        2017-06-09 Simon Wright <simon@pushface.org>

        PR target/80556
        * configure.ac (stage1_ldflags): For Darwin, include -lSystem.
          (poststage1_ldflags): likewise.
        * configure: regenerated.


[-- Attachment #2: 80556-3.diff --]
[-- Type: application/octet-stream, Size: 2598 bytes --]

--- configure.ac-orig	2017-06-05 10:58:51.000000000 +0100
+++ configure.ac	2017-06-08 17:42:08.000000000 +0100
@@ -1650,7 +1650,12 @@
  # if supported.  But if the user explicitly specified the libraries to use,
  # trust that they are doing what they want.
  if test "$stage1_libs" = "" -a "$have_static_libs" = yes; then
-   stage1_ldflags="-static-libstdc++ -static-libgcc"
+   # .. but on Darwin, for exception handling, we include -lSystem
+   # (which is included before -lgcc, and uses the correct unwinder)
+   case $host in
+    x86_64-apple-darwin*) stage1_ldflags="-lSystem -static-libstdc++ -static-libgcc";;
+    *) stage1_ldflags="-static-libstdc++ -static-libgcc";;
+   esac
  fi])
 AC_SUBST(stage1_ldflags)
 
@@ -1679,7 +1684,12 @@
  # statically.  But if the user explicitly specified the libraries to
  # use, trust that they are doing what they want.
  if test "$poststage1_libs" = ""; then
-   poststage1_ldflags="-static-libstdc++ -static-libgcc"
+   # .. but on Darwin, for exception handling, we include -lSystem
+   # (which is included before -lgcc, and uses the correct unwinder)
+   case $host in
+    x86_64-apple-darwin*) poststage1_ldflags="-lSystem -static-libstdc++ -static-libgcc";;
+    *) poststage1_ldflags="-static-libstdc++ -static-libgcc";;
+   esac
  fi])
 AC_SUBST(poststage1_ldflags)
 
--- configure-orig	2017-06-05 10:59:09.000000000 +0100
+++ configure	2017-06-08 17:42:25.000000000 +0100
@@ -5819,7 +5819,12 @@
  # if supported.  But if the user explicitly specified the libraries to use,
  # trust that they are doing what they want.
  if test "$stage1_libs" = "" -a "$have_static_libs" = yes; then
-   stage1_ldflags="-static-libstdc++ -static-libgcc"
+   # .. but on Darwin, for exception handling, we include -lSystem
+   # (which is included before -lgcc, and uses the correct unwinder)
+   case $host in
+    x86_64-apple-darwin*) stage1_ldflags="-lSystem -static-libstdc++ -static-libgcc";;
+    *) stage1_ldflags="-static-libstdc++ -static-libgcc";;
+   esac
  fi
 fi
 
@@ -5855,7 +5860,12 @@
  # statically.  But if the user explicitly specified the libraries to
  # use, trust that they are doing what they want.
  if test "$poststage1_libs" = ""; then
-   poststage1_ldflags="-static-libstdc++ -static-libgcc"
+   # .. but on Darwin, for exception handling, we include -lSystem
+   # (which is included before -lgcc, and uses the correct unwinder)
+   case $host in
+    x86_64-apple-darwin*) poststage1_ldflags="-lSystem -static-libstdc++ -static-libgcc";;
+    *) poststage1_ldflags="-static-libstdc++ -static-libgcc";;
+   esac
  fi
 fi
 

Re: [PATCH] PR target/80556

[Jeff Law]

On 06/09/2017 07:57 AM, Simon Wright wrote:
>         2017-06-09 Simon Wright <simon@pushface.org>
> 
>         PR target/80556
>         * configure.ac (stage1_ldflags): For Darwin, include -lSystem.
>           (poststage1_ldflags): likewise.
>         * configure: regenerated.
I'm a bit confused here.  Isn't -lSystem included in darwin's LIB_SPEC
in which case the right things ought to already be happening, shouldn't it?

Jeff

Re: [PATCH] PR target/80556

[Mike Stump]

On Jun 9, 2017, at 6:57 AM, Simon Wright <simon@pushface.org> wrote:
> 
> This PR was raised because of a bootstrap failure on Darwin.

> A question: I've checked for x86_64-apple-darwin*, is this OK or
> should it be more restrictive?

That seems ok.

Ok.

If anyone sees any fallout from this, please speak up.  I'm hoping this isn't a back port candidate.

Re: [PATCH] PR target/80556

[Simon Wright]

On 28 Jun 2017, at 18:40, Jeff Law <law@redhat.com> wrote:
> 
> On 06/09/2017 07:57 AM, Simon Wright wrote:
>>        2017-06-09 Simon Wright <simon@pushface.org>
>> 
>>        PR target/80556
>>        * configure.ac (stage1_ldflags): For Darwin, include -lSystem.
>>          (poststage1_ldflags): likewise.
>>        * configure: regenerated.
> I'm a bit confused here.  Isn't -lSystem included in darwin's LIB_SPEC
> in which case the right things ought to already be happening, shouldn't it?

The specs that involve -lSystem are

*link_gcc_c_sequence:
%:version-compare(>= 10.6 mmacosx-version-min= -no_compact_unwind)    %{!static:%{!static-libgcc:       %:version-compare(>= 10.6 mmacosx-version-min= -lSystem) } }    %{fno-pic|fno-PIC|fno-pie|fno-PIE|fapple-kext|mkernel|static|mdynamic-no-pic:       %:version-compare(>= 10.7 mmacosx-version-min= -no_pie) } %G %L

*lib:
%{!static:-lSystem}

but I also see

*libgcc:
%{static-libgcc|static: -lgcc_eh -lgcc; ....

which might be the root of the problem?

Looking at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80556#c39, I report that

   $ gnatmake raiser -largs -static-libgcc -static-libstdc++

resulted in the link command

   /usr/bin/ld -dynamic -arch x86_64 -macosx_version_min 10.12.0
   -weak_reference_mismatches non-weak -o raiser -L./
   -L/opt/gcc-7.1.0/lib/gcc/x86_64-apple-darwin15/7.1.0/adalib/
   -L/opt/gcc-7.1.0/lib/gcc/x86_64-apple-darwin15/7.1.0
   -L/opt/gcc-7.1.0/lib/gcc/x86_64-apple-darwin15/7.1.0/../../.. b~raiser.o
   ./raiser.o -v
   /opt/gcc-7.1.0/lib/gcc/x86_64-apple-darwin15/7.1.0/adalib/libgnat.a
   -no_compact_unwind -lgcc_eh -lgcc -lSystem

i.e. -lSystem is *after* -lgcc, so that its exception handling won't be invoked.

I don't know what -lgcc_eh does, but my patch would be pretty much equivalent to changing the libgcc spec above to

*libgcc:
%{static-libgcc|static: -lSystem -lgcc_eh -lgcc; ....

and if that would be OK it would obviously be much better.

I've rebuilt gcc-8-20170528 with this change alone (i.e. not the patch currently posted here), successfully.

If I propose this alternative patch, should it be a new post, or should I continue this thread?

Re: [PATCH] PR target/80556

[Simon Wright]

[-- Attachment #1: Type: text/plain, Size: 2543 bytes --]

On 29 Jun 2017, at 21:41, Simon Wright <simon@pushface.org> wrote:
> 
> On 28 Jun 2017, at 18:40, Jeff Law <law@redhat.com> wrote:
>> 
>> On 06/09/2017 07:57 AM, Simon Wright wrote:
>>>       2017-06-09 Simon Wright <simon@pushface.org>
>>> 
>>>       PR target/80556
>>>       * configure.ac (stage1_ldflags): For Darwin, include -lSystem.
>>>         (poststage1_ldflags): likewise.
>>>       * configure: regenerated.
>> I'm a bit confused here.  Isn't -lSystem included in darwin's LIB_SPEC
>> in which case the right things ought to already be happening, shouldn't it?
> 
> The specs that involve -lSystem are
> 
> *link_gcc_c_sequence:
> %:version-compare(>= 10.6 mmacosx-version-min= -no_compact_unwind)    %{!static:%{!static-libgcc:       %:version-compare(>= 10.6 mmacosx-version-min= -lSystem) } }    %{fno-pic|fno-PIC|fno-pie|fno-PIE|fapple-kext|mkernel|static|mdynamic-no-pic:       %:version-compare(>= 10.7 mmacosx-version-min= -no_pie) } %G %L
> 
> *lib:
> %{!static:-lSystem}
> 
> but I also see
> 
> *libgcc:
> %{static-libgcc|static: -lgcc_eh -lgcc; ....
> 
> which might be the root of the problem?
> 
> Looking at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80556#c39, I report that
> 
>   $ gnatmake raiser -largs -static-libgcc -static-libstdc++
> 
> resulted in the link command
> 
>   /usr/bin/ld -dynamic -arch x86_64 -macosx_version_min 10.12.0
>   -weak_reference_mismatches non-weak -o raiser -L./
>   -L/opt/gcc-7.1.0/lib/gcc/x86_64-apple-darwin15/7.1.0/adalib/
>   -L/opt/gcc-7.1.0/lib/gcc/x86_64-apple-darwin15/7.1.0
>   -L/opt/gcc-7.1.0/lib/gcc/x86_64-apple-darwin15/7.1.0/../../.. b~raiser.o
>   ./raiser.o -v
>   /opt/gcc-7.1.0/lib/gcc/x86_64-apple-darwin15/7.1.0/adalib/libgnat.a
>   -no_compact_unwind -lgcc_eh -lgcc -lSystem
> 
> i.e. -lSystem is *after* -lgcc, so that its exception handling won't be invoked.
> 
> I don't know what -lgcc_eh does, but my patch would be pretty much equivalent to changing the libgcc spec above to
> 
> *libgcc:
> %{static-libgcc|static: -lSystem -lgcc_eh -lgcc; ....
> 
> and if that would be OK it would obviously be much better.
> 
> I've rebuilt gcc-8-20170528 with this change alone (i.e. not the patch currently posted here), successfully.

I've rebuilt and tested gcc-8-20170820 with this change, successfully.

gcc/Changelog:

       2017-09-01 Simon Wright <simon@pushface.org>

       PR target/80556
       * config/darwin.h (REAL_LIBGCC_SPEC): for static-libgcc|static, include -lSystem first.


[-- Attachment #2: 80556-darwin.h.diff --]
[-- Type: application/octet-stream, Size: 796 bytes --]

--- gcc/config/darwin.h.orig	2017-01-16 21:33:07.000000000 +0000
+++ gcc/config/darwin.h	2017-07-03 14:19:55.000000000 +0100
@@ -339,10 +339,12 @@
    libraries, you need to explicitly say -static-libgcc.
 
    If it is linked against, it has to be before -lgcc, because it may
-   need symbols from -lgcc.  */
+   need symbols from -lgcc.
+
+   For PR80556 and PR61027, link libSystem before a static libgcc.  */
 #undef REAL_LIBGCC_SPEC
 #define REAL_LIBGCC_SPEC						   \
-   "%{static-libgcc|static: -lgcc_eh -lgcc;				   \
+   "%{static-libgcc|static: -lSystem -lgcc_eh -lgcc;			   \
       shared-libgcc|fexceptions|fgnu-runtime:				   \
        %:version-compare(!> 10.5 mmacosx-version-min= -lgcc_s.10.4)	   \
        %:version-compare(>< 10.5 10.6 mmacosx-version-min= -lgcc_s.10.5)   \

[-- Attachment #3: Type: text/plain, Size: 2 bytes --]

Re: [PATCH] PR target/80556

[Iain Sandoe]

Hi Simon,

> On 29 Jun 2017, at 21:41, Simon Wright <simon@pushface.org> wrote:
> 
> On 28 Jun 2017, at 18:40, Jeff Law <law@redhat.com> wrote:
>> 
>> On 06/09/2017 07:57 AM, Simon Wright wrote:
>>>       2017-06-09 Simon Wright <simon@pushface.org>
>>> 
>>>       PR target/80556
>>>       * configure.ac (stage1_ldflags): For Darwin, include -lSystem.
>>>         (poststage1_ldflags): likewise.
>>>       * configure: regenerated.
>> I'm a bit confused here.  Isn't -lSystem included in darwin's LIB_SPEC
>> in which case the right things ought to already be happening, shouldn't it?
> 
> The specs that involve -lSystem are

> I've rebuilt gcc-8-20170528 with this change alone (i.e. not the patch currently posted here), successfully.
> 
> If I propose this alternative patch, should it be a new post, or should I continue this thread?

thanks for the patch.

The basic idea seems sound - as a workaround (as noted in comment #20 in the PR, we should really rationalise the libgcc/crts stuff to reflect the modern world, but these things take time...).

The patch as you have it would apply to every version of Darwin.

AFAICT from the published sources, i386 Darwin should be able to work with the libgcc unwinder  (and all earlier Darwin *have* to) - so I’ve proposed a modified patch in the PR that makes the changes specific to m64 x86 and doesn’t make any alteration for PPC and/or Darwin < 10.

HTH,
Iain

Iain Sandoe
CodeSourcery / Mentor Embedded / Siemens

Re: [PATCH] PR target/80556

[Simon Wright]

[-- Attachment #1: Type: text/plain, Size: 1690 bytes --]

On 18 Sep 2017, at 21:09, Iain Sandoe <iain@codesourcery.com> wrote:
> 
> Hi Simon,
> 
>> On 29 Jun 2017, at 21:41, Simon Wright <simon@pushface.org> wrote:
>> 
>> On 28 Jun 2017, at 18:40, Jeff Law <law@redhat.com> wrote:
>>> 
>>> On 06/09/2017 07:57 AM, Simon Wright wrote:
>>>>      2017-06-09 Simon Wright <simon@pushface.org>
>>>> 
>>>>      PR target/80556
>>>>      * configure.ac (stage1_ldflags): For Darwin, include -lSystem.
>>>>        (poststage1_ldflags): likewise.
>>>>      * configure: regenerated.
>>> I'm a bit confused here.  Isn't -lSystem included in darwin's LIB_SPEC
>>> in which case the right things ought to already be happening, shouldn't it?
>> 
>> The specs that involve -lSystem are
> 
>> I've rebuilt gcc-8-20170528 with this change alone (i.e. not the patch currently posted here), successfully.
>> 
>> If I propose this alternative patch, should it be a new post, or should I continue this thread?
> 
> thanks for the patch.
> 
> The basic idea seems sound - as a workaround (as noted in comment #20 in the PR, we should really rationalise the libgcc/crts stuff to reflect the modern world, but these things take time...).
> 
> The patch as you have it would apply to every version of Darwin.
> 
> AFAICT from the published sources, i386 Darwin should be able to work with the libgcc unwinder (and all earlier Darwin *have* to) - so I’ve proposed a modified patch in the PR that makes the changes specific to m64 x86 and doesn’t make any alteration for PPC and/or Darwin < 10.

That sounds like the right thing to do. I hadn't considered the older hardware/os issues (I only have kit back to macOS 10.11, Darwin 15).

Re: [PATCH] PR target/80556

[Iain Sandoe]

[-- Attachment #1: Type: text/plain, Size: 1412 bytes --]

> On 18 Sep 2017, at 22:08, Simon Wright <simon@pushface.org> wrote:
> 
> On 18 Sep 2017, at 21:09, Iain Sandoe <iain@codesourcery.com> wrote:
>> 

>>> If I propose this alternative patch, should it be a new post, or should I continue this thread?
>> 
>> thanks for the patch.
>> 
>> The basic idea seems sound - as a workaround (as noted in comment #20 in the PR, we should really rationalise the libgcc/crts stuff to reflect the modern world, but these things take time...).
>> 
>> The patch as you have it would apply to every version of Darwin.
>> 
>> AFAICT from the published sources, i386 Darwin should be able to work with the libgcc unwinder (and all earlier Darwin *have* to) - so I’ve proposed a modified patch in the PR that makes the changes specific to m64 x86 and doesn’t make any alteration for PPC and/or Darwin < 10.
> 
> That sounds like the right thing to do. I hadn't considered the older hardware/os issues (I only have kit back to macOS 10.11, Darwin 15).

So here’s the revised version with the comments slightly updated, checked Darwin10,15,16 x86_64 and i386 in progress,
OK if i386 succeeds?

Iain Sandoe
CodeSourcery / Mentor Embedded / Siemens

gcc/

2017-09-xx Iain Sandoe <iain@codesourcery.com>

	PR target/80556
	* config/i386/darwin.h (REAL_LIB_SPEC): New; put libSystem ahead of libgcc_eh
	for m64.
	* config/i386/darwin64.h: Likewise.



[-- Attachment #2: pr80556-v2.txt --]
[-- Type: text/plain, Size: 3766 bytes --]

diff --git a/gcc/config/i386/darwin.h b/gcc/config/i386/darwin.h
index fccaf7e..321ed27 100644
--- a/gcc/config/i386/darwin.h
+++ b/gcc/config/i386/darwin.h
@@ -39,6 +39,32 @@ along with GCC; see the file COPYING3.  If not see
 #endif
 #endif
 
+/* WORKAROUND pr80556:
+   For x86_64 Darwin10 and later, the unwinder is in libunwind (redirected
+   from libSystem).  This doesn't use the keymgr (see keymgr.c) and therefore
+   the calls that libgcc makes to obtain the KEYMGR_GCC3_DW2_OBJ_LIST are not
+   updated to include new images, and might not even be valid for a single
+   image.
+   Therefore, for 64b exes at least, we must use the libunwind implementation,
+   even when static-libgcc is specified.  We put libSystem first so that
+   unwinder symbols are satisfied from there. */
+#undef REAL_LIBGCC_SPEC
+#define REAL_LIBGCC_SPEC						   \
+   "%{static-libgcc|static: 						   \
+      %{m64:%:version-compare(>= 10.6 mmacosx-version-min= -lSystem)}	   \
+        -lgcc_eh -lgcc;							   \
+      shared-libgcc|fexceptions|fgnu-runtime:				   \
+       %:version-compare(!> 10.5 mmacosx-version-min= -lgcc_s.10.4)	   \
+       %:version-compare(>< 10.5 10.6 mmacosx-version-min= -lgcc_s.10.5)   \
+       %:version-compare(!> 10.5 mmacosx-version-min= -lgcc_ext.10.4)	   \
+       %:version-compare(>= 10.5 mmacosx-version-min= -lgcc_ext.10.5)	   \
+       -lgcc ;								   \
+      :%:version-compare(>< 10.3.9 10.5 mmacosx-version-min= -lgcc_s.10.4) \
+       %:version-compare(>< 10.5 10.6 mmacosx-version-min= -lgcc_s.10.5)   \
+       %:version-compare(!> 10.5 mmacosx-version-min= -lgcc_ext.10.4)	   \
+       %:version-compare(>= 10.5 mmacosx-version-min= -lgcc_ext.10.5)	   \
+       -lgcc }"
+
 /* Size of the Obj-C jump buffer.  */
 #define OBJC_JBLEN ((TARGET_64BIT) ? ((9 * 2) + 3 + 16) : (18))
 
diff --git a/gcc/config/i386/darwin64.h b/gcc/config/i386/darwin64.h
index f2982ed..32cb789 100644
--- a/gcc/config/i386/darwin64.h
+++ b/gcc/config/i386/darwin64.h
@@ -21,6 +21,32 @@ along with GCC; see the file COPYING3.  If not see
 #undef  DARWIN_ARCH_SPEC
 #define DARWIN_ARCH_SPEC "%{m32:i386;:x86_64}"
 
+/* WORKAROUND pr80556:
+   For x86_64 Darwin10 and later, the unwinder is in libunwind (redirected
+   from libSystem).  This doesn't use the keymgr (see keymgr.c) and therefore
+   the calls that libgcc makes to obtain the KEYMGR_GCC3_DW2_OBJ_LIST are not
+   updated to include new images, and might not even be valid for a single
+   image.
+   Therefore, for 64b exes at least, we must use the libunwind implementation,
+   even when static-libgcc is specified.  We put libSystem first so that
+   unwinder symbols are satisfied from there. */
+#undef REAL_LIBGCC_SPEC
+#define REAL_LIBGCC_SPEC						   \
+   "%{static-libgcc|static: 						   \
+      %{!m32:%:version-compare(>= 10.6 mmacosx-version-min= -lSystem)}	   \
+        -lgcc_eh -lgcc;							   \
+      shared-libgcc|fexceptions|fgnu-runtime:				   \
+       %:version-compare(!> 10.5 mmacosx-version-min= -lgcc_s.10.4)	   \
+       %:version-compare(>< 10.5 10.6 mmacosx-version-min= -lgcc_s.10.5)   \
+       %:version-compare(!> 10.5 mmacosx-version-min= -lgcc_ext.10.4)	   \
+       %:version-compare(>= 10.5 mmacosx-version-min= -lgcc_ext.10.5)	   \
+       -lgcc ;								   \
+      :%:version-compare(>< 10.3.9 10.5 mmacosx-version-min= -lgcc_s.10.4) \
+       %:version-compare(>< 10.5 10.6 mmacosx-version-min= -lgcc_s.10.5)   \
+       %:version-compare(!> 10.5 mmacosx-version-min= -lgcc_ext.10.4)	   \
+       %:version-compare(>= 10.5 mmacosx-version-min= -lgcc_ext.10.5)	   \
+       -lgcc }"
+
 #undef  DARWIN_SUBARCH_SPEC
 #define DARWIN_SUBARCH_SPEC DARWIN_ARCH_SPEC
 

[-- Attachment #3: Type: text/plain, Size: 4 bytes --]

Re: [PATCH] PR target/80556

[Mike Stump]

[-- Attachment #1: Type: text/plain, Size: 1249 bytes --]

On Sep 22, 2017, at 2:55 AM, Iain Sandoe <iain@codesourcery.com> wrote:
> 
>> On 18 Sep 2017, at 22:08, Simon Wright <simon@pushface.org> wrote:
>> 
>> On 18 Sep 2017, at 21:09, Iain Sandoe <iain@codesourcery.com> wrote:
>>> 
> 
>>>> If I propose this alternative patch, should it be a new post, or should I continue this thread?
>>> 
>>> thanks for the patch.
>>> 
>>> The basic idea seems sound - as a workaround (as noted in comment #20 in the PR, we should really rationalise the libgcc/crts stuff to reflect the modern world, but these things take time...).
>>> 
>>> The patch as you have it would apply to every version of Darwin.
>>> 
>>> AFAICT from the published sources, i386 Darwin should be able to work with the libgcc unwinder (and all earlier Darwin *have* to) - so I’ve proposed a modified patch in the PR that makes the changes specific to m64 x86 and doesn’t make any alteration for PPC and/or Darwin < 10.
>> 
>> That sounds like the right thing to do. I hadn't considered the older hardware/os issues (I only have kit back to macOS 10.11, Darwin 15).
> 
> So here’s the revised version with the comments slightly updated, checked Darwin10,15,16 x86_64 and i386 in progress,
> OK if i386 succeeds?

Ok.

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 1578 bytes --]