Message ID | ydd7fkh71pw.fsf@lokon.CeBiTec.Uni-Bielefeld.DE |
---|---|
State | New |
Headers | show |
Hi Rainer, Thanks for looking at this! > On 14 Dec 2015, at 10:40, Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> wrote: > > As described in PR PR target/67973, newer assemblers on Mac OS X, which > are based on LLVM instead of gas, don't support .stab* directives any > longer. The following patch detects this situation and tries to fall > back to the older gas-based as if it is still accessible via as -Q. > > Tested on x86_64-apple-darwin15.2.0 and as expected the -gstabs* tests > now pass. > > However, I'm not really comfortable with this solution. Initially, I > forgot to wrap the -Q option to as in %{gstabs*:...}, which lead to a > bootstrap failure: the gas- and LLVM-based assemblers differ in a > number of other ways, as can be seen when comparing gcc/auto-host.h: FAOD, the changes below only occur if you omit the guard on “-Q” ? or they are present always? > --- build.llvm-as/gcc/auto-host.h 2015-11-27 10:53:31.000000000 +0100 > +++ build.gas/gcc/auto-host.h 2015-12-04 20:25:30.000000000 +0100 > @@ -351 +357 @@ > -/* #undef HAVE_AS_GDWARF2_DEBUG_FLAG */ > +#define HAVE_AS_GDWARF2_DEBUG_FLAG 1 > @@ -369 +375 @@ > -/* #undef HAVE_AS_GSTABS_DEBUG_FLAG */ > +#define HAVE_AS_GSTABS_DEBUG_FLAG 1 > @@ -388 +394 @@ > -/* #undef HAVE_AS_IX86_FFREEP */ > +#define HAVE_AS_IX86_FFREEP 1 > @@ -412 +418 @@ > -#define HAVE_AS_IX86_INTERUNIT_MOVQ 1 > +#define HAVE_AS_IX86_INTERUNIT_MOVQ 0 > @@ -424 +430 @@ > -#define HAVE_AS_IX86_REP_LOCK_PREFIX 1 > +/* #undef HAVE_AS_IX86_REP_LOCK_PREFIX */ > @@ -1176 +1182 @@ > -#define HAVE_GAS_CFI_PERSONALITY_DIRECTIVE 1 > +#define HAVE_GAS_CFI_PERSONALITY_DIRECTIVE 0 > @@ -1179 +1185 @@ > -#define HAVE_GAS_CFI_SECTIONS_DIRECTIVE 1 > +#define HAVE_GAS_CFI_SECTIONS_DIRECTIVE 0 ^^^ These two are definitely not safe without other changes (I have a local patch, but it might not be stage3 stuff). > @@ -1183 +1189 @@ > -#define HAVE_GAS_DISCRIMINATOR 1 > +/* #undef HAVE_GAS_DISCRIMINATOR */ > @@ -1298 +1304 @@ > -#define HAVE_GNU_AS 0 > +#define HAVE_GNU_AS 1 > > So, we can be pretty certain to hit cases where some file compiles and > assembles without -gstabs, but fails to assemble with -gstabs. Not > exactly the user experience I prefer. > > Given this, I'd rather have us not support stabs at all than via this > half-hearted approach. I agree, we should just disable it if the “default” assembler doesn’t support - so most of your patch would be unchanged. The actual practical outcome is really not terribly severe, it ony affects folks trying to target x86-darwin8 ( the last OS to use stabs as the first choice), and even those systems support some debugging with DWARF. > > What do you think? My view is it should still be a configure test, rather than a blanket disable for d15+ (simply because I’ve got back-ports of the newer tools that should be useable across the Darwin range, and hopefully those back-ports will be published before Xmas). The same issues would generally apply there. Iain > > Rainer > > > 2015-12-11 Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> > > PR target/67973 > * configure.ac (gcc_cv_as_stabs_directive): New test. > (gcc_cv_as_darwin_stabs_Q): New test. > * configure: Regenerate. > * config.in: Regenerate. > * config/darwin.h (DBX_DEBUGGING_INFO): Wrap in > HAVE_AS_STABS_DIRECTIVE. > (PREFERRED_DEBUGGING_TYPE): Likewise. > * config/i386/darwin.h (PREFERRED_DEBUGGING_TYPE): Only include > DBX_DEBUG if HAVE_AS_STABS_DIRECTIVE. > (ASM_STABS_Q_SPEC): Define. > (ASM_SPEC): Use it. > > # HG changeset patch > # Parent 7029fd86ac40d7ff34e1c43d729c6bf469416643 > Only support -gstabs on Mac OS X if assember supports it (PR target/67973) > > diff --git a/gcc/config/darwin.h b/gcc/config/darwin.h > --- a/gcc/config/darwin.h > +++ b/gcc/config/darwin.h > @@ -400,12 +400,13 @@ extern GTY(()) int darwin_ms_struct; > > #define ASM_DEBUG_SPEC "%{g*:%{!g0:%{!gdwarf*:--gstabs}}}" > > -/* We still allow output of STABS. */ > - > +/* We still allow output of STABS if the assembler supports it. */ > +#ifdef HAVE_AS_STABS_DIRECTIVE > #define DBX_DEBUGGING_INFO 1 > +#define PREFERRED_DEBUGGING_TYPE DBX_DEBUG > +#endif > > #define DWARF2_DEBUGGING_INFO 1 > -#define PREFERRED_DEBUGGING_TYPE DBX_DEBUG > > #define DEBUG_FRAME_SECTION "__DWARF,__debug_frame,regular,debug" > #define DEBUG_INFO_SECTION "__DWARF,__debug_info,regular,debug" > diff --git a/gcc/config/i386/darwin.h b/gcc/config/i386/darwin.h > --- a/gcc/config/i386/darwin.h > +++ b/gcc/config/i386/darwin.h > @@ -111,9 +111,16 @@ extern int darwin_emit_branch_islands; > %{g: %{!fno-eliminate-unused-debug-symbols: -feliminate-unused-debug-symbols }} " \ > DARWIN_CC1_SPEC > > +/* Pass -Q to assembler if necessary for stabs support. */ > +#ifdef HAVE_AS_DARWIN_STABS_Q > +#define ASM_STABS_Q_SPEC " %{gstabs*:-Q}" > +#else > +#define ASM_STABS_Q_SPEC "" > +#endif > + > #undef ASM_SPEC > #define ASM_SPEC "-arch %(darwin_arch) -force_cpusubtype_ALL \ > - %{static}" > + %{static}" ASM_STABS_Q_SPEC > > #define DARWIN_ARCH_SPEC "%{m64:x86_64;:i386}" > #define DARWIN_SUBARCH_SPEC DARWIN_ARCH_SPEC > @@ -226,7 +233,11 @@ do { \ > compiles default to stabs+. darwin9+ defaults to dwarf-2. */ > #ifndef DARWIN_PREFER_DWARF > #undef PREFERRED_DEBUGGING_TYPE > +#ifdef HAVE_AS_STABS_DIRECTIVE > #define PREFERRED_DEBUGGING_TYPE (TARGET_64BIT ? DWARF2_DEBUG : DBX_DEBUG) > +#else > +#define PREFERRED_DEBUGGING_TYPE DWARF2_DEBUG > +#endif > #endif > > /* Darwin uses the standard DWARF register numbers but the default > diff --git a/gcc/configure.ac b/gcc/configure.ac > --- a/gcc/configure.ac > +++ b/gcc/configure.ac > @@ -2909,6 +2909,11 @@ AC_DEFINE_UNQUOTED(HAVE_GAS_SHF_MERGE, > [`if test $gcc_cv_as_shf_merge = yes; then echo 1; else echo 0; fi`], > [Define 0/1 if your assembler supports marking sections with SHF_MERGE flag.]) > > +gcc_GAS_CHECK_FEATURE([stabs directive], gcc_cv_as_stabs_directive, ,, > +[.stabs "gcc2_compiled.",60,0,0,0],, > +[AC_DEFINE(HAVE_AS_STABS_DIRECTIVE, 1, > + [Define if your assembler supports .stabs.])]) > + > gcc_GAS_CHECK_FEATURE([COMDAT group support (GNU as)], > gcc_cv_as_comdat_group, > [elf,2,16,0], [--fatal-warnings], > @@ -3619,6 +3624,26 @@ AC_DEFINE_UNQUOTED(HAVE_LTO_PLUGIN, $gcc > [Define to the level of your linker's plugin support.]) > AC_MSG_RESULT($gcc_cv_lto_plugin) > > +# Target OS-specific assembler checks. > + > +case "$target_os" in > + darwin*) > + # If the default assembler doesn't support .stabs, check if the > + # gas-based one does. > + if test x$gcc_cv_as_stabs_directive = xno; then > + gcc_GAS_CHECK_FEATURE([.stabs directive via -Q], > + gcc_cv_as_darwin_stabs_Q,, [-Q], > + [.stabs "gcc2_compiled.",60,0,0,0],, > + [AC_DEFINE(HAVE_AS_STABS_DIRECTIVE, 1, > + [Define if your assembler supports .stabs.]) > + AC_DEFINE(HAVE_AS_DARWIN_STABS_Q, 1, > + [Define if your assembler needs -Q to support .stabs.])]) > + fi > + ;; > +esac > + > +# Target CPU-specific assembler checks. > + > case "$target" in > aarch64*-*-*) > gcc_GAS_CHECK_FEATURE([-mabi option], gcc_cv_as_aarch64_mabi,, > > -- > ----------------------------------------------------------------------------- > Rainer Orth, Center for Biotechnology, Bielefeld University
Hi Iain, >> However, I'm not really comfortable with this solution. Initially, I >> forgot to wrap the -Q option to as in %{gstabs*:...}, which lead to a >> bootstrap failure: the gas- and LLVM-based assemblers differ in a >> number of other ways, as can be seen when comparing gcc/auto-host.h: > > FAOD, > the changes below only occur if you omit the guard on “-Q” ? > or they are present always? they are from previous builds, one with the LLVM-based /usr/bin/as, the other configure with --with-as=/vol/gcc/bin/as-6.4 (gas-based as from Xcode 6.4). >> Given this, I'd rather have us not support stabs at all than via this >> half-hearted approach. > > I agree, we should just disable it if the “default” assembler doesn’t > support - so most of your patch would be unchanged. > The actual practical outcome is really not terribly severe, it ony affects > folks trying to target x86-darwin8 ( the last OS to use stabs as the first > choice), and even those systems support some debugging with DWARF. >> >> What do you think? > > My view is it should still be a configure test, rather than a blanket > disable for d15+ (simply because I’ve got back-ports of the newer tools > that should be useable across the Darwin range, and hopefully those > back-ports will be published before Xmas). The same issues would generally > apply there. Fine with me, it's certainly the safest approach. I'll update the patch accordingly. Rainer
Hi Rainer, > On 14 Dec 2015, at 11:13, Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> wrote: > >>> However, I'm not really comfortable with this solution. Initially, I >>> forgot to wrap the -Q option to as in %{gstabs*:...}, which lead to a >>> bootstrap failure: the gas- and LLVM-based assemblers differ in a >>> number of other ways, as can be seen when comparing gcc/auto-host.h: >> >> FAOD, >> the changes below only occur if you omit the guard on “-Q” ? >> or they are present always? > > they are from previous builds, one with the LLVM-based /usr/bin/as, the > other configure with --with-as=/vol/gcc/bin/as-6.4 (gas-based as from > Xcode 6.4). Hrm, this needs more investigation, and will affect 10.10 too, since xc7 is the default there. (separate issue, let’s start a new PR, or at least a new thread). Iain
Hi Iain, >> On 14 Dec 2015, at 11:13, Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> wrote: >> >>>> However, I'm not really comfortable with this solution. Initially, I >>>> forgot to wrap the -Q option to as in %{gstabs*:...}, which lead to a >>>> bootstrap failure: the gas- and LLVM-based assemblers differ in a >>>> number of other ways, as can be seen when comparing gcc/auto-host.h: >>> >>> FAOD, >>> the changes below only occur if you omit the guard on “-Q” ? >>> or they are present always? >> >> they are from previous builds, one with the LLVM-based /usr/bin/as, the >> other configure with --with-as=/vol/gcc/bin/as-6.4 (gas-based as from >> Xcode 6.4). > > Hrm, this needs more investigation, and will affect 10.10 too, since xc7 is > the default there. > (separate issue, let’s start a new PR, or at least a new thread). right, but it's only an issue if you switch assemblers (or linkers) used by gcc without rebuilding. This has never been safe on any platform. Rainer
> On 14 Dec 2015, at 11:21, Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> wrote: > > Hi Iain, > >>> On 14 Dec 2015, at 11:13, Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> wrote: >>> >>>>> However, I'm not really comfortable with this solution. Initially, I >>>>> forgot to wrap the -Q option to as in %{gstabs*:...}, which lead to a >>>>> bootstrap failure: the gas- and LLVM-based assemblers differ in a >>>>> number of other ways, as can be seen when comparing gcc/auto-host.h: >>>> >>>> FAOD, >>>> the changes below only occur if you omit the guard on “-Q” ? >>>> or they are present always? >>> >>> they are from previous builds, one with the LLVM-based /usr/bin/as, the >>> other configure with --with-as=/vol/gcc/bin/as-6.4 (gas-based as from >>> Xcode 6.4). >> >> Hrm, this needs more investigation, and will affect 10.10 too, since xc7 is >> the default there. >> (separate issue, let’s start a new PR, or at least a new thread). > > right, but it's only an issue if you switch assemblers (or linkers) used > by gcc without rebuilding. This has never been safe on any platform. The issue that worries me is that the new assembler supports .cfi_xxx (YAY!), but the Darwin port is not 100% ready for it yet (BOO!) (I have patches, and expect to make them available for folks to try in the next ~ 2 weeks). However, still not sure that they would exactly be stage3 stuff. Did you say that bootstrap fails if -Q is jammed in everywhere? (that would be a short-term safety net, falling back to the cctools assembler). Iain
Hi Iain, >>> Hrm, this needs more investigation, and will affect 10.10 too, since xc7 is >>> the default there. >>> (separate issue, let’s start a new PR, or at least a new thread). >> >> right, but it's only an issue if you switch assemblers (or linkers) used >> by gcc without rebuilding. This has never been safe on any platform. > > The issue that worries me is that the new assembler supports .cfi_xxx > (YAY!), but the Darwin port is not 100% ready for it yet (BOO!) (I have > patches, and expect to make them available for folks to try in the next ~ 2 > weeks). However, still not sure that they would exactly be stage3 stuff. ah, I see. The platform maintainers have some leeway even in stage 3, so let's see when you post the patches. > Did you say that bootstrap fails if -Q is jammed in everywhere? > (that would be a short-term safety net, falling back to the cctools assembler). Yes, but only if you run the configure test with the LLVM assembler, but always call as -Q during the build. It should be possible to perform the assembler tests with as -Q instead, so there shouldn't be an inconsistency and you get the same result as if configuring with a copy of the Xcode 6.4 as. Rainer
On Dec 14, 2015, at 2:40 AM, Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> wrote: > As described in PR PR target/67973, newer assemblers on Mac OS X, which > are based on LLVM instead of gas, don't support .stab* directives any > longer. The following patch detects this situation and tries to fall > back to the older gas-based as if it is still accessible via as -Q. > > Tested on x86_64-apple-darwin15.2.0 and as expected the -gstabs* tests > now pass. > > However, I'm not really comfortable with this solution. When I proposed automagically adding -Q, it sounded like a good idea. :-( Yeah, hard to disagree with your intuition. If a future assembler had or added stabs that had or added all these features, it would come first on the path, and it all work just work out nicely with just a configure check to disable stabs if it didn’t work. That simple check should be reliable and work well. > Initially, I > forgot to wrap the -Q option to as in %{gstabs*:...}, which lead to a > bootstrap failure: the gas- and LLVM-based assemblers differ in a > number of other ways Yeah, having the feature set be a dynamic property when our software decides on static basis is bound to hurt. Seem that the most likely patch would be to just turn off stabs in a way that the test suite disables the tests by itself, or to just quite the tests suite.
Mike Stump <mikestump@comcast.net> writes: > On Dec 14, 2015, at 2:40 AM, Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> wrote: >> As described in PR PR target/67973, newer assemblers on Mac OS X, which >> are based on LLVM instead of gas, don't support .stab* directives any >> longer. The following patch detects this situation and tries to fall >> back to the older gas-based as if it is still accessible via as -Q. >> >> Tested on x86_64-apple-darwin15.2.0 and as expected the -gstabs* tests >> now pass. >> >> However, I'm not really comfortable with this solution. > > When I proposed automagically adding -Q, it sounded like a good idea. :-( > > Yeah, hard to disagree with your intuition. If a future assembler had or > added stabs that had or added all these features, it would come first on > the path, and it all work just work out nicely with just a configure check > to disable stabs if it didn’t work. That simple check should be reliable > and work well. Right: I'm effectively keeping just the first configure test for .stabs support in the assembler to enable or disable DBX_DEBUG/DBX_DEBUGGING_INFO. I'll post it later since ... >> Initially, I >> forgot to wrap the -Q option to as in %{gstabs*:...}, which lead to a >> bootstrap failure: the gas- and LLVM-based assemblers differ in a >> number of other ways > > Yeah, having the feature set be a dynamic property when our software > decides on static basis is bound to hurt. Seem that the most likely patch > would be to just turn off stabs in a way that the test suite disables the > tests by itself, or to just quite the tests suite. ... testing revealed another instance of static assumptions which hurts us now: while support for -gstabs* is checked for dynamically in lib/gcc-dg.exp and lib/gfortran-dg.exp for the debug.exp tests, there are a couple of testcases that use -gstabs* unconditionally, but have a hardcoded list of targets that support those options. I'll introduce a new effective-target keyword (simply checking if -gstabs is accepted should be enough) to also perform this test dynamically and repost once it's tested. Rainer
On Dec 15, 2015, at 5:35 AM, Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> wrote: > Right: I'm effectively keeping just the first configure test for .stabs > support in the assembler to enable or disable > DBX_DEBUG/DBX_DEBUGGING_INFO. I'll post it later since … > ... testing revealed another instance of static assumptions which hurts > us now: while support for -gstabs* is checked for dynamically in > lib/gcc-dg.exp and lib/gfortran-dg.exp for the debug.exp tests, there > are a couple of testcases that use -gstabs* unconditionally, but have a > hardcoded list of targets that support those options. I'll introduce a > new effective-target keyword (simply checking if -gstabs is accepted > should be enough) to also perform this test dynamically and repost once > it's tested. Sounds good.
# HG changeset patch # Parent 7029fd86ac40d7ff34e1c43d729c6bf469416643 Only support -gstabs on Mac OS X if assember supports it (PR target/67973) diff --git a/gcc/config/darwin.h b/gcc/config/darwin.h --- a/gcc/config/darwin.h +++ b/gcc/config/darwin.h @@ -400,12 +400,13 @@ extern GTY(()) int darwin_ms_struct; #define ASM_DEBUG_SPEC "%{g*:%{!g0:%{!gdwarf*:--gstabs}}}" -/* We still allow output of STABS. */ - +/* We still allow output of STABS if the assembler supports it. */ +#ifdef HAVE_AS_STABS_DIRECTIVE #define DBX_DEBUGGING_INFO 1 +#define PREFERRED_DEBUGGING_TYPE DBX_DEBUG +#endif #define DWARF2_DEBUGGING_INFO 1 -#define PREFERRED_DEBUGGING_TYPE DBX_DEBUG #define DEBUG_FRAME_SECTION "__DWARF,__debug_frame,regular,debug" #define DEBUG_INFO_SECTION "__DWARF,__debug_info,regular,debug" diff --git a/gcc/config/i386/darwin.h b/gcc/config/i386/darwin.h --- a/gcc/config/i386/darwin.h +++ b/gcc/config/i386/darwin.h @@ -111,9 +111,16 @@ extern int darwin_emit_branch_islands; %{g: %{!fno-eliminate-unused-debug-symbols: -feliminate-unused-debug-symbols }} " \ DARWIN_CC1_SPEC +/* Pass -Q to assembler if necessary for stabs support. */ +#ifdef HAVE_AS_DARWIN_STABS_Q +#define ASM_STABS_Q_SPEC " %{gstabs*:-Q}" +#else +#define ASM_STABS_Q_SPEC "" +#endif + #undef ASM_SPEC #define ASM_SPEC "-arch %(darwin_arch) -force_cpusubtype_ALL \ - %{static}" + %{static}" ASM_STABS_Q_SPEC #define DARWIN_ARCH_SPEC "%{m64:x86_64;:i386}" #define DARWIN_SUBARCH_SPEC DARWIN_ARCH_SPEC @@ -226,7 +233,11 @@ do { \ compiles default to stabs+. darwin9+ defaults to dwarf-2. */ #ifndef DARWIN_PREFER_DWARF #undef PREFERRED_DEBUGGING_TYPE +#ifdef HAVE_AS_STABS_DIRECTIVE #define PREFERRED_DEBUGGING_TYPE (TARGET_64BIT ? DWARF2_DEBUG : DBX_DEBUG) +#else +#define PREFERRED_DEBUGGING_TYPE DWARF2_DEBUG +#endif #endif /* Darwin uses the standard DWARF register numbers but the default diff --git a/gcc/configure.ac b/gcc/configure.ac --- a/gcc/configure.ac +++ b/gcc/configure.ac @@ -2909,6 +2909,11 @@ AC_DEFINE_UNQUOTED(HAVE_GAS_SHF_MERGE, [`if test $gcc_cv_as_shf_merge = yes; then echo 1; else echo 0; fi`], [Define 0/1 if your assembler supports marking sections with SHF_MERGE flag.]) +gcc_GAS_CHECK_FEATURE([stabs directive], gcc_cv_as_stabs_directive, ,, +[.stabs "gcc2_compiled.",60,0,0,0],, +[AC_DEFINE(HAVE_AS_STABS_DIRECTIVE, 1, + [Define if your assembler supports .stabs.])]) + gcc_GAS_CHECK_FEATURE([COMDAT group support (GNU as)], gcc_cv_as_comdat_group, [elf,2,16,0], [--fatal-warnings], @@ -3619,6 +3624,26 @@ AC_DEFINE_UNQUOTED(HAVE_LTO_PLUGIN, $gcc [Define to the level of your linker's plugin support.]) AC_MSG_RESULT($gcc_cv_lto_plugin) +# Target OS-specific assembler checks. + +case "$target_os" in + darwin*) + # If the default assembler doesn't support .stabs, check if the + # gas-based one does. + if test x$gcc_cv_as_stabs_directive = xno; then + gcc_GAS_CHECK_FEATURE([.stabs directive via -Q], + gcc_cv_as_darwin_stabs_Q,, [-Q], + [.stabs "gcc2_compiled.",60,0,0,0],, + [AC_DEFINE(HAVE_AS_STABS_DIRECTIVE, 1, + [Define if your assembler supports .stabs.]) + AC_DEFINE(HAVE_AS_DARWIN_STABS_Q, 1, + [Define if your assembler needs -Q to support .stabs.])]) + fi + ;; +esac + +# Target CPU-specific assembler checks. + case "$target" in aarch64*-*-*) gcc_GAS_CHECK_FEATURE([-mabi option], gcc_cv_as_aarch64_mabi,,