| Message ID | b8f072f0-96de-1a10-638f-c1a1191aac05@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Dec 06, 2016 at 03:23:06PM -0600, Bill Seurer wrote: > 2016-12-06 Bill Seurer <seurer@linux.vnet.ibm.com> > > PR sanitizer/65479 > * gcc/config/rs6000/rs6000.c: Add -fasynchronous-unwind-tables option when > -fsanitize=address is specified. You should mention the function name here. > --- gcc/config/rs6000/rs6000.c (revision 243308) > +++ gcc/config/rs6000/rs6000.c (working copy) > @@ -5204,6 +5204,11 @@ rs6000_option_override (void) > { > (void) rs6000_option_override_internal (true); > > + /* -fsanitize=address needs to turn on -fasynchronous-unwind-tables for > + ppc64 in order for tracebacks to be complete. */ > + if (global_options.x_flag_sanitize & SANITIZE_USER_ADDRESS) > + global_options.x_flag_asynchronous_unwind_tables = 1; Do you need to check if the user specified -fno-asynchronous-unwind-tables here, and then not do this? I.e. similar to the rs6000_isa_flags_explicit handling (in the _internal function). Segher
On Tue, Dec 06, 2016 at 03:55:54PM -0600, Segher Boessenkool wrote: > On Tue, Dec 06, 2016 at 03:23:06PM -0600, Bill Seurer wrote: > > 2016-12-06 Bill Seurer <seurer@linux.vnet.ibm.com> > > > > PR sanitizer/65479 > > * gcc/config/rs6000/rs6000.c: Add -fasynchronous-unwind-tables option when > > -fsanitize=address is specified. > > You should mention the function name here. And not use the gcc/ prefix in the filename. > > --- gcc/config/rs6000/rs6000.c (revision 243308) > > +++ gcc/config/rs6000/rs6000.c (working copy) > > @@ -5204,6 +5204,11 @@ rs6000_option_override (void) > > { > > (void) rs6000_option_override_internal (true); > > > > + /* -fsanitize=address needs to turn on -fasynchronous-unwind-tables for > > + ppc64 in order for tracebacks to be complete. */ > > + if (global_options.x_flag_sanitize & SANITIZE_USER_ADDRESS) > > + global_options.x_flag_asynchronous_unwind_tables = 1; > > Do you need to check if the user specified -fno-asynchronous-unwind-tables > here, and then not do this? I.e. similar to the rs6000_isa_flags_explicit > handling (in the _internal function). Why are you changing it here and not in the *_internal function? Why do you use global_options.x_* instead of just *? And Segher has a point, it should be likely: if ((flag_sanitize & SANITIZE_USER_ADDRESS) && !global_options_set.x_flag_asynchronous_unwind_tables) flag_asynchronous_unwind_tables = 1; Jakub
On 12/6/16 3:55 PM, Segher Boessenkool wrote: > On Tue, Dec 06, 2016 at 03:23:06PM -0600, Bill Seurer wrote: >> --- gcc/config/rs6000/rs6000.c (revision 243308) >> +++ gcc/config/rs6000/rs6000.c (working copy) >> @@ -5204,6 +5204,11 @@ rs6000_option_override (void) >> { >> (void) rs6000_option_override_internal (true); >> >> + /* -fsanitize=address needs to turn on -fasynchronous-unwind-tables for >> + ppc64 in order for tracebacks to be complete. */ >> + if (global_options.x_flag_sanitize & SANITIZE_USER_ADDRESS) >> + global_options.x_flag_asynchronous_unwind_tables = 1; > > Do you need to check if the user specified -fno-asynchronous-unwind-tables > here, and then not do this? I.e. similar to the rs6000_isa_flags_explicit > handling (in the _internal function). I agree that if the user explicitly says -fno-asynchronous-unwind-tables, we shouldn't implicitly enable it behind their back. I also don't see how this is ppc64 specific, but your comment mentions it. The way the code is written here, this will be used for ppc32, ppc64 and ppc64le. I'd just rewrite the comment without mentioning ppc64 or replace it with a more general term (PowerPC?). Peter
Index: gcc/config/rs6000/rs6000.c =================================================================== --- gcc/config/rs6000/rs6000.c (revision 243308) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -5204,6 +5204,11 @@ rs6000_option_override (void) { (void) rs6000_option_override_internal (true); + /* -fsanitize=address needs to turn on -fasynchronous-unwind-tables for + ppc64 in order for tracebacks to be complete. */ + if (global_options.x_flag_sanitize & SANITIZE_USER_ADDRESS) + global_options.x_flag_asynchronous_unwind_tables = 1; + /* Register machine-specific passes. This needs to be done at start-up. It's convenient to do it here (like i386 does). */ opt_pass *pass_analyze_swaps = make_pass_analyze_swaps (g);