On Thu, Sep 10, 2020 at 1:39 AM Jakub Jelinek via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> If the gcc -c -flto ... commands to compile some or all objects are run in a
> different directory (or in different directories) from the directory in
> which the gcc -flto link line is invoked, then the .debug_line will be
> incorrect if there are any relative filenames, it will use those relative
> filenames while .debug_info will contain a different DW_AT_comp_dir.
>
> The following patch streams (at most once after each clear_line_info)
> the current working directory (what we record in DW_AT_comp_dir) when
> encountering the first relative pathname, and when reading the location info
> reads it back and if the current working directory at that point is
> different from the saved one, adjusts relative paths by adding a relative
> prefix how to go from the current working directory to the previously saved
> path (with a fallback e.g. for DOS e:\\foo vs. d:\\bar change to use
> absolute directory).
>
> Bootstrapped/regtested on x86_64-linux (both lto bootstrap and normal one;
> i686-linux doesn't build due to some unrelated libgo bugs), ok for trunk?
>
> 2020-09-10 Jakub Jelinek <ja...@redhat.com>
>
> PR debug/93865
> * lto-streamer.h (struct output_block): Add emit_pad member.
> * lto-streamer-out.c: Include toplev.h.
> (clear_line_info): Set emit_pwd.
> (lto_output_location_1): Encode the ob->current_file != xloc.file
> bit directly into the location number. If changing file, emit
> additionally a bit whether pwd is emitted and emit it before the
> first relative pathname since clear_line_info.
> (output_function, output_constructor): Don't call clear_line_info
> here.
> * lto-streamer-in.c (struct string_pair_map): New type.
> (struct string_pair_map_hasher): New type.
> (string_pair_map_hasher::hash): New method.
> (string_pair_map_hasher::equal): New method.
> (path_name_pair_hash_table, string_pair_map_allocator): New variables.
> (relative_path_prefix, canon_relative_path_prefix,
> canon_relative_file_name): New functions.
> (canon_file_name): Add relative_prefix argument, if non-NULL
> and string is a relative path, return canon_relative_file_name.
> (lto_location_cache::input_location_and_block): Decode file change
> bit from the location number. If changing file, unpack bit whether
> pwd is streamed and stream in pwd. Adjust canon_file_name caller.
> (lto_free_file_name_hash): Delete path_name_pair_hash_table
> and string_pair_map_allocator.
Hi, I've noticed that this patch is incomplete. It streams the result
of get_src_pwd without passing it through remap_debug_filename. As in
comp_dir_output in dwarf2out.cc, we should always remap all file and
directory names, including the result of get_src_pwd.
Ian
> --- gcc/lto-streamer.h.jj 2020-09-09 09:08:13.102815586 +0200
> +++ gcc/lto-streamer.h 2020-09-09 12:36:13.120070769 +0200
> @@ -718,6 +718,7 @@ struct output_block
> int current_col;
> bool current_sysp;
> bool reset_locus;
> + bool emit_pwd;
> tree current_block;
>
> /* Cache of nodes written in this section. */
> --- gcc/lto-streamer-out.c.jj 2020-09-09 09:08:13.077815963 +0200
> +++ gcc/lto-streamer-out.c 2020-09-09 13:21:34.093021582 +0200
> @@ -47,6 +47,7 @@ along with GCC; see the file COPYING3.
> #include "file-prefix-map.h" /* remap_debug_filename() */
> #include "output.h"
> #include "ipa-utils.h"
> +#include "toplev.h"
>
>
> static void lto_write_tree (struct output_block*, tree, bool);
> @@ -61,6 +62,7 @@ clear_line_info (struct output_block *ob
> ob->current_col = 0;
> ob->current_sysp = false;
> ob->reset_locus = true;
> + ob->emit_pwd = true;
> /* Initialize to something that will never appear as block,
> so that the first location with block in a function etc.
> always streams a change_block bit and the first block. */
> @@ -189,9 +191,6 @@ lto_output_location_1 (struct output_blo
> {
> location_t loc = LOCATION_LOCUS (orig_loc);
>
> - bp_pack_int_in_range (bp, 0, RESERVED_LOCATION_COUNT,
> - loc < RESERVED_LOCATION_COUNT
> - ? loc : RESERVED_LOCATION_COUNT);
> if (loc >= RESERVED_LOCATION_COUNT)
> {
> expanded_location xloc = expand_location (loc);
> @@ -207,13 +206,30 @@ lto_output_location_1 (struct output_blo
> ob->reset_locus = false;
> }
>
> - bp_pack_value (bp, ob->current_file != xloc.file, 1);
> + /* As RESERVED_LOCATION_COUNT is 2, we can use the spare value of
> + 3 without wasting additional bits to signalize file change.
> + If RESERVED_LOCATION_COUNT changes, reconsider this. */
> + gcc_checking_assert (RESERVED_LOCATION_COUNT == 2);
> + bp_pack_int_in_range (bp, 0, RESERVED_LOCATION_COUNT + 1,
> + RESERVED_LOCATION_COUNT
> + + (ob->current_file != xloc.file));
> +
> bp_pack_value (bp, ob->current_line != xloc.line, 1);
> bp_pack_value (bp, ob->current_col != xloc.column, 1);
>
> if (ob->current_file != xloc.file)
> {
> - bp_pack_string (ob, bp, remap_debug_filename (xloc.file), true);
> + bool stream_pwd = false;
> + const char *remapped = remap_debug_filename (xloc.file);
> + if (ob->emit_pwd && remapped && !IS_ABSOLUTE_PATH (remapped))
> + {
> + stream_pwd = true;
> + ob->emit_pwd = false;
> + }
> + bp_pack_value (bp, stream_pwd, 1);
> + if (stream_pwd)
> + bp_pack_string (ob, bp, get_src_pwd (), true);
> + bp_pack_string (ob, bp, remapped, true);
> bp_pack_value (bp, xloc.sysp, 1);
> }
> ob->current_file = xloc.file;
> @@ -227,6 +243,8 @@ lto_output_location_1 (struct output_blo
> bp_pack_var_len_unsigned (bp, xloc.column);
> ob->current_col = xloc.column;
> }
> + else
> + bp_pack_int_in_range (bp, 0, RESERVED_LOCATION_COUNT + 1, loc);
>
> if (block_p)
> {
> @@ -2376,7 +2394,6 @@ output_function (struct cgraph_node *nod
> fn = DECL_STRUCT_FUNCTION (function);
> ob = create_output_block (LTO_section_function_body);
>
> - clear_line_info (ob);
> ob->symbol = node;
>
> gcc_assert (current_function_decl == NULL_TREE && cfun == NULL);
> @@ -2462,7 +2479,6 @@ output_constructor (struct varpool_node
> timevar_push (TV_IPA_LTO_CTORS_OUT);
> ob = create_output_block (LTO_section_function_body);
>
> - clear_line_info (ob);
> ob->symbol = node;
>
> /* Make string 0 be a NULL string. */
> --- gcc/lto-streamer-in.c.jj 2020-09-09 09:08:13.076815979 +0200
> +++ gcc/lto-streamer-in.c 2020-09-09 17:04:08.015377451 +0200
> @@ -43,6 +43,7 @@ along with GCC; see the file COPYING3.
> #include "cfgloop.h"
> #include "debug.h"
> #include "alloc-pool.h"
> +#include "toplev.h"
>
> /* Allocator used to hold string slot entries for line map streaming. */
> static struct object_allocator<struct string_slot> *string_slot_allocator;
> @@ -50,11 +51,58 @@ static struct object_allocator<struct st
> /* The table to hold the file names. */
> static hash_table<string_slot_hasher> *file_name_hash_table;
>
> +/* The table to hold the relative pathname prefixes. */
> +
> /* This obstack holds file names used in locators. Line map datastructures
> points here and thus it needs to be kept allocated as long as linemaps
> exists. */
> static struct obstack file_name_obstack;
>
> +/* Map a pair of nul terminated strings where the first one can be
> + pointer compared, but the second can't, to another string. */
> +struct string_pair_map
> +{
> + const char *str1;
> + const char *str2;
> + const char *str3;
> + hashval_t hash;
> + bool prefix;
> +};
> +
> +/* Allocator used to hold string pair map entries for line map streaming. */
> +static struct object_allocator<struct string_pair_map>
> + *string_pair_map_allocator;
> +
> +struct string_pair_map_hasher : nofree_ptr_hash <string_pair_map>
> +{
> + static inline hashval_t hash (const string_pair_map *);
> + static inline bool equal (const string_pair_map *, const string_pair_map
> *);
> +};
> +
> +inline hashval_t
> +string_pair_map_hasher::hash (const string_pair_map *spm)
> +{
> + return spm->hash;
> +}
> +
> +inline bool
> +string_pair_map_hasher::equal (const string_pair_map *spm1,
> + const string_pair_map *spm2)
> +{
> + return (spm1->hash == spm2->hash
> + && spm1->str1 == spm2->str1
> + && spm1->prefix == spm2->prefix
> + && strcmp (spm1->str2, spm2->str2) == 0);
> +}
> +
> +/* The table to hold the pairs of pathnames and corresponding
> + resulting pathname. Used for both mapping of get_src_pwd ()
> + and recorded source working directory to relative path prefix
> + from current working directory to the recorded one, and for
> + mapping of that relative path prefix and some relative path
> + to those concatenated. */
> +static hash_table<string_pair_map_hasher> *path_name_pair_hash_table;
> +
>
> /* Check that tag ACTUAL has one of the given values. NUM_TAGS is the
> number of valid tag values to check. */
> @@ -90,13 +138,216 @@ lto_input_data_block (class lto_input_bl
> buffer[i] = streamer_read_uchar (ib);
> }
>
> +/* Compute the relative path to get to DATA_WD (absolute directory name)
> + from CWD (another absolute directory name). E.g. for
> + DATA_WD of "/tmp/foo/bar" and CWD of "/tmp/baz/qux" return
> + "../../foo/bar". Returned string should be freed by the caller.
> + Return NULL if absolute file name needs to be used. */
> +
> +static char *
> +relative_path_prefix (const char *data_wd, const char *cwd)
> +{
> + const char *d = data_wd;
> + const char *c = cwd;
> +#ifdef HAVE_DOS_BASED_FILE_SYSTEM
> + if (d[1] == ':')
> + {
> + if (!IS_DIR_SEPARATOR (d[2]))
> + return NULL;
> + if (c[0] == d[0] && c[1] == ':' && IS_DIR_SEPARATOR (c[2]))
> + {
> + c += 3;
> + d += 3;
> + }
> + else
> + return NULL;
> + }
> + else if (c[1] == ':')
> + return NULL;
> +#endif
> + do
> + {
> + while (IS_DIR_SEPARATOR (*d))
> + d++;
> + while (IS_DIR_SEPARATOR (*c))
> + c++;
> + size_t i;
> + for (i = 0; c[i] && !IS_DIR_SEPARATOR (c[i]) && c[i] == d[i]; i++)
> + ;
> + if ((c[i] == '\0' || IS_DIR_SEPARATOR (c[i]))
> + && (d[i] == '\0' || IS_DIR_SEPARATOR (d[i])))
> + {
> + c += i;
> + d += i;
> + if (*c == '\0' || *d == '\0')
> + break;
> + }
> + else
> + break;
> + }
> + while (1);
> + size_t num_up = 0;
> + do
> + {
> + while (IS_DIR_SEPARATOR (*c))
> + c++;
> + if (*c == '\0')
> + break;
> + num_up++;
> + while (*c && !IS_DIR_SEPARATOR (*c))
> + c++;
> + }
> + while (1);
> + while (IS_DIR_SEPARATOR (*d))
> + d++;
> + size_t len = strlen (d);
> + if (len == 0 && num_up == 0)
> + return xstrdup (".");
> + char *ret = XNEWVEC (char, num_up * 3 + len + 1);
> + char *p = ret;
> + for (; num_up; num_up--)
> + {
> + const char dir_up[3] = { '.', '.', DIR_SEPARATOR };
> + memcpy (p, dir_up, 3);
> + p += 3;
> + }
> + memcpy (p, d, len + 1);
> + return ret;
> +}
> +
> +/* Look up DATA_WD in hash table of relative prefixes. If found,
> + return relative path from CWD to DATA_WD from the hash table,
> + otherwise create it. */
> +
> +static const char *
> +canon_relative_path_prefix (const char *data_wd, const char *cwd)
> +{
> + if (!IS_ABSOLUTE_PATH (data_wd) || !IS_ABSOLUTE_PATH (cwd))
> + return NULL;
> +
> + if (!path_name_pair_hash_table)
> + {
> + path_name_pair_hash_table
> + = new hash_table<string_pair_map_hasher> (37);
> + string_pair_map_allocator
> + = new object_allocator <struct string_pair_map>
> + ("line map string pair map hash");
> + }
> +
> + inchash::hash h;
> + h.add_ptr (cwd);
> + h.merge_hash (htab_hash_string (data_wd));
> + h.add_int (true);
> +
> + string_pair_map s_slot;
> + s_slot.str1 = cwd;
> + s_slot.str2 = data_wd;
> + s_slot.str3 = NULL;
> + s_slot.hash = h.end ();
> + s_slot.prefix = true;
> +
> + string_pair_map **slot
> + = path_name_pair_hash_table->find_slot (&s_slot, INSERT);
> + if (*slot == NULL)
> + {
> + /* Compute relative path from cwd directory to data_wd directory.
> + E.g. if cwd is /tmp/foo/bar and data_wd is /tmp/baz/qux ,
> + it will return ../../baz/qux . */
> + char *relative_path = relative_path_prefix (data_wd, cwd);
> + const char *relative = relative_path ? relative_path : data_wd;
> + size_t relative_len = strlen (relative);
> + gcc_assert (relative_len);
> +
> + size_t data_wd_len = strlen (data_wd);
> + bool add_separator = false;
> + if (!IS_DIR_SEPARATOR (relative[relative_len - 1]))
> + add_separator = true;
> +
> + size_t len = relative_len + 1 + data_wd_len + 1 + add_separator;
> +
> + char *saved_string = XOBNEWVEC (&file_name_obstack, char, len);
> + struct string_pair_map *new_slot
> + = string_pair_map_allocator->allocate ();
> + memcpy (saved_string, data_wd, data_wd_len + 1);
> + memcpy (saved_string + data_wd_len + 1, relative, relative_len);
> + if (add_separator)
> + saved_string[len - 2] = DIR_SEPARATOR;
> + saved_string[len - 1] = '\0';
> + new_slot->str1 = cwd;
> + new_slot->str2 = saved_string;
> + new_slot->str3 = saved_string + data_wd_len + 1;
> + if (relative_len == 1 && relative[0] == '.')
> + new_slot->str3 = NULL;
> + new_slot->hash = s_slot.hash;
> + new_slot->prefix = true;
> + *slot = new_slot;
> + free (relative_path);
> + return new_slot->str3;
> + }
> + else
> + {
> + string_pair_map *old_slot = *slot;
> + return old_slot->str3;
> + }
> +}
> +
> +/* Look up the pair of RELATIVE_PREFIX and STRING strings in a hash table.
> + If found, return the concatenation of those from the hash table,
> + otherwise concatenate them. */
> +
> +static const char *
> +canon_relative_file_name (const char *relative_prefix, const char *string)
> +{
> + inchash::hash h;
> + h.add_ptr (relative_prefix);
> + h.merge_hash (htab_hash_string (string));
> +
> + string_pair_map s_slot;
> + s_slot.str1 = relative_prefix;
> + s_slot.str2 = string;
> + s_slot.str3 = NULL;
> + s_slot.hash = h.end ();
> + s_slot.prefix = false;
> +
> + string_pair_map **slot
> + = path_name_pair_hash_table->find_slot (&s_slot, INSERT);
> + if (*slot == NULL)
> + {
> + size_t relative_prefix_len = strlen (relative_prefix);
> + size_t string_len = strlen (string);
> + size_t len = relative_prefix_len + string_len + 1;
> +
> + char *saved_string = XOBNEWVEC (&file_name_obstack, char, len);
> + struct string_pair_map *new_slot
> + = string_pair_map_allocator->allocate ();
> + memcpy (saved_string, relative_prefix, relative_prefix_len);
> + memcpy (saved_string + relative_prefix_len, string, string_len + 1);
> + new_slot->str1 = relative_prefix;
> + new_slot->str2 = saved_string + relative_prefix_len;
> + new_slot->str3 = saved_string;
> + new_slot->hash = s_slot.hash;
> + new_slot->prefix = false;
> + *slot = new_slot;
> + return new_slot->str3;
> + }
> + else
> + {
> + string_pair_map *old_slot = *slot;
> + return old_slot->str3;
> + }
> +}
>
> /* Lookup STRING in file_name_hash_table. If found, return the existing
> - string, otherwise insert STRING as the canonical version. */
> + string, otherwise insert STRING as the canonical version.
> + If STRING is a relative pathname and RELATIVE_PREFIX is non-NULL, use
> + canon_relative_file_name instead. */
>
> static const char *
> -canon_file_name (const char *string)
> +canon_file_name (const char *relative_prefix, const char *string)
> {
> + if (relative_prefix && !IS_ABSOLUTE_PATH (string))
> + return canon_relative_file_name (relative_prefix, string);
> +
> string_slot **slot;
> struct string_slot s_slot;
> size_t len = strlen (string);
> @@ -261,10 +512,12 @@ lto_location_cache::input_location_and_b
> static int stream_col;
> static bool stream_sysp;
> static tree stream_block;
> + static const char *stream_relative_path_prefix;
>
> gcc_assert (current_cache == this);
>
> - *loc = bp_unpack_int_in_range (bp, "location", 0, RESERVED_LOCATION_COUNT);
> + *loc = bp_unpack_int_in_range (bp, "location", 0,
> + RESERVED_LOCATION_COUNT + 1);
>
> if (*loc < RESERVED_LOCATION_COUNT)
> {
> @@ -279,16 +532,28 @@ lto_location_cache::input_location_and_b
> return;
> }
>
> + bool file_change = (*loc == RESERVED_LOCATION_COUNT + 1);
> /* Keep value RESERVED_LOCATION_COUNT in *loc as linemap lookups will
> ICE on it. */
> -
> - bool file_change = bp_unpack_value (bp, 1);
> + *loc = RESERVED_LOCATION_COUNT;
> bool line_change = bp_unpack_value (bp, 1);
> bool column_change = bp_unpack_value (bp, 1);
>
> if (file_change)
> {
> - stream_file = canon_file_name (bp_unpack_string (data_in, bp));
> + bool pwd_change = bp_unpack_value (bp, 1);
> + if (pwd_change)
> + {
> + const char *pwd = bp_unpack_string (data_in, bp);
> + const char *src_pwd = get_src_pwd ();
> + if (strcmp (pwd, src_pwd) == 0)
> + stream_relative_path_prefix = NULL;
> + else
> + stream_relative_path_prefix
> + = canon_relative_path_prefix (pwd, src_pwd);
> + }
> + stream_file = canon_file_name (stream_relative_path_prefix,
> + bp_unpack_string (data_in, bp));
> stream_sysp = bp_unpack_value (bp, 1);
> }
>
> @@ -1857,6 +2122,10 @@ lto_free_file_name_hash (void)
> file_name_hash_table = NULL;
> delete string_slot_allocator;
> string_slot_allocator = NULL;
> + delete path_name_pair_hash_table;
> + path_name_pair_hash_table = NULL;
> + delete string_pair_map_allocator;
> + string_pair_map_allocator = NULL;
> /* file_name_obstack must stay allocated since it is referred to by
> line map table. */
> }
>
> Jakub
>
