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

2019-11-18 Thread Egeyar Bagcioglu




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 

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

2019-11-15 Thread Jeff Law
On 11/14/19 2:15 AM, Martin Liška wrote:
> On 11/13/19 8:23 PM, Jeff Law wrote:
>> On 11/13/19 2:37 AM, Martin Liška wrote:

 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.
> 
> Hi.
> 
>> Right.  We inject -g into the default build flags.  We extract the
>> resultant debug info into a .debuginfo RPM.
> 
> Which means it can be possible to you to process a rpm check on the
> .debuginfo
> RPM packages. Right?
Yea, but there was a forward looking requirement that we also be able to
query a binary/DSO without debuginfo.

> 
>>
>> The original motivation behind annobin was to verify how well the
>> injection mechanism worked.
> 
> I thought the original motivation was to provide a sanity check on RPM
> level
> which will verify that all object files use the proper $Optflags
> (mainly security hardening ones like -D_FORTIFY_SOURCE=1,
> -fstack-protector-strong, -fstack-clash-protection, ..)?
> And so that you can guarantee that the packages are "safe" :)
In my mind those are the same problem.

If your flags injection sucks, then you'll get lousy coverage for things
like stack protector, fortification, etc.  annobin lets us find gaps in
coverage easily and fix them.

When you get gaps for something like cf-protection, then all the work
for cf-protection is wasted.  So identifying these gaps is critical.

Jeff



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

2019-11-14 Thread Martin Liška

On 11/13/19 8:23 PM, Jeff Law wrote:

On 11/13/19 2:37 AM, Martin Liška wrote:


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.


Hi.


Right.  We inject -g into the default build flags.  We extract the
resultant debug info into a .debuginfo RPM.


Which means it can be possible to you to process a rpm check on the .debuginfo
RPM packages. Right?



The original motivation behind annobin was to verify how well the
injection mechanism worked.


I thought the original motivation was to provide a sanity check on RPM level
which will verify that all object files use the proper $Optflags
(mainly security hardening ones like -D_FORTIFY_SOURCE=1, 
-fstack-protector-strong, -fstack-clash-protection, ..)?
And so that you can guarantee that the packages are "safe" :)

Martin


We originally wanted to do something like
what Egeyar has done, but it's been proposed in the past and was highly
controversial.  Rather than fight that problem or have a Red Hat
specific patch, we built annobin/annocheck which (IMHO) handles this
kind of need quite well.


Jeff





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

2019-11-13 Thread Jeff Law
On 11/6/19 10:21 AM, Egeyar Bagcioglu wrote:
> Hello,
> 
> 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.
> 
So the first question I think we need to answer is do we want this
feature.  Similar features have been proposed in the past and rejected
as undesirable.

I don't have particularly strong opinions here.

Jeff




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

2019-11-13 Thread Jeff Law
On 11/13/19 2:37 AM, Martin Liška wrote:
>>
>> 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.
Right.  We inject -g into the default build flags.  We extract the
resultant debug info into a .debuginfo RPM.

The original motivation behind annobin was to verify how well the
injection mechanism worked.  We originally wanted to do something like
what Egeyar has done, but it's been proposed in the past and was highly
controversial.  Rather than fight that problem or have a Red Hat
specific patch, we built annobin/annocheck which (IMHO) handles this
kind of need quite well.


Jeff



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

2019-11-13 Thread Martin Liška

On 11/7/19 4:13 PM, Nick Clifton wrote:

Hi Egeyar,

Thanks for including me in this discussion.


This option is similar to -frecord-gcc-switches.


For the record I will also note that there is -fverbose-asm which
does almost the same thing, but only records the options as comments
in the assembler.  They are never converted into data in the actual
object files.


Heh, we have even more options..



It is also worth noting that if your goal is to record how a binary
was produced, possibly with an eye to reproducibility, then you may
also need to record some environment variables too.

One thing I found with annobin is that capturing preprocessor options
(eg -D_FORTIFY_SOURCE) can be quite hard from inside gcc, since often
they have already been processed and discarded.  I do not know if this
affects your actual patch though.

Speaking of annobin, I will bang the gcc plugin gong again here and say
that if your patch is rejected then you might want to consider turning
it into a plugin instead.  In that way you will not need approval from
the gcc maintainers.  But of course you will have to maintain and
publicise the plugin yourself.

One other thought occurs to me, which is that if the patch is acceptable,
or at least the idea of it, then maybe it would be better to amalgamate
all of the current command line recording options into a single version.
Eg:

   --frecord-options=[dwarf,assembler,object]

where:

   --frecord-options=dwarf  is a synonym for -grecord-switches
   --frecord-options=assembler  is a synonym for -fverbose-asm
   --frecord-options=object is a synonym for your option


I really like the suggested option name unification.

Martin



The user could supply one or more of the selectors to have the recording
happen in multiple places.

Just an idea.

Cheers
   Nick





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

2019-11-13 Thread Martin Liška

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.



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 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 

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

2019-11-07 Thread Egeyar Bagcioglu




On 11/7/19 4:13 PM, Nick Clifton wrote:

Hi Egeyar,

Thanks for including me in this discussion.


This option is similar to -frecord-gcc-switches.

For the record I will also note that there is -fverbose-asm which
does almost the same thing, but only records the options as comments
in the assembler.  They are never converted into data in the actual
object files.


Right.


