From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id 9xGCAhBPpmHUbAAAWB0awg (envelope-from ) for ; Tue, 30 Nov 2021 11:19:28 -0500 Received: by simark.ca (Postfix, from userid 112) id EAEAA1F0CE; Tue, 30 Nov 2021 11:19:27 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,RDNS_DYNAMIC,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id A587A1EDF0 for ; Tue, 30 Nov 2021 11:19:26 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id C6464385AC3A for ; Tue, 30 Nov 2021 16:19:25 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org C6464385AC3A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1638289165; bh=mftWRIxPorwDKAwZWxm9Wr9rrLFv8ZbHg7Fw/MieIrE=; h=Date:To:Subject:References:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=LyhzdrTsWPah0Kf4dMR2ExfQfAWIGlnp4eYRjaCXFFm2jRr3QMwI9cy0pJkz6lhdf fMNnhaFXkKxNkTXJ2i3j35UgJYNk4nxurOLJEQja4gqLpSZPoGNOPYJLpUEeuesQX9 WbDMegMTNNv/2jGQGbEsPNwyRpBZg19GVfLlUgP4= Received: from lndn.lancelotsix.com (vps-42846194.vps.ovh.net [IPv6:2001:41d0:801:2000::2400]) by sourceware.org (Postfix) with ESMTPS id CD268385BF92 for ; Tue, 30 Nov 2021 16:18:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org CD268385BF92 Received: from Plymouth (unknown [IPv6:2a02:390:9086:0:7ba7:dea0:4aa1:7c8d]) by lndn.lancelotsix.com (Postfix) with ESMTPSA id 856D68198D; Tue, 30 Nov 2021 16:18:23 +0000 (UTC) Date: Tue, 30 Nov 2021 16:18:18 +0000 To: Tom Tromey Subject: Re: [PATCH 2/3] Move file_and_directory to new file and C++-ize Message-ID: <20211130161818.m7hlyoebsqasfo6y@Plymouth> References: <20211130013304.4135367-1-tom@tromey.com> <20211130013304.4135367-3-tom@tromey.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20211130013304.4135367-3-tom@tromey.com> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.5.11 (lndn.lancelotsix.com [0.0.0.0]); Tue, 30 Nov 2021 16:18:23 +0000 (UTC) X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Lancelot SIX via Gdb-patches Reply-To: Lancelot SIX Cc: gdb-patches@sourceware.org Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" Hi, I have some remarks below. On Mon, Nov 29, 2021 at 06:33:03PM -0700, Tom Tromey wrote: > This moves file_and_directory to a new file, and then C++-izes it -- > replacing direct assignments with methods, and arranging for it to own > any string that must be computed. Finally, the CU's objfile will only > be used on demand; this is an important property for the new DWARF > indexer's parallel mode. > --- > gdb/dwarf2/file-and-dir.h | 107 ++++++++++++++++++++++++++++++++++++++ > gdb/dwarf2/read.c | 66 ++++++++--------------- > gdb/dwarf2/read.h | 1 + > 3 files changed, 129 insertions(+), 45 deletions(-) > create mode 100644 gdb/dwarf2/file-and-dir.h > > diff --git a/gdb/dwarf2/file-and-dir.h b/gdb/dwarf2/file-and-dir.h > new file mode 100644 > index 00000000000..64d5a08bf0c > --- /dev/null > +++ b/gdb/dwarf2/file-and-dir.h > @@ -0,0 +1,107 @@ > +/* DWARF file and directory > + > + Copyright (C) 1994-2021 Free Software Foundation, Inc. > + > + Adapted by Gary Funck (gary@intrepid.com), Intrepid Technology, > + Inc. with support from Florida State University (under contract > + with the Ada Joint Program Office), and Silicon Graphics, Inc. > + Initial contribution by Brent Benson, Harris Computer Systems, Inc., > + based on Fred Fish's (Cygnus Support) implementation of DWARF 1 > + support. > + > + This file is part of GDB. > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 3 of the License, or > + (at your option) any later version. > + > + This program is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program. If not, see . */ > + > +#ifndef GDB_DWARF2_FILE_AND_DIR_H > +#define GDB_DWARF2_FILE_AND_DIR_H > + > +#include "objfiles.h" > +#include > + > +/* The return type of find_file_and_directory. Note, the enclosed > + string pointers are only valid while this object is valid. */ > + > +struct file_and_directory > +{ > + file_and_directory (const char *name_, const char *dir_) > + : name (name_), > + comp_dir (dir_) The members are usually prefixed with 'm_'. I think you change that in then next patch, but I feel this change would be relevant in the 'c++ization' done by the current one. > + { > + } > + > + /* Return true if the file name is unknown. */ > + bool is_unknown () const > + { > + return name == nullptr; > + } > + > + /* Set the compilation directory. */ > + void set_comp_dir (std::string &&dir) > + { > + comp_dir_storage = std::move (dir); > + comp_dir = nullptr; > + } > + > + /* Fetch the compilation directory. This may return NULL in some > + circumstances. Note that the return value here is not stable -- > + it may change if this object is moved. To get a stable pointer, > + you should call intern_comp_dir. */ > + const char *get_comp_dir () const > + { > + if (!comp_dir_storage.empty ()) > + return comp_dir_storage.c_str (); > + return comp_dir; > + } > + > + /* If necessary, intern the compilation directory using OBJFILE's > + string cache. Returns the compilation directory. */ > + const char *intern_comp_dir (struct objfile *objfile) > + { > + if (!comp_dir_storage.empty ()) > + { > + comp_dir = objfile->intern (comp_dir_storage); > + comp_dir_storage.clear (); > + } > + return comp_dir; > + } > + > + /* Fetch the filename. This never returns NULL. */ > + const char *get_name () const > + { > + return name == nullptr ? "" : name; > + } > + > + /* Set the filename. */ > + void set_name (const char *name_) > + { > + name = name_; > + } > + > +private: > + > + /* The filename. This is never NULL. */ >From how the class is implemented, it looks like this comment is not accurate anymore and NAME can be NULL. Best, Lancelot. > + const char *name; > + > + /* The compilation directory. NULL if not known. If we needed to > + compute a new string, it will be stored in the comp_dir_storage > + member, and this will be NULL. Otherwise, points directly to the > + DW_AT_comp_dir string attribute. */ > + const char *comp_dir; > + > + /* The compilation directory, if it needed to be allocated. */ > + std::string comp_dir_storage; > +}; > + > +#endif /* GDB_DWARF2_FILE_AND_DIR_H */ > diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c > index 3cf0c9ea2a8..5802fc0ef04 100644 > --- a/gdb/dwarf2/read.c > +++ b/gdb/dwarf2/read.c > @@ -1206,7 +1206,6 @@ static struct die_info *die_specification (struct die_info *die, > static line_header_up dwarf_decode_line_header (sect_offset sect_off, > struct dwarf2_cu *cu); > > -struct file_and_directory; > static void dwarf_decode_lines (struct line_header *, > const file_and_directory &, > struct dwarf2_cu *, dwarf2_psymtab *, > @@ -1538,21 +1537,6 @@ dwarf2_per_cu_data_deleter::operator() (dwarf2_per_cu_data *data) > delete data; > } > > -/* The return type of find_file_and_directory. Note, the enclosed > - string pointers are only valid while this object is valid. */ > - > -struct file_and_directory > -{ > - /* The filename. This is never NULL. */ > - const char *name; > - > - /* The compilation directory. NULL if not known. If we needed to > - compute a new string, it will be stored in the per-BFD string > - bcache; otherwise, points directly to the DW_AT_comp_dir string > - attribute owned by the obstack that owns the DIE. */ > - const char *comp_dir; > -}; > - > static file_and_directory find_file_and_directory (struct die_info *die, > struct dwarf2_cu *cu); > > @@ -3024,7 +3008,7 @@ dw2_get_file_names_reader (const struct die_reader_specs *reader, > file_and_directory fnd = find_file_and_directory (comp_unit_die, cu); > > int offset = 0; > - if (strcmp (fnd.name, "") != 0) > + if (fnd.is_unknown ()) > ++offset; > else if (lh == nullptr) > return; > @@ -3053,12 +3037,12 @@ dw2_get_file_names_reader (const struct die_reader_specs *reader, > } > > qfn->num_file_names = offset + include_names.size (); > - qfn->comp_dir = fnd.comp_dir; > + qfn->comp_dir = fnd.intern_comp_dir (per_objfile->objfile); > qfn->file_names = > XOBNEWVEC (&per_objfile->per_bfd->obstack, const char *, > qfn->num_file_names); > if (offset != 0) > - qfn->file_names[0] = xstrdup (fnd.name); > + qfn->file_names[0] = xstrdup (fnd.get_name ()); > > if (!include_names.empty ()) > memcpy (&qfn->file_names[offset], include_names.data (), > @@ -7005,15 +6989,15 @@ process_psymtab_comp_unit_reader (const struct die_reader_specs *reader, > gdb::unique_xmalloc_ptr debug_filename; > static const char artificial[] = ""; > file_and_directory fnd = find_file_and_directory (comp_unit_die, cu); > - if (strcmp (fnd.name, artificial) == 0) > + if (strcmp (fnd.get_name (), artificial) == 0) > { > debug_filename.reset (concat (artificial, "@", > sect_offset_str (per_cu->sect_off), > (char *) NULL)); > - fnd.name = debug_filename.get (); > + fnd.set_name (debug_filename.get ()); > } > > - pst = create_partial_symtab (per_cu, per_objfile, fnd.name); > + pst = create_partial_symtab (per_cu, per_objfile, fnd.get_name ()); > > /* This must be done before calling dwarf2_build_include_psymtabs. */ > pst->dirname = dwarf2_string_attr (comp_unit_die, DW_AT_comp_dir, cu); > @@ -10493,25 +10477,16 @@ producer_is_gcc_lt_4_3 (struct dwarf2_cu *cu) > static file_and_directory > find_file_and_directory (struct die_info *die, struct dwarf2_cu *cu) > { > - file_and_directory res; > - > /* Find the filename. Do not use dwarf2_name here, since the filename > is not a source language identifier. */ > - res.name = dwarf2_string_attr (die, DW_AT_name, cu); > - res.comp_dir = dwarf2_string_attr (die, DW_AT_comp_dir, cu); > - > - if (res.comp_dir == NULL > - && producer_is_gcc_lt_4_3 (cu) && res.name != NULL > - && IS_ABSOLUTE_PATH (res.name)) > - { > - std::string comp_dir_storage = ldirname (res.name); > - if (!comp_dir_storage.empty ()) > - res.comp_dir > - = cu->per_objfile->objfile->intern (comp_dir_storage.c_str ()); > - } > + file_and_directory res (dwarf2_string_attr (die, DW_AT_name, cu), > + dwarf2_string_attr (die, DW_AT_comp_dir, cu)); > > - if (res.name == NULL) > - res.name = ""; > + if (res.get_comp_dir () == nullptr > + && producer_is_gcc_lt_4_3 (cu) > + && res.get_name () != nullptr > + && IS_ABSOLUTE_PATH (res.get_name ())) > + res.set_comp_dir (ldirname (res.get_name ())); > > return res; > } > @@ -10643,7 +10618,7 @@ read_file_scope (struct die_info *die, struct dwarf2_cu *cu) > > file_and_directory fnd = find_file_and_directory (die, cu); > > - cu->start_symtab (fnd.name, fnd.comp_dir, lowpc); > + cu->start_symtab (fnd.get_name (), fnd.intern_comp_dir (objfile), lowpc); > > gdb_assert (per_objfile->sym_cu == nullptr); > scoped_restore restore_sym_cu > @@ -20752,7 +20727,7 @@ compute_include_file_name (const struct line_header *lh, const file_entry &fe, > > gdb::unique_xmalloc_ptr hold_compare; > if (!IS_ABSOLUTE_PATH (include_name) > - && (dir_name != NULL || cu_info.comp_dir != NULL)) > + && (dir_name != nullptr || cu_info.get_comp_dir () != nullptr)) > { > /* Avoid creating a duplicate name for CU_INFO. > We do this by comparing INCLUDE_NAME and CU_INFO. > @@ -20782,19 +20757,20 @@ compute_include_file_name (const struct line_header *lh, const file_entry &fe, > include_name = name_holder->get (); > include_name_to_compare = include_name; > } > - if (!IS_ABSOLUTE_PATH (include_name) && cu_info.comp_dir != nullptr) > + if (!IS_ABSOLUTE_PATH (include_name) > + && cu_info.get_comp_dir () != nullptr) > { > - hold_compare.reset (concat (cu_info.comp_dir, SLASH_STRING, > + hold_compare.reset (concat (cu_info.get_comp_dir (), SLASH_STRING, > include_name, (char *) NULL)); > include_name_to_compare = hold_compare.get (); > } > } > > gdb::unique_xmalloc_ptr copied_name; > - const char *cu_filename = cu_info.name; > - if (!IS_ABSOLUTE_PATH (cu_filename) && cu_info.comp_dir != nullptr) > + const char *cu_filename = cu_info.get_name (); > + if (!IS_ABSOLUTE_PATH (cu_filename) && cu_info.get_comp_dir () != nullptr) > { > - copied_name.reset (concat (cu_info.comp_dir, SLASH_STRING, > + copied_name.reset (concat (cu_info.get_comp_dir (), SLASH_STRING, > cu_filename, (char *) NULL)); > cu_filename = copied_name.get (); > } > diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h > index fe34e3f95ae..0afcda1c3b0 100644 > --- a/gdb/dwarf2/read.h > +++ b/gdb/dwarf2/read.h > @@ -23,6 +23,7 @@ > #include > #include > #include "dwarf2/comp-unit-head.h" > +#include "dwarf2/file-and-dir.h" > #include "dwarf2/index-cache.h" > #include "dwarf2/section.h" > #include "filename-seen-cache.h" > -- > 2.31.1 > -- Lancelot SIX