| Message ID | 5b977b30da8ccde13e8e2632584009e3b5d94202.1522920973.git.segher@kernel.crashing.org |
|---|---|
| State | New |
| Headers | show |
| Series | rs6000: Enable -fasynchronous-unwind-tables by default | expand |
On Thu, Apr 05, 2018 at 09:45:54AM +0000, Segher Boessenkool wrote: > To find out where on-entry register values live at any point in a > program, GDB currently tries to parse to parse the executable code. > This does not work very well, for example it gets confused if some > accesses to the stack use the frame pointer (r31) and some use the > stack pointer (r1). A symptom is that backtraces can be cut short. > > This patch enables -fasynchronous-unwind-tables by default for rs6000, > which causes us to emit DWARF unwind tables for all functions, solving > these problems. > > This not do anything for sub-targets without DWARF. > > It increases executable size, but only modestly, and does not change > memory use, only the disk image. > > Various other targets already do this (x86, s390, tile*). aarch64-linux* too (since r258871). > Tested on powerpc64-linux {-m32,-m64}. David, I'd like to commit this > to current trunk; does that seem too dangerous to you? If David is ok with it, it is fine for trunk even in stage4. Jakub
On Thu, Apr 05, 2018 at 11:50:38AM +0200, Jakub Jelinek wrote: > On Thu, Apr 05, 2018 at 09:45:54AM +0000, Segher Boessenkool wrote: > > To find out where on-entry register values live at any point in a > > program, GDB currently tries to parse to parse the executable code. > > This does not work very well, for example it gets confused if some > > accesses to the stack use the frame pointer (r31) and some use the > > stack pointer (r1). A symptom is that backtraces can be cut short. > > > > This patch enables -fasynchronous-unwind-tables by default for rs6000, > > which causes us to emit DWARF unwind tables for all functions, solving > > these problems. > > > > This not do anything for sub-targets without DWARF. > > > > It increases executable size, but only modestly, and does not change > > memory use, only the disk image. > > > > Various other targets already do this (x86, s390, tile*). > > aarch64-linux* too (since r258871). Ah yes, I forgot. Aarch does it in its struct default_options; is that preferred? I did it in rs6000_option_init_struct because originally I avoided enabling it on some non-DWARF platforms, but the flag is ignored elsewhere anyway, and various other things set the flags for everything, too, so now I enable it always. > > Tested on powerpc64-linux {-m32,-m64}. David, I'd like to commit this > > to current trunk; does that seem too dangerous to you? > > If David is ok with it, it is fine for trunk even in stage4. Thanks. Segher
On Thu, Apr 05, 2018 at 05:08:36AM -0500, Segher Boessenkool wrote: > On Thu, Apr 05, 2018 at 11:50:38AM +0200, Jakub Jelinek wrote: > > On Thu, Apr 05, 2018 at 09:45:54AM +0000, Segher Boessenkool wrote: > > > To find out where on-entry register values live at any point in a > > > program, GDB currently tries to parse to parse the executable code. > > > This does not work very well, for example it gets confused if some > > > accesses to the stack use the frame pointer (r31) and some use the > > > stack pointer (r1). A symptom is that backtraces can be cut short. > > > > > > This patch enables -fasynchronous-unwind-tables by default for rs6000, > > > which causes us to emit DWARF unwind tables for all functions, solving > > > these problems. > > > > > > This not do anything for sub-targets without DWARF. > > > > > > It increases executable size, but only modestly, and does not change > > > memory use, only the disk image. > > > > > > Various other targets already do this (x86, s390, tile*). > > > > aarch64-linux* too (since r258871). > > Ah yes, I forgot. Aarch does it in its struct default_options; is that > preferred? I did it in rs6000_option_init_struct because originally I > avoided enabling it on some non-DWARF platforms, but the flag is ignored > elsewhere anyway, and various other things set the flags for everything, > too, so now I enable it always. I think either is fine, just note that both x86_64 -m64 and aarch64 turn -funwind-tables on too by default, aarch64 through the default_options, x86 through setting f_a_u_t to 2 and then: if (opts->x_flag_asynchronous_unwind_tables == 2) opts->x_flag_unwind_tables = opts->x_flag_asynchronous_unwind_tables = 1; in ix86_option_override_internal. Note i?86 -m32 doesn't make -funwind-tables the default on the other side, not sure if it really makes a difference. toplev.c process_options has: if (flag_non_call_exceptions) flag_asynchronous_unwind_tables = 1; if (flag_asynchronous_unwind_tables) flag_unwind_tables = 1; too though. Jakub
On Thu, Apr 5, 2018 at 5:50 AM, Jakub Jelinek <jakub@redhat.com> wrote: > On Thu, Apr 05, 2018 at 09:45:54AM +0000, Segher Boessenkool wrote: >> To find out where on-entry register values live at any point in a >> program, GDB currently tries to parse to parse the executable code. >> This does not work very well, for example it gets confused if some >> accesses to the stack use the frame pointer (r31) and some use the >> stack pointer (r1). A symptom is that backtraces can be cut short. >> >> This patch enables -fasynchronous-unwind-tables by default for rs6000, >> which causes us to emit DWARF unwind tables for all functions, solving >> these problems. >> >> This not do anything for sub-targets without DWARF. >> >> It increases executable size, but only modestly, and does not change >> memory use, only the disk image. >> >> Various other targets already do this (x86, s390, tile*). > > aarch64-linux* too (since r258871). > >> Tested on powerpc64-linux {-m32,-m64}. David, I'd like to commit this >> to current trunk; does that seem too dangerous to you? > > If David is ok with it, it is fine for trunk even in stage4. I don't have a fundamental objection. AIX does use DWARF EH. I'm not worried about the EH processing itself, but this may trigger paths in dwarf2out, etc. that aren't prepared for AIX XCOFF. I'm concerned that it may assume ELF section or insert additional section names or generally output code that is incompatible with the AIX assembler or linker. Also, AIX XCOFF does not have a special section for DWARF EH information, so it is stuffed in data section and will increase the memory image size. We need to test it on AIX and see what happens. - David
Hi! On Thu, Apr 05, 2018 at 01:07:21PM -0400, David Edelsohn wrote: > I don't have a fundamental objection. AIX does use DWARF EH. I'm not > worried about the EH processing itself, but this may trigger paths in > dwarf2out, etc. that aren't prepared for AIX XCOFF. I'm concerned that > it may assume ELF section or insert additional section names or > generally output code that is incompatible with the AIX assembler or > linker. > > Also, AIX XCOFF does not have a special section for DWARF EH > information, so it is stuffed in data section and will increase the > memory image size. > > We need to test it on AIX and see what happens. I tested it and it did not go well: there are bootstrap comparison errors. I tried to fix it and failed miserably. I'm committing the following, only enabling this for ELF targets for now, until we have time to sort it out. Tested on powerpc64-linux and on powerpc-ibm-aix7.2.0.0 . Segher --- From: Segher Boessenkool <segher@kernel.crashing.org> Date: Thu, 5 Apr 2018 09:27:31 +0000 Subject: [PATCH] rs6000: Enable -fasynchronous-unwind-tables by default To find out where on-entry register values live at any point in a program, GDB currently tries to parse to parse the executable code. This does not work very well, for example it gets confused if some accesses to the stack use the frame pointer (r31) and some use the stack pointer (r1). A symptom is that backtraces can be cut short. This patch enables -fasynchronous-unwind-tables by default for rs6000, which causes us to emit DWARF unwind tables for all functions, solving these problems. This not do anything for sub-targets without DWARF, and only for ELF sub-targets for now. It increases executable size, but only modestly, and does not change memory use, only the disk image. 2018-04-10 Segher Boessenkool <segher@kernel.crashing.org> * common/config/rs6000/rs6000-common.c (rs6000_option_init_struct): Enable -fasynchronous-unwind-tables by default if OBJECT_FORMAT_ELF. gcc/testsuite/ * gcc.target/powerpc/dfmode_off.c: Add -fno-asynchronous-unwind-tables. * gcc.target/powerpc/dimode_off.c: Ditto. * gcc.target/powerpc/tfmode_off.c: Ditto. * gcc.target/powerpc/timode_off.c: Ditto. --- gcc/common/config/rs6000/rs6000-common.c | 9 +++++++++ gcc/testsuite/gcc.target/powerpc/dfmode_off.c | 2 +- gcc/testsuite/gcc.target/powerpc/dimode_off.c | 2 +- gcc/testsuite/gcc.target/powerpc/tfmode_off.c | 2 +- gcc/testsuite/gcc.target/powerpc/timode_off.c | 2 +- 5 files changed, 13 insertions(+), 4 deletions(-) diff --git a/gcc/common/config/rs6000/rs6000-common.c b/gcc/common/config/rs6000/rs6000-common.c index 0ddadfd..ed348f5 100644 --- a/gcc/common/config/rs6000/rs6000-common.c +++ b/gcc/common/config/rs6000/rs6000-common.c @@ -49,6 +49,15 @@ rs6000_option_init_struct (struct gcc_options *opts) /* Enable section anchors by default. */ if (!TARGET_MACHO) opts->x_flag_section_anchors = 1; + + /* By default, always emit DWARF-2 unwind info. This allows debugging + without maintaining a stack frame back-chain. It also allows the + debugger to find out where on-entry register values are stored at any + point in a function, without having to analyze the executable code (which + isn't even possible to do in the general case). */ +#ifdef OBJECT_FORMAT_ELF + opts->x_flag_asynchronous_unwind_tables = 1; +#endif } /* Implement TARGET_OPTION_DEFAULT_PARAMS. */ diff --git a/gcc/testsuite/gcc.target/powerpc/dfmode_off.c b/gcc/testsuite/gcc.target/powerpc/dfmode_off.c index 1942f48..b5940cb 100644 --- a/gcc/testsuite/gcc.target/powerpc/dfmode_off.c +++ b/gcc/testsuite/gcc.target/powerpc/dfmode_off.c @@ -1,5 +1,5 @@ /* { dg-do assemble } */ -/* { dg-options "-O2 -fno-align-functions -mtraceback=no -save-temps" } */ +/* { dg-options "-O2 -fno-align-functions -fno-asynchronous-unwind-tables -mtraceback=no -save-temps" } */ void w1 (void *x, double y) { *(double *) (x + 32767) = y; } void w2 (void *x, double y) { *(double *) (x + 32766) = y; } diff --git a/gcc/testsuite/gcc.target/powerpc/dimode_off.c b/gcc/testsuite/gcc.target/powerpc/dimode_off.c index 77a1863..19ca40c 100644 --- a/gcc/testsuite/gcc.target/powerpc/dimode_off.c +++ b/gcc/testsuite/gcc.target/powerpc/dimode_off.c @@ -1,5 +1,5 @@ /* { dg-do assemble } */ -/* { dg-options "-O2 -fno-align-functions -mtraceback=no -save-temps" } */ +/* { dg-options "-O2 -fno-align-functions -fno-asynchronous-unwind-tables -mtraceback=no -save-temps" } */ void w1 (void *x, long long y) { *(long long *) (x + 32767) = y; } void w2 (void *x, long long y) { *(long long *) (x + 32766) = y; } diff --git a/gcc/testsuite/gcc.target/powerpc/tfmode_off.c b/gcc/testsuite/gcc.target/powerpc/tfmode_off.c index cbb3d75..f19e759 100644 --- a/gcc/testsuite/gcc.target/powerpc/tfmode_off.c +++ b/gcc/testsuite/gcc.target/powerpc/tfmode_off.c @@ -2,7 +2,7 @@ /* { dg-skip-if "" { powerpc-ibm-aix* } } */ /* { dg-skip-if "no TFmode" { powerpc-*-eabi* } } */ /* { dg-require-effective-target longdouble128 } */ -/* { dg-options "-O2 -fno-align-functions -mtraceback=no -save-temps" } */ +/* { dg-options "-O2 -fno-align-functions -fno-asynchronous-unwind-tables -mtraceback=no -save-temps" } */ typedef float TFmode __attribute__ ((mode (TF))); diff --git a/gcc/testsuite/gcc.target/powerpc/timode_off.c b/gcc/testsuite/gcc.target/powerpc/timode_off.c index efeffa7..b635953 100644 --- a/gcc/testsuite/gcc.target/powerpc/timode_off.c +++ b/gcc/testsuite/gcc.target/powerpc/timode_off.c @@ -1,6 +1,6 @@ /* { dg-do assemble { target { lp64 } } } */ /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power5" } } */ -/* { dg-options "-O2 -fno-align-functions -mtraceback=no -save-temps -mcpu=power5" } */ +/* { dg-options "-O2 -fno-align-functions -fno-asynchronous-unwind-tables -mtraceback=no -save-temps -mcpu=power5" } */ typedef int TImode __attribute__ ((mode (TI)));
diff --git a/gcc/common/config/rs6000/rs6000-common.c b/gcc/common/config/rs6000/rs6000-common.c index 0ddadfd..e45ed5e 100644 --- a/gcc/common/config/rs6000/rs6000-common.c +++ b/gcc/common/config/rs6000/rs6000-common.c @@ -49,6 +49,13 @@ rs6000_option_init_struct (struct gcc_options *opts) /* Enable section anchors by default. */ if (!TARGET_MACHO) opts->x_flag_section_anchors = 1; + + /* By default, always emit DWARF-2 unwind info. This allows debugging + without maintaining a stack frame back-chain. It also allows the + debugger to find out where on-entry register values are stored at any + point in a function, without having to analyze the executable code (which + isn't even possible to do in the general case). */ + opts->x_flag_asynchronous_unwind_tables = 1; } /* Implement TARGET_OPTION_DEFAULT_PARAMS. */ diff --git a/gcc/testsuite/gcc.target/powerpc/dfmode_off.c b/gcc/testsuite/gcc.target/powerpc/dfmode_off.c index 1942f48..b5940cb 100644 --- a/gcc/testsuite/gcc.target/powerpc/dfmode_off.c +++ b/gcc/testsuite/gcc.target/powerpc/dfmode_off.c @@ -1,5 +1,5 @@ /* { dg-do assemble } */ -/* { dg-options "-O2 -fno-align-functions -mtraceback=no -save-temps" } */ +/* { dg-options "-O2 -fno-align-functions -fno-asynchronous-unwind-tables -mtraceback=no -save-temps" } */ void w1 (void *x, double y) { *(double *) (x + 32767) = y; } void w2 (void *x, double y) { *(double *) (x + 32766) = y; } diff --git a/gcc/testsuite/gcc.target/powerpc/dimode_off.c b/gcc/testsuite/gcc.target/powerpc/dimode_off.c index 77a1863..19ca40c 100644 --- a/gcc/testsuite/gcc.target/powerpc/dimode_off.c +++ b/gcc/testsuite/gcc.target/powerpc/dimode_off.c @@ -1,5 +1,5 @@ /* { dg-do assemble } */ -/* { dg-options "-O2 -fno-align-functions -mtraceback=no -save-temps" } */ +/* { dg-options "-O2 -fno-align-functions -fno-asynchronous-unwind-tables -mtraceback=no -save-temps" } */ void w1 (void *x, long long y) { *(long long *) (x + 32767) = y; } void w2 (void *x, long long y) { *(long long *) (x + 32766) = y; } diff --git a/gcc/testsuite/gcc.target/powerpc/tfmode_off.c b/gcc/testsuite/gcc.target/powerpc/tfmode_off.c index cbb3d75..f19e759 100644 --- a/gcc/testsuite/gcc.target/powerpc/tfmode_off.c +++ b/gcc/testsuite/gcc.target/powerpc/tfmode_off.c @@ -2,7 +2,7 @@ /* { dg-skip-if "" { powerpc-ibm-aix* } } */ /* { dg-skip-if "no TFmode" { powerpc-*-eabi* } } */ /* { dg-require-effective-target longdouble128 } */ -/* { dg-options "-O2 -fno-align-functions -mtraceback=no -save-temps" } */ +/* { dg-options "-O2 -fno-align-functions -fno-asynchronous-unwind-tables -mtraceback=no -save-temps" } */ typedef float TFmode __attribute__ ((mode (TF))); diff --git a/gcc/testsuite/gcc.target/powerpc/timode_off.c b/gcc/testsuite/gcc.target/powerpc/timode_off.c index efeffa7..b635953 100644 --- a/gcc/testsuite/gcc.target/powerpc/timode_off.c +++ b/gcc/testsuite/gcc.target/powerpc/timode_off.c @@ -1,6 +1,6 @@ /* { dg-do assemble { target { lp64 } } } */ /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power5" } } */ -/* { dg-options "-O2 -fno-align-functions -mtraceback=no -save-temps -mcpu=power5" } */ +/* { dg-options "-O2 -fno-align-functions -fno-asynchronous-unwind-tables -mtraceback=no -save-temps -mcpu=power5" } */ typedef int TImode __attribute__ ((mode (TI)));
To find out where on-entry register values live at any point in a program, GDB currently tries to parse to parse the executable code. This does not work very well, for example it gets confused if some accesses to the stack use the frame pointer (r31) and some use the stack pointer (r1). A symptom is that backtraces can be cut short. This patch enables -fasynchronous-unwind-tables by default for rs6000, which causes us to emit DWARF unwind tables for all functions, solving these problems. This not do anything for sub-targets without DWARF. It increases executable size, but only modestly, and does not change memory use, only the disk image. Various other targets already do this (x86, s390, tile*). Tested on powerpc64-linux {-m32,-m64}. David, I'd like to commit this to current trunk; does that seem too dangerous to you? Segher 2018-04-05 Segher Boessenkool <segher@kernel.crashing.org> * common/config/rs6000/rs6000-common.c (rs6000_option_init_struct): Enable -fasynchronous-unwind-tables by default. gcc/testsuite/ * gcc.target/powerpc/dfmode_off.c: Add -fno-asynchronous-unwind-tables. * gcc.target/powerpc/dimode_off.c: Ditto. * gcc.target/powerpc/tfmode_off.c: Ditto. * gcc.target/powerpc/timode_off.c: Ditto. --- gcc/common/config/rs6000/rs6000-common.c | 7 +++++++ gcc/testsuite/gcc.target/powerpc/dfmode_off.c | 2 +- gcc/testsuite/gcc.target/powerpc/dimode_off.c | 2 +- gcc/testsuite/gcc.target/powerpc/tfmode_off.c | 2 +- gcc/testsuite/gcc.target/powerpc/timode_off.c | 2 +- 5 files changed, 11 insertions(+), 4 deletions(-)