diff mbox

Reproducible gcc builds, gfortran, and -grecord-gcc-switches

Message ID CAPTY64pVuBoW9Tu-6GNLtfeGLYj62wA_8zZS3hOxtyHFmxwCXw@mail.gmail.com
State New
Headers show

Commit Message

Simon Baldwin Aug. 16, 2012, 4:59 p.m. UTC 
On 16 August 2012 16:40, Michael Matz <matz@suse.de> wrote:
>
> ,,,
>
> Do you have considered to use a new option flag (usable in the .opt files)
> instead of a langhook?  I.e. add a flag cl_dont_record to cl_option, a
> string Norecord for the .opt files, some handling for it in
> opt-functions.awk and the like?
>
> Adding lang-hooks used by debug producers make me twitch :)

Okay.  Below is an alternative approach.

I've moved discussion to gcc-patches, since it's now more concrete
than abstract.

----------

Omit OPT_cpp_ from the DWARF producer string in gfortran.

Gfortran uses -cpp=<temporary file> internally, and with -grecord_gcc_switches
this command line switch is stored by default in object files.  This causes
problems with build and packaging systems that care about gcc binary
reproducibility and file checksums; the temporary file is different on each
compiler invocation.

Fixed by adding a new opt marker NoDwarfRecord and associated flag, filtering
options for this this setting when writing the producer string, and setting
this flag for fortran -cpp=<temporary file>

Tested for fortran (suppresses -cpp=...) and c (no effect).

gcc/ChangeLog
2012-08-16  Simon Baldwin  <simonb@google.com>

	* dwarf2out.c (gen_producer_string): Omit command line switch if
	CL_NO_DWARF_RECORD flag set.
	* opts.c (print_specific_help): Add CL_NO_DWARF_RECORD handling.
	* opts.h (CL_NO_DWARF_RECORD): New.
	* opt-functions.awk (switch_flags): Add NoDwarfRecord.

gcc/fortran/ChangeLog
2012-08-16  Simon Baldwin  <simonb@google.com>

	* lang.opt (-cpp=): Mark flag NoDwarfRecord.



--
Google UK Limited | Registered Office: Belgrave House, 76 Buckingham
Palace Road, London SW1W 9TQ | Registered in England Number: 3977902

Comments

Jakub Jelinek Aug. 16, 2012, 7:28 p.m. UTC | #1 
On Thu, Aug 16, 2012 at 06:59:09PM +0200, Simon Baldwin wrote:
> On 16 August 2012 16:40, Michael Matz <matz@suse.de> wrote:
> >
> > ,,,
> >
> > Do you have considered to use a new option flag (usable in the .opt files)
> > instead of a langhook?  I.e. add a flag cl_dont_record to cl_option, a
> > string Norecord for the .opt files, some handling for it in
> > opt-functions.awk and the like?
> >
> > Adding lang-hooks used by debug producers make me twitch :)
> 
> Okay.  Below is an alternative approach.
> 
> I've moved discussion to gcc-patches, since it's now more concrete
> than abstract.

You could have just added
  case OPT_cpp_:
to the switch in gen_producer_string, instead of all this.

> --- gcc/dwarf2out.c	(revision 190442)
> +++ gcc/dwarf2out.c	(working copy)
> @@ -18101,6 +18101,9 @@ gen_producer_string (void)
>  	/* Ignore these.  */
>  	continue;
>        default:
> +        if (cl_options[save_decoded_options[j].opt_index].flags
> +	    & CL_NO_DWARF_RECORD)
> +	  continue;
>          gcc_checking_assert (save_decoded_options[j].canonical_option[0][0]
>  			     == '-');
>          switch (save_decoded_options[j].canonical_option[0][1])

	Jakub
Simon Baldwin Aug. 17, 2012, 9:46 a.m. UTC | #2 
On 16 August 2012 21:28, Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Thu, Aug 16, 2012 at 06:59:09PM +0200, Simon Baldwin wrote:
> > On 16 August 2012 16:40, Michael Matz <matz@suse.de> wrote:
> > >
> > > ,,,
> > >
> > > Do you have considered to use a new option flag (usable in the .opt files)
> > > instead of a langhook?  I.e. add a flag cl_dont_record to cl_option, a
> > > string Norecord for the .opt files, some handling for it in
> > > opt-functions.awk and the like?
> > >
> > > Adding lang-hooks used by debug producers make me twitch :)
> >
> > Okay.  Below is an alternative approach.
> >
> > I've moved discussion to gcc-patches, since it's now more concrete
> > than abstract.
>
> You could have just added
>   case OPT_cpp_:
> to the switch in gen_producer_string, instead of all this.

Thanks.  I was under the impression, apparently mistaken, that
OPT_cpp_ exists only if fortran is enabled.

--
Google UK Limited | Registered Office: Belgrave House, 76 Buckingham
Palace Road, London SW1W 9TQ | Registered in England Number: 3977902
Joseph Myers Aug. 17, 2012, 2:55 p.m. UTC | #3 
On Fri, 17 Aug 2012, Simon Baldwin wrote:

> > You could have just added
> >   case OPT_cpp_:
> > to the switch in gen_producer_string, instead of all this.
> 
> Thanks.  I was under the impression, apparently mistaken, that
> OPT_cpp_ exists only if fortran is enabled.

