From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x331.google.com (mail-wm1-x331.google.com [IPv6:2a00:1450:4864:20::331]) by sourceware.org (Postfix) with ESMTPS id 78468384D0D7 for ; Tue, 15 Nov 2022 01:35:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 78468384D0D7 Authentication-Results: sourceware.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=google.com Received: by mail-wm1-x331.google.com with SMTP id t4so8704506wmj.5 for ; Mon, 14 Nov 2022 17:35:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=SIQ+7vZWMSADZnSwCJZMRtq7JjXA6DvwMzLROXAg8Fw=; b=FsQcSxq/jqmq5uvepvahoUYTPWtx6LTdJzE3nTI0mAqOVdUtflnn99IH5CS405cA/1 i0h0yjaQB6rOJpjFQ1De0y+F1RmXNoDw3irL+wpsIcwpByxcvoTOmHoDWPjpRssVQB19 jXWCY7OZy3uEqPwFDLdODRNfNRVRSPsxMZ/zrGwzKQ2WFHwGb8w+N5odkGJF2CqDQyhn dKEjNLvVOaqFA0kmfz2nZAvbSm6HxXBGPNgwJUlyKriFuOV4wYSZz/+B+MFXPOL/c9i4 M57j6cse+8hkcpcc6HvjdWs7u/FvdY43bf+N9aErdOEOS2lLQiePdzJn019Ark3yx6nv U5dA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=SIQ+7vZWMSADZnSwCJZMRtq7JjXA6DvwMzLROXAg8Fw=; b=M5CqPAEZ42KZ9lNj8epVYro0v+wGSLhHRZ82UPvbkgEJuVlipiFqHzr7gilHsfclBK 5Rq1KBMr5pFlcLH76GmD75SSs689DRk1J9/tJNEdwlD8mvgttcIL1ki3Y5np0vghB+GY ah7QwG4xx4M9plte1NKdc4ev3M72XBU7rSuXe2NbdUzyJhnwpbQt6bS7d4MCcUam8a6l Sc/dyCLTwp4L+L2yYwXGTmRRKL3LoN+8ePfAC4qN1teCtAH42XXoD7grhngjje2Pf2Gq PPdLKbbrG2T+9yF5qJEc4kaGFaosc7WurWiPihhvdNOO4alyWCBrQg4sPAusai6dgHIv v59Q== X-Gm-Message-State: ANoB5pkb2LmL7d33CuVcwZD0yBuYzuEwYgMZqcsPlQyavQIptxzX+MhA FA/Tc/Bbal1VT2JBchRiJgZa5P8axc5xyWlI27wsXA== X-Google-Smtp-Source: AA0mqf6d9DJrWRnjhsFcACjnwzOV2+R6mSmUCdAm603mnboDp7Ihoww3t99Qs2IpSeGIUFo0jXzfnPSNOYVqVNWaLjs= X-Received: by 2002:a05:600c:2215:b0:3cf:55bd:4944 with SMTP id z21-20020a05600c221500b003cf55bd4944mr9499996wml.64.1668476129033; Mon, 14 Nov 2022 17:35:29 -0800 (PST) MIME-Version: 1.0 References: <20200910083811.GU18149@tucnak> In-Reply-To: <20200910083811.GU18149@tucnak> From: Ian Lance Taylor Date: Mon, 14 Nov 2022 17:35:16 -0800 Message-ID: Subject: Re: [PATCH] lto: Stream current working directory for first streamed relative filename and adjust relative paths [PR93865] To: Jakub Jelinek Cc: Richard Biener , gcc-patches@gcc.gnu.org Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-17.4 required=5.0 tests=BAYES_00,DKIMWL_WL_MED,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,ENV_AND_HDR_SPF_MATCH,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,USER_IN_DEF_DKIM_WL,USER_IN_DEF_SPF_WL autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Thu, Sep 10, 2020 at 1:39 AM Jakub Jelinek via Gcc-patches 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 > > 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 *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 >