[PATCH 0/2] Introduce a new GCC option, --record-gcc-command-line

Mon Nov 18 19:30:00 GMT 2019

On 11/13/19 10:37 AM, Martin LiÅ¡ka wrote:
> On 11/7/19 3:50 PM, Egeyar Bagcioglu wrote:
>>
>> On 11/7/19 10:24 AM, Martin LiÅ¡ka wrote:
>>> On 11/6/19 6:21 PM, Egeyar Bagcioglu wrote:
>>>> Hello,
>>>
>>> Hello.
>>
>> Thanks for your detailed reply Martin. You'll find my reply inline. 
>> Since you added Nick Clifton to your following reply, I am adding him 
>> to this email too. He is not only the author of annobin, he also 
>> submitted the -frecord-gcc-switches to GCC. I agree that this 
>> discussion can benefit from his input.
>>
>>>
>>>> I would like to propose the following patches which introduce a 
>>>> compile option --record-gcc-command-line. When passed to gcc, it 
>>>> saves the command line option into the produced object file. The 
>>>> option makes it trivial to trace back how a file was compiled and 
>>>> by which version of the gcc. It helps with debugging, reproducing 
>>>> bugs and repeating the build process.
>>>
>>> I like your motivation, we as SUSE would like to have a similar 
>>> functionality. But the current approach has some limitations that 
>>> make it not usable (will explain later).
>>
>> I am glad you agree with the motivation. Let me answer below the 
>> other concerns that you have.
>>
>>>> This option is similar to -frecord-gcc-switches. However, they have 
>>>> three fundamental differences: Firstly, -frecord-gcc-switches saves 
>>>> the internal state after the argv is processed and passed by the 
>>>> driver. As opposed to that, --record-gcc-command-line saves the 
>>>
>>> I would not name it as a fundamental changes, it's doing very 
>>> similar to what -frecord-gcc-switches does.
>>
>> It is very similar; however, I still insist that what I outlined are 
>> fundamental differences. As I mentioned in my previous email, I built 
>> binutils as my test-case-project. I attach to this email the output 
>> of "readelf -p .GCC.command.line ld/ld-new", so that you can see how 
>> well the output is merged in general. Please take a look. It saves 
>> the command line *as is* and as one entry per invocation.
>
> Hello.
>
> Ok, works for me and I'm glad you also wrote a counterpart for 
> bintuils which can easily present the information to a user.

I am glad you liked the output. It is output by readelf without any 
additional patches.

>
>
>>
>> For the record, this is just to test and showcase the functionality. 
>> This patch in fact has nothing to do with binutils.
>>
>>> Moreover, we also have one another option -grecord-gcc-switches
>>> that saves command line into DWARF.
>>
>> As Nick also mentioned many times, -grecord-gcc-switches is in DWARF 
>> and this causes a great disadvantage: it gets stripped out.
>
> Well, that's still something I disagree. I bet RedHat is similarly to 
> openSUSE also building all packages with a debug info, which
> is later stripped and put into a foo-devel package. That's why one can 
> easily read the compile options from these sub-packages.
> My motivation is to write a rpm linter check that will verify that all 
> object files really used flags that we expect.

I understand your use case. However, some of the use cases we have for 
this patch are not for the distros but for the development. Having the 
compile options in the object files allows developers to pass around 
objects that are compiled differently without needing to tag them 
separately. This eases for example the performance analysis. A similar 
argument can also be made for reporting bugs.

