On 07/21/2017 10:15 AM, Ximin Luo wrote:
> Define the BUILD_PATH_PREFIX_MAP environment variable, and treat it as
> implicit
> -fdebug-prefix-map CLI options specified before any explicit such options.
>
> Much of the generic code for applying and parsing prefix-maps is implemented
> in
> libiberty instead of the dwarf2 parts of the code, in order to make subsequent
> patches unrelated to debuginfo easier.
>
> Acknowledgements
> ----------------
>
> Daniel Kahn Gillmor who wrote the patch for r231835, which saved me a lot of
> time figuring out what to edit.
>
> HW42 for discussion on the details of the proposal, and for suggesting that we
> retain the ability to map the prefix to something other than ".".
>
> Other contributors to the BUILD_PATH_PREFIX_MAP specification, see
> https://reproducible-builds.org/specs/build-path-prefix-map/
>
> ChangeLogs
> ----------
>
> include/ChangeLog:
>
> 2017-07-21 Ximin Luo <infini...@pwned.gg>
>
> * prefix-map.h: New file implementing the BUILD_PATH_PREFIX_MAP
> specification; includes code from /gcc/final.c and code adapted from
> examples attached to the specification.
>
> libiberty/ChangeLog:
>
> 2017-07-21 Ximin Luo <infini...@pwned.gg>
>
> * prefix-map.c: New file implementing the BUILD_PATH_PREFIX_MAP
> specification; includes code from /gcc/final.c and code adapted from
> examples attached to the specification.
> * Makefile.in: Update for new files.
>
> gcc/ChangeLog:
>
> 2017-07-21 Ximin Luo <infini...@pwned.gg>
>
> * debug.h: Declare add_debug_prefix_map_from_envvar.
> * final.c: Define add_debug_prefix_map_from_envvar, and refactor
> prefix-map utilities to use equivalent code from libiberty instead.
> * opts-global.c: (handle_common_deferred_options): Call
> add_debug_prefix_map_from_envvar before processing options.
>
> gcc/testsuite/ChangeLog:
>
> 2017-07-21 Ximin Luo <infini...@pwned.gg>
>
> * lib/gcc-dg.exp: Allow dg-set-compiler-env-var to take only one
> argument in which case it unsets the given env var.
> * gcc.dg/debug/dwarf2/build_path_prefix_map-1.c: New test.
> * gcc.dg/debug/dwarf2/build_path_prefix_map-2.c: New test.
I think some revamping will be needed here to avoid using the
environment variable.
Conceptually I'm OK with doing much of the work in libiberty, driven by GCC.
>
> Index: gcc-8-20170716/include/prefix-map.h
> ===================================================================
> --- /dev/null
> +++ gcc-8-20170716/include/prefix-map.h
> @@ -0,0 +1,94 @@
> +/* Declarations for manipulating filename prefixes.
> + Written 2017 by Ximin Luo <infini...@pwned.gg>
> + This code is in the public domain. */
> +
> +#ifndef _PREFIX_MAP_H
> +#define _PREFIX_MAP_H
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#ifdef HAVE_STDLIB_H
> +#include <stdlib.h>
> +#endif
> +
> +/* Linked-list of mappings from old prefixes to new prefixes. */
Going to assume using a linked list for this stuff isn't a performance
concern in practice. We shouldn't have enough prefixes for it to matter.
> +
> +struct prefix_map
> +{
> + const char *old_prefix;
> + const char *new_prefix;
> + size_t old_len;
> + size_t new_len;
> + struct prefix_map *next;
> +};
> +
> +
> +/* Find a mapping suitable for the given OLD_NAME in the linked list MAP.\
> +
> + If a mapping is found, writes a pointer to the non-matching suffix part of
> + OLD_NAME in SUFFIX, and its length in SUF_LEN.
> +
> + Returns NULL if there was no suitable mapping. */
> +struct prefix_map *
> +prefix_map_find (struct prefix_map *map, const char *old_name,
> + const char **suffix, size_t *suf_len);
> +
> +/* Prepend a prefix map before a given SUFFIX.
> +
> + The remapped name is written to NEW_NAME and returned as a const pointer.
> No
> + allocations are performed; the caller must ensure it can hold at least
> + MAP->NEW_LEN + SUF_LEN + 1 characters. */
> +const char *
> +prefix_map_prepend (struct prefix_map *map, char *new_name,
> + const char *suffix, size_t suf_len);
> +
> +/* Remap a filename.
> +
> + Returns OLD_NAME unchanged if there was no remapping, otherwise returns a
> + pointer to newly-allocated memory for the remapped filename. The memory
> is
> + allocated by the given ALLOC function, which also determines who is
> + responsible for freeing it. */
> +#define prefix_map_remap_alloc_(map_head, old_name, alloc) \
> + __extension__
> \
> + ({ \
> + const char *__suffix; \
> + size_t __suf_len;
> \
> + struct prefix_map *__map;
> \
> + (__map = prefix_map_find ((map_head), (old_name), &__suffix,
> &__suf_len)) \
> + ? prefix_map_prepend (__map, \
> + (char *) alloc (__map->new_len + __suf_len + 1), \
> + __suffix, __suf_len) \
> + : (old_name); \
> + })
Is there some significant value in using the statements as expressions
extension here? If not, just make this a function.
> +
> +/* Remap a filename.
> +
> + Returns OLD_NAME unchanged if there was no remapping, otherwise returns a
> + stack-allocated pointer to the newly-remapped filename. */
> +#define prefix_map_remap_alloca(map_head, old_name) \
> + prefix_map_remap_alloc_ (map_head, old_name, alloca)
Seems unwise to #define something like this is the normal namespace
within a header file. At the least you should #undef it so that it
doesn't bleed out. But again, why not just make this a function?
> Index: gcc-8-20170716/libiberty/prefix-map.c
> ===================================================================
> --- /dev/null
> +++ gcc-8-20170716/libiberty/prefix-map.c
> Index: gcc-8-20170716/gcc/debug.h
> ===================================================================
> --- gcc-8-20170716.orig/gcc/debug.h
> +++ gcc-8-20170716/gcc/debug.h
> @@ -236,6 +236,7 @@ extern void dwarf2out_switch_text_sectio
>
> const char *remap_debug_filename (const char *);
> void add_debug_prefix_map (const char *);
> +void add_debug_prefix_map_from_envvar ();
>
> /* For -fdump-go-spec. */
>
> Index: gcc-8-20170716/gcc/final.c
> ===================================================================
> --- gcc-8-20170716.orig/gcc/final.c
> +++ gcc-8-20170716/gcc/final.c
> @@ -46,6 +46,7 @@ along with GCC; see the file COPYING3.
> #define INCLUDE_ALGORITHM /* reverse */
> #include "system.h"
> #include "coretypes.h"
> +#include "prefix-map.h"
> #include "backend.h"
> #include "target.h"
> #include "rtl.h"
> @@ -1506,22 +1507,9 @@ asm_str_count (const char *templ)
> return count;
> }
>
> -/* ??? This is probably the wrong place for these. */
> -/* Structure recording the mapping from source file and directory
> - names at compile time to those to be embedded in debug
> - information. */
> -struct debug_prefix_map
> -{
> - const char *old_prefix;
> - const char *new_prefix;
> - size_t old_len;
> - size_t new_len;
> - struct debug_prefix_map *next;
> -};
> -
> -/* Linked list of such structures. */
> -static debug_prefix_map *debug_prefix_maps;
>
> +/* Linked list of `struct prefix_map'. */
> +static prefix_map *debug_prefix_maps = NULL;
>
> /* Record a debug file prefix mapping. ARG is the argument to
> -fdebug-prefix-map and must be of the form OLD=NEW. */
> @@ -1529,7 +1517,7 @@ static debug_prefix_map *debug_prefix_ma
> void
> add_debug_prefix_map (const char *arg)
> {
> - debug_prefix_map *map;
> + prefix_map *map;
> const char *p;
>
> p = strchr (arg, '=');
> @@ -1538,7 +1526,7 @@ add_debug_prefix_map (const char *arg)
> error ("invalid argument %qs to -fdebug-prefix-map", arg);
> return;
> }
> - map = XNEW (debug_prefix_map);
> + map = XNEW (prefix_map);
> map->old_prefix = xstrndup (arg, p - arg);
> map->old_len = p - arg;
> p++;
> @@ -1548,28 +1536,32 @@ add_debug_prefix_map (const char *arg)
> debug_prefix_maps = map;
> }
>
> +/* Add debug-prefix-maps from BUILD_PATH_PREFIX_MAP environment variable. */
> +
> +void
> +add_debug_prefix_map_from_envvar ()
> +{
> + const char *arg = getenv ("BUILD_PATH_PREFIX_MAP");
> +
> + if (!arg || prefix_map_parse (&debug_prefix_maps, arg))
> + return;
> +
> + error ("environment variable BUILD_PATH_PREFIX_MAP is "
> + "not well formed; see the GCC documentation for more details.");
> +}
> +
> /* Perform user-specified mapping of debug filename prefixes. Return
> the new name corresponding to FILENAME. */
>
> const char *
> remap_debug_filename (const char *filename)
> {
> - debug_prefix_map *map;
> - char *s;
> - const char *name;
> - size_t name_len;
> -
> - for (map = debug_prefix_maps; map; map = map->next)
> - if (filename_ncmp (filename, map->old_prefix, map->old_len) == 0)
> - break;
> - if (!map)
> + const char *name = prefix_map_remap_alloca (debug_prefix_maps, filename);
alloca? Let's avoid alloca if reasonably possible. Particularly if the
size of the alloca'd area could potentially be under user control.
jeff