OPT_* for Fortran options only exist when the Fortran front-end is in the 
source tree (whether or not enabled).  I think we try to avoid knowingly 
breaking use cases where people remove some front ends from the source 
tree, although we don't actively test them and no longer provide split-up 
source tarballs.
Simon Baldwin Aug. 20, 2012, 2:30 p.m. UTC | #4 
On 17 August 2012 16:55, Joseph S. Myers <joseph@codesourcery.com> wrote:
>
> On Fri, 17 Aug 2012, Simon Baldwin wrote:
>
> > > You could have just added
> > >   case OPT_cpp_:
> > > to the switch in gen_producer_string, instead of all this.
> >
> > Thanks.  I was under the impression, apparently mistaken, that
> > OPT_cpp_ exists only if fortran is enabled.
>
> OPT_* for Fortran options only exist when the Fortran front-end is in the
> source tree (whether or not enabled).  I think we try to avoid knowingly
> breaking use cases where people remove some front ends from the source
> tree, although we don't actively test them and no longer provide split-up
> source tarballs.

Thanks for the update.  Which fix should move forwards?

--
Google UK Limited | Registered Office: Belgrave House, 76 Buckingham
Palace Road, London SW1W 9TQ | Registered in England Number: 3977902
Joseph Myers Aug. 20, 2012, 2:45 p.m. UTC | #5 
On Mon, 20 Aug 2012, Simon Baldwin wrote:

> > OPT_* for Fortran options only exist when the Fortran front-end is in the
> > source tree (whether or not enabled).  I think we try to avoid knowingly
> > breaking use cases where people remove some front ends from the source
> > tree, although we don't actively test them and no longer provide split-up
> > source tarballs.
> 
> Thanks for the update.  Which fix should move forwards?

I think the approach using a new option flag is the way to go, though the 
patch needs (at least) documentation for the new flag in options.texi.
diff mbox

Patch

Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c	(revision 190442)
+++ gcc/dwarf2out.c	(working copy)
@@ -18101,6 +18101,9 @@  gen_producer_string (void)
 	/* Ignore these.  */
 	continue;
       default:
+        if (cl_options[save_decoded_options[j].opt_index].flags
+	    & CL_NO_DWARF_RECORD)
+	  continue;
         gcc_checking_assert (save_decoded_options[j].canonical_option[0][0]
 			     == '-');
         switch (save_decoded_options[j].canonical_option[0][1])
Index: gcc/opts.c
===================================================================
--- gcc/opts.c	(revision 190442)
+++ gcc/opts.c	(working copy)
@@ -1186,7 +1186,9 @@  print_specific_help (unsigned int includ
     {
       if (any_flags == 0)
 	{
-	  if (include_flags & CL_UNDOCUMENTED)
+	  if (include_flags & CL_NO_DWARF_RECORD)
+	    description = _("The following options are not recorded by DWARF");
+          else if (include_flags & CL_UNDOCUMENTED)
 	    description = _("The following options are not documented");
 	  else if (include_flags & CL_SEPARATE)
 	    description = _("The following options take separate arguments");
@@ -1292,7 +1294,7 @@  common_handle_option (struct gcc_options
 	/* Walk along the argument string, parsing each word in turn.
 	   The format is:
 	   arg = [^]{word}[,{arg}]
-	   word = {optimizers|target|warnings|undocumented|
+	   word = {optimizers|target|warnings|undocumented|nodwarfrecord|
 		   params|common|<language>}  */
 	while (* a != 0)
 	  {
@@ -1307,6 +1309,7 @@  common_handle_option (struct gcc_options
 	      { "target", CL_TARGET },
 	      { "warnings", CL_WARNING },
 	      { "undocumented", CL_UNDOCUMENTED },
+	      { "nodwarfrecord", CL_NO_DWARF_RECORD },
 	      { "params", CL_PARAMS },
 	      { "joined", CL_JOINED },
 	      { "separate", CL_SEPARATE },
Index: gcc/opts.h
===================================================================
--- gcc/opts.h	(revision 190442)
+++ gcc/opts.h	(working copy)
@@ -145,6 +145,7 @@  extern const unsigned int cl_lang_count;
 #define CL_JOINED		(1U << 22) /* If takes joined argument.  */
 #define CL_SEPARATE		(1U << 23) /* If takes a separate argument.  */
 #define CL_UNDOCUMENTED		(1U << 24) /* Do not output with --help.  */
+#define CL_NO_DWARF_RECORD	(1U << 25) /* Do not add to producer string.  */

 /* Flags for an enumerated option argument.  */
 #define CL_ENUM_CANONICAL	(1 << 0) /* Canonical for this value.  */
Index: gcc/fortran/lang.opt
===================================================================
--- gcc/fortran/lang.opt	(revision 190442)
+++ gcc/fortran/lang.opt	(working copy)
@@ -287,7 +287,7 @@  Fortran Negative(nocpp)
 Enable preprocessing

 cpp=
-Fortran Joined Negative(nocpp) Undocumented
+Fortran Joined Negative(nocpp) Undocumented NoDwarfRecord
 ; Internal option generated by specs from -cpp.

 nocpp
Index: gcc/opt-functions.awk
===================================================================
--- gcc/opt-functions.awk	(revision 190442)
+++ gcc/opt-functions.awk	(working copy)
@@ -103,6 +103,7 @@  function switch_flags (flags)
 	  test_flag("JoinedOrMissing", flags, " | CL_JOINED") \
 	  test_flag("Separate", flags, " | CL_SEPARATE") \
 	  test_flag("Undocumented", flags,  " | CL_UNDOCUMENTED") \
+	  test_flag("NoDwarfRecord", flags,  " | CL_NO_DWARF_RECORD") \
 	  test_flag("Warning", flags,  " | CL_WARNING") \
 	  test_flag("Optimization", flags,  " | CL_OPTIMIZATION")
 	sub( "^0 \\| ", "", result )