From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id 8B3BD3857806 for ; Thu, 10 Sep 2020 09:17:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 8B3BD3857806 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=rguenther@suse.de X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id EA413AF47; Thu, 10 Sep 2020 09:17:43 +0000 (UTC) Date: Thu, 10 Sep 2020 11:17:28 +0200 (CEST) From: Richard Biener To: Jakub Jelinek cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] lto: Stream current working directory for first streamed relative filename and adjust relative paths [PR93865] In-Reply-To: <20200910083811.GU18149@tucnak> Message-ID: References: <20200910083811.GU18149@tucnak> User-Agent: Alpine 2.21 (LSU 202 2017-01-01) MIME-Version: 1.0 X-Spam-Status: No, score=-5.1 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 8BIT X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 10 Sep 2020 09:17:32 -0000 On Thu, 10 Sep 2020, Jakub Jelinek wrote: > Hi! > > 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 > > PR debug/93865 > * lto-streamer.h (struct output_block): Add emit_pad member. emit_pwd. OK. Thanks for working on these issues, Richard. > * 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. > > --- 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 *string_slot_allocator; > @@ -50,11 +51,58 @@ static struct object_allocator /* The table to hold the file names. */ > static hash_table *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 > + *string_pair_map_allocator; > + > +struct string_pair_map_hasher : nofree_ptr_hash > +{ > + 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 *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 (37); > + string_pair_map_allocator > + = new object_allocator > + ("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 > > -- Richard Biener SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)