On Fri 2015-12-11 12:03:28 -0500, Bernd Schmidt wrote:
> On 12/11/2015 05:49 PM, Daniel Kahn Gillmor wrote:
>> I've re-rolled the patch (attached below, here) to use the ENV: prefix
>> instead of the $.
>
> It might be irrelevant at this point, but the "ENV:" prefix is used in 
> AmigaOS and could be part of a filename.

As a full-path prefix?

>> +  if (0 == strncmp(ENV_PREFIX, arg, ENV_PREFIX_OFFSET))
>> +    {
>> +      env = xstrndup (arg+ENV_PREFIX_OFFSET, p - (arg+ENV_PREFIX_OFFSET));
>
> Spaces before ( and around operators like +. Please review our coding 
> guidelines and have a look at the surrounding code.

right, sorry about that.  Attached at the bottom of this mail is a patch
that i think should clean this up.  (i've also updated bugzilla
correctly after a couple tries -- sorry for the noise there)

> Wouldn't it be simpler just to special-case -fdebug-prefix-map in 
> gen_producer_string? The environment variable thing strikes me as 
> unnecessary.

I think you mean so that we would just ignore -fdebug-prefix-map
entirely when writing DW_AT_producer, so that you could build
reproducibly with (for example):

 gcc -fdebug-prefix-map=$(pwd)=/usr/src

We'd considered and discarded this approach in the past out of concern
for possible build systems that can easily vary the environment, but
work with a static list of CFLAGS to pass to the compiler.  On further
inspection, it's not clear that anyone has a concrete example of a build
system with this constraint.

Here's a one-liner patch for this approach (also at
https://gcc.gnu.org/bugzilla/attachment.cgi?id=37007):

>From 13ca37337660f5385295052b3c1aebfc4e29092c Mon Sep 17 00:00:00 2001
From: Daniel Kahn Gillmor <d...@fifthhorseman.net>
Date: Fri, 11 Dec 2015 13:47:45 -0500
Subject: [PATCH] gcc: ignore -fdebug-prefix-map in DW_AT_producer (improved
 reproducibility)

Work on the reproducible-builds project [0] has identified that build
paths are one cause of output variation between builds.  This
changeset allows users to avoid this variation when building C objects
with debug symbols, while leaving the default behavior unchanged.

Background
----------

gcc includes the build path in any generated DWARF debugging symbols,
specifically in DW_AT_comp_dir, but allows the embedded path to be
changed via -fdebug-prefix-map.

When -fdebug-prefix-map is used with the current build path, it
removes the build path from DW_AT_comp_dir but places it instead in
DW_AT_producer, so the reproducibility problem isn't resolved.

When building software for binary redistribution, the actual build
path on the build machine is irrelevant, and doesn't need to be
exposed in the debug symbols.

Resolution
----------

This patch resolves the problem by never including -fdebug-prefix-map
in the DW_AT_producer attribute.

People interested in building software reproducibly irrespective of
build path can do so by passing the build path itself as the first
argument to -fdebug-prefix-map.

[0] https://reproducible-builds.org
---
 gcc/dwarf2out.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 6af57b5..1a75ed7 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -20151,6 +20151,7 @@ gen_producer_string (void)
       case OPT_fpreprocessed:
       case OPT_fltrans_output_list_:
       case OPT_fresolution_:
+      case OPT_fdebug_prefix_map_:
 	/* Ignore these.  */
 	continue;
       default:
-- 
2.6.2


And here's the revised ENV:-prefixed approach with corrected coding
conventions (also at
https://gcc.gnu.org/bugzilla/attachment.cgi?id=37005):


>From 41d3bcf94ecd9b1ce2c4bccc0039bdff0b464315 Mon Sep 17 00:00:00 2001
From: Daniel Kahn Gillmor <d...@fifthhorseman.net>
Date: Thu, 10 Dec 2015 12:09:45 -0500
Subject: [PATCH v3] gcc: read -fdebug-prefix-map OLD from environment
 (improved reproducibility)

Work on the reproducible-builds project [0] has identified that build
paths are one cause of output variation between builds.  This
changeset allows users to avoid this variation when building C objects
with debug symbols, while leaving the default behavior unchanged.

Background
----------

gcc includes the build path in any generated DWARF debugging symbols,
specifically in DW_AT_comp_dir, but allows the embedded path to be
changed via -fdebug-prefix-map.

When -fdebug-prefix-map is used with the current build path, it
removes the build path from DW_AT_comp_dir but places it instead in
DW_AT_producer, so the reproducibility problem isn't resolved.

When building software for binary redistribution, the actual build
path on the build machine is irrelevant, and doesn't need to be
exposed in the debug symbols.

Resolution
----------

This patch extends the first argument to -fdebug-prefix-map ("old") to
be able to read from the environment, which allows a packager to avoid
embedded build paths in the debugging symbols with something like:

  export SOURCE_BUILD_DIR="$(pwd)"
  gcc -fdebug-prefix-map=ENV:SOURCE_BUILD_DIR=/usr/src

Details
-------

Specifically, if the "old" argument starts with a literal "ENV:", then
gcc will treat it as an environment variable name, and use the value
of the env var for prefix mapping.

As a result, DW_AT_producer contains the literal envvar name,
DW_AT_comp_dir contains the transformed build path, and the actual
build path is not at all present in the generated object file.

This has been tested successfully on amd64 machines, and i see no
reason why it would be platform-specific.

More discussion of alternate approaches considered and discarded in
the development of this change can be found at [1] for those
interested.

Feedback welcome!

[0] https://reproducible-builds.org
[1] https://lists.alioth.debian.org/pipermail/reproducible-builds/Week-of-Mon-20151130/004051.html
---
 gcc/doc/invoke.texi |  4 +++-
 gcc/final.c         | 30 ++++++++++++++++++++++++++++--
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 5256031..3f76e03 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -6440,7 +6440,9 @@ link processing time.  Merging is enabled by default.
 @item -fdebug-prefix-map=@var{old}=@var{new}
 @opindex fdebug-prefix-map
 When compiling files in directory @file{@var{old}}, record debugging
-information describing them as in @file{@var{new}} instead.
+information describing them as in @file{@var{new}} instead.  If
+@file{@var{old}} starts with a @samp{ENV:}, the corresponding environment
+variable will be dereferenced, and its value will be used instead.
 
 @item -fno-dwarf2-cfi-asm
 @opindex fdwarf2-cfi-asm
diff --git a/gcc/final.c b/gcc/final.c
index 8cb5533..670941d 100644
--- a/gcc/final.c
+++ b/gcc/final.c
@@ -1520,11 +1520,17 @@ static debug_prefix_map *debug_prefix_maps;
 /* Record a debug file prefix mapping.  ARG is the argument to
    -fdebug-prefix-map and must be of the form OLD=NEW.  */
 
+#define ENV_PREFIX "ENV:"
+#define ENV_PREFIX_OFFSET (sizeof (ENV_PREFIX) - 1)
+
 void
 add_debug_prefix_map (const char *arg)
 {
   debug_prefix_map *map;
   const char *p;
+  char *env;
+  const char *old;
+  size_t oldlen;
 
   p = strchr (arg, '=');
   if (!p)
@@ -1532,9 +1538,29 @@ add_debug_prefix_map (const char *arg)
       error ("invalid argument %qs to -fdebug-prefix-map", arg);
       return;
     }
+  if (0 == strncmp (ENV_PREFIX, arg, ENV_PREFIX_OFFSET))
+    {
+      env = xstrndup (arg + ENV_PREFIX_OFFSET, p - (arg + ENV_PREFIX_OFFSET));
+      old = getenv(env);
+      if (!old)
+	{
+	  warning (0, "environment variable %qs not set in argument to "
+		   "-fdebug-prefix-map", env);
+	  free(env);
+	  return;
+	}
+      oldlen = strlen(old);
+      free(env);
+    }
+  else
+    {
+      old = xstrndup (arg, p - arg);
+      oldlen = p - arg;
+    }
+
   map = XNEW (debug_prefix_map);
-  map->old_prefix = xstrndup (arg, p - arg);
-  map->old_len = p - arg;
+  map->old_prefix = old;
+  map->old_len = oldlen;
   p++;
   map->new_prefix = xstrdup (p);
   map->new_len = strlen (p);
-- 
2.6.2

I've tested both patches and they work for me, independently.

Thanks for your consideration,

       --dkg

Reply via email to