>> I believe the -grecord-gcc-switches is moot for the sake of this 
>> discussion. Because I think the discussion surrounding the submission 
>> of -frecord-gcc-switches makes it clear that the necessity to keep 
>> the compile options in the object file is something that is already 
>> agreed on.
>>
>>> Plus there's a Red Hat plugin called Annobin:
>>> https://www.youtube.com/watch?v=uzffr1M-w5M
>>> https://developers.redhat.com/blog/2018/02/20/annobin-storing-information-binaries/ 
>>>
>>
>> I am aware of annobin, which is already released as a part of RHEL8. 
>> I think it is much superior to what I am aiming here. The sole 
>> purpose of this patch is to keep the command line options in the 
>> object file. I believe this is a basic functionality that should be 
>> supported by the GCC itself, without requiring a plugin.
>
> I fully aggree with you.
>
>> In other words, I think pushing a different build of a GCC plugin for 
>> each GCC version we use on each distro (i.e. versions-times-distros 
>> many plugin builds) is an overkill for such a basic need.
>
> Yep.
>
>>
>> Those who already use annobin for any of its many use cases, might of 
>> course prefer it over this functionality. For the rest of the distros 
>> and the GCC versions, I believe this patch is quite useful and 
>> extendable for its quite basic purpose.
>>
>>>
>>> Main limitation of current approach (and probably the suggested 
>>> patch) are:
>>> a) it does not print per function options, which can be modified 
>>> with __attribute__ (or pragma):
>>>
>>> $ cat demo.c
>>> int foo()
>>> {
>>> Â  return 1;
>>> }
>>>
>>> #pragma GCC optimize ("-O3")
>>>
>>> int bar()
>>> {
>>> Â  return 0;
>>> }
>>>
>>> int main()
>>> {
>>> Â  return 0;
>>> }
>>
>> I understand the need here. However, the purpose of this patch is 
>> only to save the command line options. Your example is a change in 
>> the source file. Of course, the source file can change. Even the 
>> implementation of the functions themselves might change. But I 
>> believe this is out of the scope of this patch, which is the command 
>> line.
>
> I can easily live with that.
>
>>
>>>
>>> b) we as SUSE are switching to LTO (-flto); doing that each LTO 
>>> LTRANS will become one compilation unit and
>>> one will see a misleading command line invocation:
>>>
>>> $ gcc -flto -O2 demo2.c -c
>>> $ gcc -flto -O3 demo.c -c
>>> $ gcc demo.o demo2.o -o a.out -frecord-gcc-switches
>>> ...
>>> Â Â Â Â .fileÂ Â Â  "<artificial>"
>>> Â Â Â Â .sectionÂ Â Â  .GCC.command.line,"MS",@progbits,1
>>> Â Â Â Â .asciiÂ Â Â  "-mtune=generic"
>>> Â Â Â Â .zeroÂ Â Â  1
>>> Â Â Â Â .asciiÂ Â Â  "-march=x86-64"
>>> Â Â Â Â .zeroÂ Â Â  1
>>> Â Â Â Â .asciiÂ Â Â  "-auxbase-strip a.out.ltrans0.ltrans.o"
>>> Â Â Â Â .zeroÂ Â Â  1
>>> Â Â Â Â .asciiÂ Â Â  "-O3"
>>> Â Â Â Â .zeroÂ Â Â  1
>>> Â Â Â Â .asciiÂ Â Â  "-fno-openmp"
>>> Â Â Â Â .zeroÂ Â Â  1
>>> Â Â Â Â .asciiÂ Â Â  "-fno-openacc"
>>> Â Â Â Â .zeroÂ Â Â  1
>>> Â Â Â Â .asciiÂ Â Â  "-fno-pie"
>>> Â Â Â Â .zeroÂ Â Â  1
>>> Â Â Â Â .asciiÂ Â Â  "-frecord-gcc-switches"
>>> Â Â Â Â .zeroÂ Â Â  1
>>> Â Â Â Â .asciiÂ Â Â  "-fltrans"
>>> Â Â Â Â .zeroÂ Â Â  1
>>> Â Â Â Â .asciiÂ Â Â  "a.out.ltrans0.o"
>>> Â Â Â Â .zeroÂ Â Â  1
>>> Â Â Â Â .text
>>> Â Â Â Â .typeÂ Â Â  foo, @function
>>
>> This is a very interesting case indeed. Thanks for bringing it to my 
>> attention. LTO seems to be discarding .GCC.command.line sections.
>>
>> [egeyar@localhost lto]$ gcc -flto -O3 demo.c -c 
>> --record-gcc-command-line
>> [egeyar@localhost lto]$ readelf -p .GCC.command.line demo.o
>> String dump of section '.GCC.command.line':
>> Â Â  [Â Â Â Â  0]Â  10.0.0 20191025 (experimental) : gcc -flto -O3 demo.c -c 
>> --record-gcc-command-line
>>
>> [egeyar@localhost lto]$ gcc -flto -O2 demo2.c -c 
>> --record-gcc-command-line
>> [egeyar@localhost lto]$ readelf -p .GCC.command.line demo2.o
>> String dump of section '.GCC.command.line':
>> Â Â  [Â Â Â Â  0]Â  10.0.0 20191025 (experimental) : gcc -flto -O2 demo2.c 
>> -c --record-gcc-command-line
>>
>> [egeyar@localhost lto]$ gcc demo.o demo2.o -o a.out
>> [egeyar@localhost lto]$ readelf -p .GCC.command.line a.out
>> readelf: a.out: Warning: Section '.GCC.command.line' was not dumped 
>> because it does not exist!
>>
>> [egeyar@localhost lto]$ gcc demo.o demo2.o -o a.out 
>> --record-gcc-command-line
>> [egeyar@localhost lto]$ readelf -p .GCC.command.line a.out
>> String dump of section '.GCC.command.line':
>> Â Â  [Â Â Â Â  0]Â  10.0.0 20191025 (experimental) : gcc @/tmp/ccSlWkxd
>>
>> Running this last command with "-v" reveals that gcc is called twice 
>> with temporary files. Only the .GCC.command.line of the second call 
>> survives.> I believe the beauty of this implementation is that driver 
>> saves the options *as it receives* in an intermediate file. It then 
>> passes this -or not- to the child processes. I am not familiar with 
>> the internals of LTO. However, thinking that those string sections 
>> can actually be merged and output, this implementation can also be 
>> extended for lto1 to handle the option correctly, and to suppress it 
>> for its calls to gcc.
>
> As already mentioned, the following feature is crucial for me. LTO 
> works in following steps:
>
> 1) you first generate LTO bytecode with -c -flto (so you have N input 
> files for which you want
> to report command lines)
> 2) Then you do so called WPA phase, you load all N files and stream 
> bytecode to M files (usually to 128)
> 3) Then, these M files emit proper .s file and you eventually end up 
> with an executable (or a shared library).
>
> That said, you have to stream the parsed command line through LTO and 
> eventually emit multiple records based
> for each incoming N files.

