Hello, I see Boris' patch was accepted with some further changes.

Any chance my additions for reproducible builds can be included as well? They 
are described below.

It should still apply on top, minus the strchr -> strrchr change which was 
folded into Boris' accepted version.

Ximin

Ximin Luo:
> Boris Kolpackov:
>> Thanks for the review. Third revision of the patch attached.
>>
>> [..]
> Here is a follow-up patch, meant to be applied on top of Boris' patch, to 
> address some issues we found based on discussions and experiments at the 
> Reproducible Builds project.
> 
> Most of this patch is re-implementing the patch from [1] originally by Daniel 
> Kahn Gillmor. Back then it was rejected in favour of a simpler approach 
> because 
> its necessity was not realised. However, more experiments since then have 
> revealed that its basic approach is useful in more general scenarios:
> 
> Higher-level build scripts sometimes like to save CFLAGS etc into the build 
> output, making the overall build output unreproducible even if GCC is playing 
> nicely. Rather than add logic to strip -f{file,debug,macro,...}-prefix-map, 
> into all possible higher-level programs that might want to save CFLAGS, it is
> simpler if GCC could read it from a user-specified environment variable. The
> fact that the name of the environment variable is still present on the
> command-line, should make it obvious what is being done, and avoid confusion
> that might arise if an implicit "magic" environment variable is used.
> 
> In the patch thread (see [1] and follow-ups) there was concern that the 
> prefix 
> '$' would cause awkward escaping behaviour, so the prefix 'ENV:' was chosen. 
> A 
> concern about this conflicting with AmigaOS paths was voiced but not 
> explored; 
> of course it can be changed to something else if needed.
> 
> (NetBSD is already carrying this patch in their system GCC [2], presumably for
> the same reason mentioned above.)
> 
> [1] https://gcc.gnu.org/ml/gcc-patches/2015-12/msg01168.html
> [2] 
> http://cvsweb.netbsd.org/bsdweb.cgi/src/external/gpl3/gcc/dist/gcc/final.c?rev=1.2&content-type=text/x-cvsweb-markup
> 
> The other thing that this patch does, is to split the old and new prefixs by
> the last '=' sign rather than the first. This allows arbitrary old prefixes to
> be mapped, which would be more flexible than mapping to arbitrary new 
> prefixes.
> The rust compiler is adopting this strategy too: [3].
> 
> [3] https://github.com/rust-lang/rust/issues/41555#issuecomment-321078174
> 
> ChangeLogs
> ----------
> 
> gcc/ChangeLog:
> 
> 2017-12-19  Ximin Luo  <infini...@pwned.gg>
> 
>       PR other/70268
>       * file-prefix-map.c (add_prefix_map): Support reading old prefix from 
> the
>       environment. Split old and new parts by the last '=' rather than the 
> first.
>       * file-prefix-map.h: Define macros for the ENV: prefix.
> 
> gcc/testsuite/ChangeLog:
> 
> 2017-12-19  Ximin Luo  <infini...@pwned.gg>
> 
>       PR other/70268
>       * c-c++-common/ffile-prefix-map-env.c: New test.
> 
> Index: gcc-8-20171217/gcc/file-prefix-map.c
> ===================================================================
> --- gcc-8-20171217.orig/gcc/file-prefix-map.c
> +++ gcc-8-20171217/gcc/file-prefix-map.c
> @@ -41,16 +41,40 @@ add_prefix_map (file_prefix_map *&maps,
>  {
>    file_prefix_map *map;
>    const char *p;
> +  char *env;
> +  const char *old;
> +  size_t oldlen;
>  
> -  p = strchr (arg, '=');
> +  p = strrchr (arg, '=');
>    if (!p)
>      {
>        error ("invalid argument %qs to %qs", arg, opt);
>        return;
>      }
> +  if (0 == strncmp(FILE_PREFIX_MAP_ENV_PREFIX, arg, 
> FILE_PREFIX_MAP_ENV_PREFIX_OFFSET))
> +    {
> +      const char *arg_offset = arg + FILE_PREFIX_MAP_ENV_PREFIX_OFFSET;
> +      env = xstrndup (arg_offset, p - arg_offset);
> +      old = getenv (env);
> +      if (!old)
> +     {
> +       warning (0, "environment variable %qs not set in argument to %s",
> +                env, opt);
> +       free (env);
> +       return;
> +     }
> +      oldlen = strlen (old);
> +      free (env);
> +    }
> +  else
> +    {
> +      old = xstrndup (arg, p - arg);
> +      oldlen = p - arg;
> +    }
> +
>    map = XNEW (file_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);
> Index: gcc-8-20171217/gcc/file-prefix-map.h
> ===================================================================
> --- gcc-8-20171217.orig/gcc/file-prefix-map.h
> +++ gcc-8-20171217/gcc/file-prefix-map.h
> @@ -18,6 +18,9 @@
>  #ifndef GCC_FILE_PREFIX_MAP_H
>  #define GCC_FILE_PREFIX_MAP_H
>  
> +#define FILE_PREFIX_MAP_ENV_PREFIX "ENV:"
> +#define FILE_PREFIX_MAP_ENV_PREFIX_OFFSET 
> (sizeof(FILE_PREFIX_MAP_ENV_PREFIX) - 1)
> +
>  void add_macro_prefix_map (const char *);
>  void add_debug_prefix_map (const char *);
>  void add_file_prefix_map (const char *);
> Index: gcc-8-20171217/gcc/testsuite/c-c++-common/ffile-prefix-map-env.c
> ===================================================================
> --- /dev/null
> +++ gcc-8-20171217/gcc/testsuite/c-c++-common/ffile-prefix-map-env.c
> @@ -0,0 +1,13 @@
> +/* Test __builtin_FILE() with ENV: prefix-map. */
> +/* { dg-do run } */
> +/* { dg-set-compiler-env-var SRCDIR "$srcdir" } */
> +/* { dg-options "-ffile-prefix-map=ENV:SRCDIR=xxx" } */
> +
> +#include <stdio.h>
> +
> +int main ()
> +{
> +  printf ("__builtin_FILE starts with %s\n", __builtin_FILE ());
> +}
> +
> +/* { dg-output "__builtin_FILE starts with xxx" } */
> 


-- 
GPG: ed25519/56034877E1F87C35
GPG: rsa4096/1318EFAC5FBBDBCE
https://github.com/infinity0/pubkeys.git

Reply via email to