Re: [PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.
I'm sending the updated patch based on Egeyar's work. It utilizes a new environmental variable and uses the currently existing -frecord-gcc-switches option. Thoughts? I am leaving it to the more experienced to comment on redefining the functionality of -frecord-gcc-switches. The code seems pretty neat to me. Thanks Martin! Best regards Egeyar
Re: [PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.
Hi. I'm sending the updated patch based on Egeyar's work. It utilizes a new environmental variable and uses the currently existing -frecord-gcc-switches option. Thoughts? Martin >From 9436d12e7a540691c6f2d6e2db4730a138e5c458 Mon Sep 17 00:00:00 2001 From: Martin Liska Date: Thu, 5 Mar 2020 09:39:17 +0100 Subject: [PATCH] Change semantics of -frecord-gcc-switches. gcc/ChangeLog: 2020-03-05 Martin Liska Egeyar Bagcioglu * common.opt: Make flag_record_gcc_switches also a driver option. * doc/tm.texi: Regenerate. * gcc.c (set_driver_command_line_envvar): New. (driver_handle_option): Handle OPT_frecord_gcc_switches and export command line into a ENV variable. (driver::main): Save command line. (driver::set_commandline): New. * gcc.h (set_commandline): New. * target.def: Simplify documentation by removal of unused enum values. * target.h (elf_record_gcc_switches): Change function type. * toplev.c (init_asm_output): Update call. * varasm.c (elf_record_gcc_switches): Record GCC version string and content of GCC_DRIVER_COMMAND_LINE. --- gcc/common.opt | 2 +- gcc/doc/tm.texi | 37 ++--- gcc/gcc.c | 41 + gcc/gcc.h | 1 + gcc/target.def | 37 ++--- gcc/target.h| 2 +- gcc/toplev.c| 11 +-- gcc/varasm.c| 46 ++ 8 files changed, 59 insertions(+), 118 deletions(-) diff --git a/gcc/common.opt b/gcc/common.opt index fa9da505fc2..60ca5a521c5 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -2246,7 +2246,7 @@ Common Joined RejectNegative Var(common_deferred_options) Defer ; records information in the assembler output file as comments, so ; they never reach the object file. frecord-gcc-switches -Common Report Var(flag_record_gcc_switches) +Common Driver Report Var(flag_record_gcc_switches) Record gcc command line switches in the object file. freg-struct-return diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi index 19985adac3e..30c71d60aff 100644 --- a/gcc/doc/tm.texi +++ b/gcc/doc/tm.texi @@ -8061,43 +8061,10 @@ need to override this if your target has special flags that might be set via @code{__attribute__}. @end deftypefn -@deftypefn {Target Hook} int TARGET_ASM_RECORD_GCC_SWITCHES (print_switch_type @var{type}, const char *@var{text}) +@deftypefn {Target Hook} void TARGET_ASM_RECORD_GCC_SWITCHES (void) Provides the target with the ability to record the gcc command line switches that have been passed to the compiler, and options that are -enabled. The @var{type} argument specifies what is being recorded. -It can take the following values: - -@table @gcctabopt -@item SWITCH_TYPE_PASSED -@var{text} is a command line switch that has been set by the user. - -@item SWITCH_TYPE_ENABLED -@var{text} is an option which has been enabled. This might be as a -direct result of a command line switch, or because it is enabled by -default or because it has been enabled as a side effect of a different -command line switch. For example, the @option{-O2} switch enables -various different individual optimization passes. - -@item SWITCH_TYPE_DESCRIPTIVE -@var{text} is either NULL or some descriptive text which should be -ignored. If @var{text} is NULL then it is being used to warn the -target hook that either recording is starting or ending. The first -time @var{type} is SWITCH_TYPE_DESCRIPTIVE and @var{text} is NULL, the -warning is for start up and the second time the warning is for -wind down. This feature is to allow the target hook to make any -necessary preparations before it starts to record switches and to -perform any necessary tidying up after it has finished recording -switches. - -@item SWITCH_TYPE_LINE_START -This option can be ignored by this target hook. - -@item SWITCH_TYPE_LINE_END -This option can be ignored by this target hook. -@end table - -The hook's return value must be zero. Other return values may be -supported in the future. +enabled. By default this hook is set to NULL, but an example implementation is provided for ELF based targets. Called @var{elf_record_gcc_switches}, diff --git a/gcc/gcc.c b/gcc/gcc.c index 9f790db0daf..673ba2935f9 100644 --- a/gcc/gcc.c +++ b/gcc/gcc.c @@ -235,6 +235,11 @@ static int verbose_only_flag; static int print_subprocess_help; +/* argc and argv used to call gcc. Necessary for + --record-gcc-command-line option. */ +static unsigned int driver_gcc_argc; +static const char **driver_gcc_argv; + /* Linker suffix passed to -fuse-ld=... */ static const char *use_ld; @@ -3724,6 +3729,28 @@ set_source_date_epoch_envvar () setenv ("SOURCE_DATE_EPOCH", source_date_epoch, 0); } +/* Set GCC_DRIVER_COMMAND_LINE enviromental variable that is later + used by -frecord-gcc-switches option. */ + +static void +set_driver_command_line_envvar () +{ + unsigned int length = 0; + for (unsigned int i = 0; i < driver_gcc_argc; i++) +length +=
Re: [PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.
On Wed, Mar 4, 2020 at 8:39 PM Egeyar Bagcioglu wrote: > > > > On 3/4/20 6:23 PM, Martin Liška wrote: > > On 3/4/20 4:25 PM, Egeyar Bagcioglu wrote: > >> Thanks Richard. > >> > >> I do not have write-access to the GCC repo. I'd be glad if someone > >> commits it for me. > > > > Can we please wait? I'm really convinced we do not want one another > > very similar > > functionality. > > I am sorry. I thought Richard's comment was only about the third patch, > which is also necessary for the current -frecord-gcc-switches. > Therefore, my reply was only for that patch as well. Yes, my comment / approval was only about preserving the early compile options for LTO. I'll commit that piece momentarily. Richard. > Regards > Egeyar > > > I would definitely recommend to change the semantics > > of -frecord-gcc-switches to what the patchset does. > > > > That's why I added Nick as he's the author of the original > > implementation. > > Reasoning is provided in my previous email. > > > > Martin >
Re: [PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.
On 3/4/20 6:33 PM, Jakub Jelinek wrote: On Wed, Mar 04, 2020 at 06:23:10PM +0100, Martin Liška wrote: On 3/4/20 4:25 PM, Egeyar Bagcioglu wrote: Thanks Richard. I do not have write-access to the GCC repo. I'd be glad if someone commits it for me. Can we please wait? I'm really convinced we do not want one another very similar functionality. I would definitely recommend to change the semantics of -frecord-gcc-switches to what the patchset does. That's why I added Nick as he's the author of the original implementation. Reasoning is provided in my previous email. Also, what is the reason for storing the option in a file, rather than say putting them into an environment variable from which the backend can pick it up? I needed to pass that information from one process to the other and I picked a way to do so. Moreover, passing files is what gcc does; therefore, it didn't seem unnatural to do so. Furthermore, gcc specs already has its way to create temporary file names so I didn't have to worry about what level of thread safety is acceptable. If you tell me why an environment variable is better, we can discuss and I might implement that too. I must say I don't really see advantages of this over -grecord-gcc-switches, recording all options looks very bloaty and will include mostly stuff you don't really care about (such as, e.g. the -I options without knowing what was the current directory when the source file has been compiled), on the other side will not record interesting options that -grecord-gcc-switches records (say, if some code is compiled with -march=native, this new option will record that, rather than what it really is), 1) -grecord-gcc-switches is quite similar to -frecord-gcc-switches and my first and main arguement about the latter is valid for the former: You cannot easily construct the right command line to invoke the same compilation from the switches. Switches are useful to check that the compiler is doing the expected. The command line is useful when you want the compiler to do the same thing again. 2) The output is exactly what gcc takes as command line to make that compilation. It is bloaty, surely, when so is what's provided to gcc to make it compile as desired. On the other hand, -grecord-gcc-switches is just much bloatier since it does not work alone. It's embedded in dwarf. 3) Many systems automatically strip dwarf, together with this information. I do not think there is an easy way to separate this information and keep it. Since this comes up often, I'd like to emphasize that the internal switches of gcc are different than the user-given options of gcc. The purpose of this new option is really to catch the user given options of gcc and only them, so much so that I mark it SWITCH_IGNORE within the specs to avoid it propagated to any child processes of gcc. This option is simply to retrieve how the call was made to GCC. Currently, there are no other options giving us this information. but I won't stand in a way unless such an option would be on by default. I totally agree that this should not be on by default. Best regards Egeyar
Re: [PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.
On 3/4/20 6:23 PM, Martin Liška wrote: On 3/4/20 4:25 PM, Egeyar Bagcioglu wrote: Thanks Richard. I do not have write-access to the GCC repo. I'd be glad if someone commits it for me. Can we please wait? I'm really convinced we do not want one another very similar functionality. I am sorry. I thought Richard's comment was only about the third patch, which is also necessary for the current -frecord-gcc-switches. Therefore, my reply was only for that patch as well. Regards Egeyar I would definitely recommend to change the semantics of -frecord-gcc-switches to what the patchset does. That's why I added Nick as he's the author of the original implementation. Reasoning is provided in my previous email. Martin
Re: [PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.
On Wed, Mar 04, 2020 at 06:42:29PM +0100, Martin Liška wrote: > > I must say I don't really see advantages of this over > > -grecord-gcc-switches, recording all options looks very bloaty and will > > include mostly stuff you don't really care about (such as, e.g. the -I > > options without knowing what was the current directory when the source file > > has been compiled), on the other side will not record interesting options > > that -grecord-gcc-switches records (say, if some code is compiled with > > -march=native, this new option will record that, rather than what it really > > is), but I won't stand in a way unless such an option would be on by > > default. > > Yes, it's a minor disadvantage. On the other hand one can check the fortify > macros. I don't care much about them too, but what's the biggest benefit to me > is that each argument will not go into it's own mergeable section. Then > you will not see something like: Well, the fortify macro is questionable, because as a macro, it can be either specified on the command line, or e.g. defined in the source before including headers, so -g3 seems much better way to query it. > The output is useless and can't disambiguate each compiler > invocations. Sure, I'm not talking about -frecord-gcc-switches, that option is indeed not really useful. Jakub
Re: [PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.
On 3/4/20 6:33 PM, Jakub Jelinek wrote: On Wed, Mar 04, 2020 at 06:23:10PM +0100, Martin Liška wrote: On 3/4/20 4:25 PM, Egeyar Bagcioglu wrote: Thanks Richard. I do not have write-access to the GCC repo. I'd be glad if someone commits it for me. Can we please wait? I'm really convinced we do not want one another very similar functionality. I would definitely recommend to change the semantics of -frecord-gcc-switches to what the patchset does. That's why I added Nick as he's the author of the original implementation. Reasoning is provided in my previous email. Also, what is the reason for storing the option in a file, rather than say putting them into an environment variable from which the backend can pick it up? That's smart suggestion! It would not be the first environment variable that we use, right? I must say I don't really see advantages of this over -grecord-gcc-switches, recording all options looks very bloaty and will include mostly stuff you don't really care about (such as, e.g. the -I options without knowing what was the current directory when the source file has been compiled), on the other side will not record interesting options that -grecord-gcc-switches records (say, if some code is compiled with -march=native, this new option will record that, rather than what it really is), but I won't stand in a way unless such an option would be on by default. Yes, it's a minor disadvantage. On the other hand one can check the fortify macros. I don't care much about them too, but what's the biggest benefit to me is that each argument will not go into it's own mergeable section. Then you will not see something like: $ gcc -O0 foo.c -frecord-gcc-switches -c -g $ gcc -Ofast bar.c -frecord-gcc-switches -c -g0 $ gcc foo.o bar.o $ readelf -p .GCC.command.line a.out String dump of section '.GCC.command.line': [ 0] foo.c [ 6] -mtune=generic [15] -march=x86-64 [23] -g [26] -O0 [2a] -frecord-gcc-switches [40] bar.c [46] -g0 [4a] -Ofast The output is useless and can't disambiguate each compiler invocations. Martin Jakub
Re: [PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.
On Wed, Mar 04, 2020 at 06:23:10PM +0100, Martin Liška wrote: > On 3/4/20 4:25 PM, Egeyar Bagcioglu wrote: > > Thanks Richard. > > > > I do not have write-access to the GCC repo. I'd be glad if someone commits > > it for me. > > Can we please wait? I'm really convinced we do not want one another very > similar > functionality. I would definitely recommend to change the semantics > of -frecord-gcc-switches to what the patchset does. > > That's why I added Nick as he's the author of the original implementation. > Reasoning is provided in my previous email. Also, what is the reason for storing the option in a file, rather than say putting them into an environment variable from which the backend can pick it up? I must say I don't really see advantages of this over -grecord-gcc-switches, recording all options looks very bloaty and will include mostly stuff you don't really care about (such as, e.g. the -I options without knowing what was the current directory when the source file has been compiled), on the other side will not record interesting options that -grecord-gcc-switches records (say, if some code is compiled with -march=native, this new option will record that, rather than what it really is), but I won't stand in a way unless such an option would be on by default. Jakub
Re: [PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.
On 3/4/20 4:25 PM, Egeyar Bagcioglu wrote: Thanks Richard. I do not have write-access to the GCC repo. I'd be glad if someone commits it for me. Can we please wait? I'm really convinced we do not want one another very similar functionality. I would definitely recommend to change the semantics of -frecord-gcc-switches to what the patchset does. That's why I added Nick as he's the author of the original implementation. Reasoning is provided in my previous email. Martin
Re: [PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.
On 3/4/20 4:34 PM, Andreas Schwab wrote: On Mär 04 2020, Richard Biener wrote: --record-gcc-command-line is not a FSF GCC option, there's -frecord-gcc-switches though which --record-gcc-command-line is translated to -frecord-gcc-switches by the driver. That happens for all double-dash options that match an -f option. I think there is a misunderstanding here. I have just implemented --record-gcc-command-line. I have been testing it and no it does not act as -frecord-gcc-switches. Regards Egeyar Andreas.
Re: [PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.
On Mär 04 2020, Richard Biener wrote: > --record-gcc-command-line is not a FSF GCC option, there's > -frecord-gcc-switches though which --record-gcc-command-line is translated to -frecord-gcc-switches by the driver. That happens for all double-dash options that match an -f option. Andreas. -- Andreas Schwab, SUSE Labs, sch...@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different."
Re: [PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.
On 3/4/20 10:00 AM, Richard Biener wrote: On Tue, Mar 3, 2020 at 5:41 PM Egeyar Bagcioglu wrote: This patch is for .GCC.command.line sections in LTO objects to be copied into the final objects as in the following example: [egeyar@localhost lto]$ gcc -flto -O3 demo.c -c -g --record-gcc-command-line [egeyar@localhost lto]$ gcc -flto -O2 demo2.c -c -g --record-gcc-command-line -DFORTIFY=2 [egeyar@localhost lto]$ gcc demo.o demo2.o -o a.out [egeyar@localhost lto]$ readelf -p .GCC.command.line a.out String dump of section '.GCC.command.line': [ 0] 10.0.1 20200227 (experimental) : gcc -flto -O3 demo.c -c -g --record-gcc-command-line [56] 10.0.1 20200227 (experimental) : gcc -flto -O2 demo2.c -c -g --record-gcc-command-line -DFORTIFY=2 --record-gcc-command-line is not a FSF GCC option, there's -frecord-gcc-switches though which (also) populates .GCC.command.line Right. This is also necessary for preserving the outcome of -frecord-gcc-switches option and that issue is reported to me by Martin Liska. OK. Thanks Richard. I do not have write-access to the GCC repo. I'd be glad if someone commits it for me. Regards Egeyar Thanks, Richard. Regards Egeyar libiberty: 2020-02-27 Egeyar Bagcioglu * simple-object.c (handle_lto_debug_sections): Name ".GCC.command.line" among debug sections to be copied over from lto objects. --- libiberty/simple-object.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libiberty/simple-object.c b/libiberty/simple-object.c index d9c648a..3b3ca9c 100644 --- a/libiberty/simple-object.c +++ b/libiberty/simple-object.c @@ -298,6 +298,9 @@ handle_lto_debug_sections (const char *name, int rename) COMDAT sections in objects produced by GCC. */ else if (strcmp (name, ".comment") == 0) return strcpy (newname, name); + /* Copy over .GCC.command.line section under the same name if present. */ + else if (strcmp (name, ".GCC.command.line") == 0) +return strcpy (newname, name); free (newname); return NULL; } -- 1.8.3.1
Re: [PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.
On Tue, Mar 3, 2020 at 5:41 PM Egeyar Bagcioglu wrote: > > This patch is for .GCC.command.line sections in LTO objects to be copied > into the final objects as in the following example: > > [egeyar@localhost lto]$ gcc -flto -O3 demo.c -c -g --record-gcc-command-line > [egeyar@localhost lto]$ gcc -flto -O2 demo2.c -c -g --record-gcc-command-line > -DFORTIFY=2 > [egeyar@localhost lto]$ gcc demo.o demo2.o -o a.out > [egeyar@localhost lto]$ readelf -p .GCC.command.line a.out > > String dump of section '.GCC.command.line': > [ 0] 10.0.1 20200227 (experimental) : gcc -flto -O3 demo.c -c -g > --record-gcc-command-line > [56] 10.0.1 20200227 (experimental) : gcc -flto -O2 demo2.c -c -g > --record-gcc-command-line -DFORTIFY=2 --record-gcc-command-line is not a FSF GCC option, there's -frecord-gcc-switches though which (also) populates .GCC.command.line OK. Thanks, Richard. > Regards > Egeyar > > libiberty: > 2020-02-27 Egeyar Bagcioglu > > * simple-object.c (handle_lto_debug_sections): Name > ".GCC.command.line" among debug sections to be copied over > from lto objects. > --- > libiberty/simple-object.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/libiberty/simple-object.c b/libiberty/simple-object.c > index d9c648a..3b3ca9c 100644 > --- a/libiberty/simple-object.c > +++ b/libiberty/simple-object.c > @@ -298,6 +298,9 @@ handle_lto_debug_sections (const char *name, int rename) > COMDAT sections in objects produced by GCC. */ >else if (strcmp (name, ".comment") == 0) > return strcpy (newname, name); > + /* Copy over .GCC.command.line section under the same name if present. */ > + else if (strcmp (name, ".GCC.command.line") == 0) > +return strcpy (newname, name); >free (newname); >return NULL; > } > -- > 1.8.3.1 >
[PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.
This patch is for .GCC.command.line sections in LTO objects to be copied into the final objects as in the following example: [egeyar@localhost lto]$ gcc -flto -O3 demo.c -c -g --record-gcc-command-line [egeyar@localhost lto]$ gcc -flto -O2 demo2.c -c -g --record-gcc-command-line -DFORTIFY=2 [egeyar@localhost lto]$ gcc demo.o demo2.o -o a.out [egeyar@localhost lto]$ readelf -p .GCC.command.line a.out String dump of section '.GCC.command.line': [ 0] 10.0.1 20200227 (experimental) : gcc -flto -O3 demo.c -c -g --record-gcc-command-line [56] 10.0.1 20200227 (experimental) : gcc -flto -O2 demo2.c -c -g --record-gcc-command-line -DFORTIFY=2 Regards Egeyar libiberty: 2020-02-27 Egeyar Bagcioglu * simple-object.c (handle_lto_debug_sections): Name ".GCC.command.line" among debug sections to be copied over from lto objects. --- libiberty/simple-object.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libiberty/simple-object.c b/libiberty/simple-object.c index d9c648a..3b3ca9c 100644 --- a/libiberty/simple-object.c +++ b/libiberty/simple-object.c @@ -298,6 +298,9 @@ handle_lto_debug_sections (const char *name, int rename) COMDAT sections in objects produced by GCC. */ else if (strcmp (name, ".comment") == 0) return strcpy (newname, name); + /* Copy over .GCC.command.line section under the same name if present. */ + else if (strcmp (name, ".GCC.command.line") == 0) +return strcpy (newname, name); free (newname); return NULL; } -- 1.8.3.1