Before receiving this email, I checked the linker side of it, only to 
realize that the linker is replacing those N files with dummy bfds and 
ignores everything that's put in there when the files are claimed by the 
plugin. Therefore, it turns out my guess that they needed to be merged 
on the linker side might not be the least invasive.

Independently from this patch, I believe it would be good to have LTO 
output such sections.

>>>
>>> c) Current option recording is missing macros, which can influence 
>>> compilation significantly:
>>> -D_FORTIFY_SOURCE=2
>>
>> This is not the case for this patch:
>>
>> [egeyar@localhost save-commandline]$ g++ main.c 
>> --record-gcc-command-line -D_FORTIFY_SOURCE=2 -O3
>> [egeyar@localhost save-commandline]$ readelf -p .GCC.command.line a.out
>>
>> String dump of section '.GCC.command.line':
>> Â Â  [Â Â Â Â  0]Â  10.0.0 20191025 (experimental) : g++ main.c 
>> --record-gcc-command-line -D_FORTIFY_SOURCE=2 -O3
>
> That's fine, thanks.
>
> May I please ask you to take a look at LTO deeper? The patch will 
> really help us significantly.

I will look into that within a few weeks. Thanks again for bringing this 
case to my attention.

Best regards
Egeyar

>
> Thanks,
> Martin
>
>>
>>
>> The attached file showcases this as well. Please take a look, I think 
>> you might find it useful.
>>
>> Regards
>> Egeyar
>>
>>> Martin
>>>
>>> command-line as received by the driver. Secondly, 
>>> -frecord-gcc-switches saves the switches as separate entries into a 
>>> mergeable string section. Therefore, the entries belonging to 
>>> different object files get mixed up after being linked. The new 
>>> --record-gcc-command-line, on the other hand, creates one entry per 
>>> invocation. By doing so, it makes it clear which options were used 
>>> together in a single gcc invocation. Lastly, 
>>> --record-gcc-command-line also adds the version of the gcc into this 
>>> single entry to make it clear which version of gcc was called with 
>>> any given command line. This is useful in cases where .comment 
>>> section reports multiple versions.
>>>>
>>>> While there are also similarities between the implementations of 
>>>> these two options, they are completely independent. These commands 
>>>> can be used separately or together without issues. I used the same 
>>>> section that -frecord-gcc-switches uses on purpose. I could not use 
>>>> the name -frecord-gcc-command-line for this option; because of a 
>>>> {f*} in the specs, which forwards all options starting with -f to 
>>>> cc1/cc1plus as is. This is not we want for this option. We would 
>>>> like to append it a filename as well to pass the argv of the driver 
>>>> to child processes.
>>>>
>>>> This functionality operates as the following: It saves gcc's argv 
>>>> into a temporary file, and passes --record-gcc-command-line 
>>>> <tempfilename> to cc1 or cc1plus. The functionality of the backend 
>>>> is implemented via a hook. This patch includes an example 
>>>> implementation of the hook for elf targets: 
>>>> elf_record_gcc_command_line function. This function reads the given 
>>>> file and writes gcc's version and the command line into a mergeable 
>>>> string section, .GCC.command.line.
>>>>
>>>> Here is an *example usage* of the option:
>>>> [egeyar@localhost save-commandline]$ gcc main.c 
>>>> --record-gcc-command-line
>>>> [egeyar@localhost save-commandline]$ readelf -p .GCC.command.line 
>>>> a.out
>>>>
>>>> String dump of section '.GCC.command.line':
>>>> Â Â  [Â Â Â Â  0]Â  10.0.0 20191025 (experimental) : gcc main.c 
>>>> --record-gcc-command-line
>>>>
>>>>
>>>> The following is a *second example* calling g++ with -save-temps, 
>>>> -frecord-gcc-switches, and repetition of options, where 
>>>> --save-temps saves the intermediate file, main.cmdline in this 
>>>> case. You can see that the options are recorded unprocessed:
>>>>
>>>> [egeyar@localhost save-commandline]$ g++ main.c -save-temps 
>>>> --record-gcc-command-line -O0 -O2 -O3 --record-gcc-command-line
>>>> [egeyar@localhost save-commandline]$ readelf -p .GCC.command.line 
>>>> a.out
>>>>
>>>> String dump of section '.GCC.command.line':
>>>> Â Â  [Â Â Â Â  0]Â  10.0.0 20191025 (experimental) : g++ main.c 
>>>> -save-temps --record-gcc-command-line -O0 -O2 -O3 
>>>> --record-gcc-command-line
>>>>
>>>>
>>>> Here is a *third example* calling g++ with both 
>>>> -frecord-gcc-switches and --record-gcc-command-line for comparison:
>>>> [egeyar@localhost save-commandline]$ g++ main.c 
>>>> --record-gcc-command-line -frecord-gcc-switches
>>>> [egeyar@localhost save-commandline]$ readelf -p .GCC.command.line 
>>>> a.out
>>>>
>>>> String dump of section '.GCC.command.line':
>>>> Â Â  [Â Â Â Â  0]Â  10.0.0 20191025 (experimental) : g++ main.c 
>>>> --record-gcc-command-line -frecord-gcc-switches
>>>> Â Â  [Â Â Â  5c]Â  -D_GNU_SOURCE
>>>> Â Â  [Â Â Â  6a]Â  main.c
>>>> Â Â  [Â Â Â  71]Â  -mtune=generic
>>>> Â Â  [Â Â Â  80]Â  -march=x86-64
>>>> Â Â  [Â Â Â  8e]Â  --record-gcc-command-line /tmp/ccgC4ZtS.cmdline
>>>>
>>>>
>>>> The first patch of this two-patch-series only extends the testsuite 
>>>> machinery, while the second patch implements this functionality and 
>>>> adds a test case for it. In addition to that new test case, I built 
>>>> binutils as my test case after passing this option to CFLAGS. The 
>>>> added .GCC.command.line section of ld listed many compile commands 
>>>> as expected. Tested on x86_64-pc-linux-gnu.
>>>>
>>>> Please review the patches, let me know what you think and apply if 
>>>> appropriate.
>>>>
>>>> Regards
>>>> Egeyar
>>>>
>>>> Egeyar Bagcioglu (2):
>>>> Â Â  Introduce dg-require-target-object-format
>>>> Â Â  Introduce the gcc option --record-gcc-command-line
>>>>
>>>> Â  gcc/common.optÂ Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  |Â  4 +++
>>>> Â  gcc/config/elfos.hÂ Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  |Â  5 +++
>>>> Â  gcc/doc/tm.texiÂ Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  | 22 ++++++++++++
>>>> Â  gcc/doc/tm.texi.inÂ Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  |Â  4 +++
>>>> Â  gcc/gcc.cÂ Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  | 41 
>>>> ++++++++++++++++++++++
>>>> Â  gcc/gcc.hÂ Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  |Â  1 +
>>>> Â  gcc/target.defÂ Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  | 30 
>>>> ++++++++++++++++
>>>> Â  gcc/target.hÂ Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  |Â  3 ++
>>>> Â  .../c-c++-common/record-gcc-command-line.cÂ Â Â Â Â Â Â Â  |Â  8 +++++
>>>> Â  gcc/testsuite/lib/target-supports-dg.expÂ Â Â Â Â Â Â Â Â Â  | 11 ++++++
>>>> Â  gcc/toplev.cÂ Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  | 13 +++++++
>>>> Â  gcc/varasm.cÂ Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  | 36 
>>>> +++++++++++++++++++
>>>> Â  12 files changed, 178 insertions(+)
>>>> Â  create mode 100644 
>>>> gcc/testsuite/c-c++-common/record-gcc-command-line.c
>>>>
>>>
>>
>