It is also worth noting that if your goal is to record how a binary
was produced, possibly with an eye to reproducibility, then you may
also need to record some environment variables too.


That is an important point and in fact, such a need might arise. Even in 
that case, I would like to keep the options of GCC as modular as 
possible so that we can pick and drop as the specific use cases require. 
This one is the one that saves the command line for now.


If we implement the aliases you suggested at the end, we can even create 
aliases that combine them for the user.



One thing I found with annobin is that capturing preprocessor options
(eg -D_FORTIFY_SOURCE) can be quite hard from inside gcc, since often
they have already been processed and discarded.  I do not know if this
affects your actual patch though.


Yes, this was one of Martin's points as well. It is not the case for 
this patch, though. I have noticed that the current options aim to 
capture more than the command line, dive into GCC, and therefore miss or 
discard some options given by the user. This patch only stores *argv* as 
the driver receives and writes it to the object file blindly. Therefore, 
capturing options such as -D_FORTIFY_SOURCE is no special case. Really, 
this patch only answers the simple question of "How did you call GCC?".



Speaking of annobin, I will bang the gcc plugin gong again here and say
that if your patch is rejected then you might want to consider turning
it into a plugin instead.  In that way you will not need approval from
the gcc maintainers.  But of course you will have to maintain and
publicise the plugin yourself.


Thanks for the suggestion. That will always be in my mind for more 
ambitious cases. In the case of this specific 160-line patch though, I 
believe it wouldn't bother us to maintain one more small patch in the 
GCC packages we distribute. It can be "only at Oracle!". However, for me 
this is really a basic functionality. Intel's icc has the most similar 
-sox option too. Thinking back on how many times I said "now, how did we 
compile this?" in the past, I would like this to be available for all 
GCC users too, in the spirit of sharing.



One other thought occurs to me, which is that if the patch is acceptable,
or at least the idea of it, then maybe it would be better to amalgamate
all of the current command line recording options into a single version.
Eg:

   --frecord-options=[dwarf,assembler,object]

where:

   --frecord-options=dwarf  is a synonym for -grecord-switches
   --frecord-options=assembler  is a synonym for -fverbose-asm
   --frecord-options=object is a synonym for your option

The user could supply one or more of the selectors to have the recording
happen in multiple places.

Just an idea.


This is a very good idea for the user experience! I already pass an 
argument to cc1; however, we can always simplify the arguments of the 
driver so that these similar functionalities can be called via one 
common name plus an option. I really like the idea.



Cheers
   Nick



Thanks Nick!

Regards
Egeyar


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

2019-11-07 Thread Nick Clifton
Hi Egeyar,

Thanks for including me in this discussion.

>>> This option is similar to -frecord-gcc-switches.

For the record I will also note that there is -fverbose-asm which
does almost the same thing, but only records the options as comments
in the assembler.  They are never converted into data in the actual
object files.

It is also worth noting that if your goal is to record how a binary
was produced, possibly with an eye to reproducibility, then you may
also need to record some environment variables too.

One thing I found with annobin is that capturing preprocessor options
(eg -D_FORTIFY_SOURCE) can be quite hard from inside gcc, since often
they have already been processed and discarded.  I do not know if this
affects your actual patch though.

Speaking of annobin, I will bang the gcc plugin gong again here and say
that if your patch is rejected then you might want to consider turning
it into a plugin instead.  In that way you will not need approval from
the gcc maintainers.  But of course you will have to maintain and 
publicise the plugin yourself.

One other thought occurs to me, which is that if the patch is acceptable,
or at least the idea of it, then maybe it would be better to amalgamate
all of the current command line recording options into a single version.
Eg:

  --frecord-options=[dwarf,assembler,object]

where:

  --frecord-options=dwarf  is a synonym for -grecord-switches
  --frecord-options=assembler  is a synonym for -fverbose-asm
  --frecord-options=object is a synonym for your option

The user could supply one or more of the selectors to have the recording
happen in multiple places.

Just an idea.

Cheers
  Nick



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

2019-11-07 Thread Egeyar Bagcioglu


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.


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. 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. 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.


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.




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    ""
.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 

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

2019-11-07 Thread Martin Liška

+ adding the author of Annobin to the email thread

On 11/7/19 10:24 AM, Martin Liška wrote:

a) it does not print per function options, which can be modified with 
__attribute__ (or pragma):


Compiler is aware of the information (and uses it in inlining (or ICF) for 
instance):

  cl_optimization *opt1 = opts_for_fn (decl);
  cl_optimization *opt2 = opts_for_fn (item->decl);

  if (opt1 != opt2 && !cl_optimization_option_eq (opt1, opt2))
{
  if (dump_file && (dump_flags & TDF_DETAILS))
{
  fprintf (dump_file, "optimization flags difference");
  cl_optimization_print_diff (dump_file, 2, opt1, opt2);
}

  return return_false_with_msg ("optimization flags are different");
}

Martin


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

2019-11-07 Thread Martin Liška

On 11/6/19 6:21 PM, Egeyar Bagcioglu wrote:

Hello,


Hello.



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).



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. Moreover, we also have one another option 
-grecord-gcc-switches
that saves command line into DWARF. 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/

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;
}

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   ""
.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

c) Current option recording is missing macros, which can influence compilation 
significantly:
-D_FORTIFY_SOURCE=2

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  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 

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

2019-11-06 Thread Egeyar Bagcioglu
Hello,

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.

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 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  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