From patchwork Fri Sep 6 21:52:49 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "Terekhov, Mikhail via Gdb-patches" X-Patchwork-Id: 34416 Received: (qmail 64994 invoked by alias); 6 Sep 2019 21:52:59 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 64983 invoked by uid 89); 6 Sep 2019 21:52:59 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-23.8 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=titles, *slot, D*ca, attaching X-HELO: mail-pl1-f202.google.com Received: from mail-pl1-f202.google.com (HELO mail-pl1-f202.google.com) (209.85.214.202) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 06 Sep 2019 21:52:56 +0000 Received: by mail-pl1-f202.google.com with SMTP id x5so4313418pln.5 for ; Fri, 06 Sep 2019 14:52:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc:content-transfer-encoding; bh=nuWb23wcrD14xrqykj6/ZPNKKm7tvkQr5IfSNUnG1Sc=; b=IvZwPcSm428M66vdynJXdNCJRWKFVFsgVEPYVDb9wE9R9LUy/xvATN/OWAcLoF/6nT N8plbKmnnLgtulltbP57pfM53laiC7meyJLUwwVc81tkfqg1bSj3dUn6anaYZYvbOu2v YtyPzwPk4I/BCx3OI/IhUX9x3egEUCHP7RPBYC90DaUFslWQVJ5yvmV2Ih1vfHA/JRkD 8B48dLYHmKHWv0hmMHtin2VAhOS+CxVPIJ7qJW44D1609uK+Bg/hJo/Ms74wAoHJElQh 4Y+eRxynyKEE9vtqIFmKMAexzf9DsFqg+1RNvkH+YNyz36lZKWWoc6a2neyt+KqsE1hG C6Vg== Date: Fri, 6 Sep 2019 14:52:49 -0700 In-Reply-To: Message-Id: <20190906215249.161246-1-tamur@google.com> Mime-Version: 1.0 References: Subject: [PATCH v2 1/4] DWARF 5 support: Handle dwo_id X-Patchwork-Original-From: "Ali Tamur via gdb-patches" From: "Terekhov, Mikhail via Gdb-patches" Reply-To: Ali Tamur To: gdb-patches@sourceware.org Cc: Ali Tamur X-IsSubscribed: yes On Tue, Aug 27, 2019 at 10:15 PM Simon Marchi wrote: > Thanks for splitting your patches.  In addition to having separate patches, please > name each patch (git commit) to reflect what it does.  Otherwise we'll get 4 commits > named "Increasing support for dwarf 5", not super clear. Done. > In commit titles / commit messages / comments, can you please use "DWARF" instead of > "dwarf", since that is the official spelling? Done (but only in the lines I changed). > Could you also precise (here and in the commit message) how you have tested this?  Which > compiler, which version, which test case? I compiled 1) synced master, and 2) that plus this patch. I used gcc as the   compiler. I ran the gdb tests with "make check" and the same set of tests     failed in each case. Until it comes to a point where gdb actually starts to   handle 'hello world' compiled for DWARF 5, I don't know if I can do better.   > > +     DW_UT_skeleton, DW_UT_split_compile also contain a signature.  */ > >    ULONGEST signature; > > Does DW_UT_partial contain a signature?  In section 7.5.1.1 of the DWARF5 spec, it doesn't seem so. My bad, corrected the comment. (Code needed no change). > I think the code will be confusing if we keep the field named simply "signature", when it could > actually mean "dwo_id".  Somebody reading the code with the DWARF 5 spec on the side will have some > trouble making the correlation between both.  Maybe we should start using a union to describe that > different fields are used with different kinds of units? Please see the definition of struct dwo_unit, 'signature' is already used     there. Renamed the field as "signature_or_dwo_id". If that is ok with you,     I'd rather not introduce a union; the code is already complicated enough and   I think it's not a good idea to mix such refactoring with behaviour changes   in the same patch.   > There's a little discrepancy in this error message, it would show up as: > >   (is 1, should be DW_UT_type) > > Since we know it's an existing DW_UT value here, I'd suggest we print it as a > string, or even both: > >   (is DW_UT_skeleton (0x4), should be DW_UT_type (0x2)) > > We would just need a small utility function that converts numerical DW_UT value to string. Done. > Here, it's fine to not print the stringified version of the value, since it's an > invalid value.  But could make the message appear like: > >   (is 0xff, should be one of: DW_UT_compile (0x1), DW_UT_skeleton (0x4), ...) Done.  > +lookup_signature (struct dwarf2_cu *cu, struct die_info* comp_unit_die) > > Again here, since the DWARF 5 standard talks about dwo_id, not signature, wouldn't it make > more sense for us to use the term dwo_id?  I renamed the function as lookup_dwo_id. > Wouldn't it make sense here to check the DWARF version from the cu_header and > take an action based on that? Done. > Even though we're unlikely to have a signature / dwo_id that is 0, I would > prefer if we didn't use it as a special value to denote failure.  The function > should return a boolean, and the signature / dwo_id would be returned as an > output parameter. Done. > Here too, should we do something based on the CU's DWARF version?  I presume we don't want > to accept a DW_AT_GNU_dwo_name attribute in a DWARF 5 CU.  clang (and maybe others) still uses DW_AT_GNU_dwo_name in DWARF 5. Thanks for the detailed review. I am attaching the updated commit message, Changelog and changes below. --- * DW_UT_skeleton and DW_UT_split_compile compilation units have dwo ids to match the compilation unit in the skeleton and .dwo files. The dwo_id is in the header. Tested with CC=/usr/bin/gcc against master and there was no increase in the set of tests that fails. This is part of an effort to support DWARF 5 in gdb. gdb/ChangeLog: * gdb/dwarf2read.c (comp_unit_head): Rename field and update comment. (dwarf2_dwo_name): New function declaration. (dwarf_unit_type_name): New function declaration. (create_signatured_type_table_from_debug_names): Reflect name change. (read_comp_unit_head): Add support for new compilation units, DW_UT_partial, DW_UT_skeleton, DW_UT_split_compile, DW_UT_split_type. Particularly, DW_UT_skeleton and DW_UT_split_compile have dwo_id in their header. Also clarify error messages. (create_debug_type_hash_table): Reflect field name change. (read_cutu_die_from_dwo): Likewise. (lookup_dwo_id): New function. Returns the dwo id of the given compile unit. (lookup_dwo_unit): API change. Use the new lookup_dwo_id function. (init_cutu_and_read_dies): Use the new dwarf2_dwo_name and lookup_dwo_id functions. (read_comp_units_from_section): Reflect field name change. (create_dwo_cu_reader): Use the added lookup_dwo_id function. (dwarf2_dwo_name): Get the dwo name if present. (dwarf_unit_type_name): Convert DW_UT_* types to string for diagnostic purposes. --- gdb/dwarf2read.c | 149 +++++++++++++++++++++++++++++++++-------------- 1 file changed, 104 insertions(+), 45 deletions(-) diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index fb888da7b8..e0697ff8c4 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -373,9 +373,12 @@ struct comp_unit_head This will be the first byte following the compilation unit header. */ cu_offset first_die_cu_offset; - /* 64-bit signature of this type unit - it is valid only for - UNIT_TYPE DW_UT_type. */ - ULONGEST signature; + + /* 64-bit signature of this unit. For type units, it denotes the signature of + the type (DW_UT_type in DWARF 4, additionally DW_UT_split_type in DWARF 5). + Also used in DWARF 5, to denote the dwo id when the unit type is + DW_UT_skeleton or DW_UT_split_compile. */ + ULONGEST signature_or_dwo_id; /* For types, offset in the type's DIE of the type defined by this TU. */ cu_offset type_cu_offset_in_tu; @@ -1579,6 +1582,8 @@ static struct attribute *dwarf2_attr_no_follow (struct die_info *, static const char *dwarf2_string_attr (struct die_info *die, unsigned int name, struct dwarf2_cu *cu); +static const char *dwarf2_dwo_name (struct die_info *die, struct dwarf2_cu *cu); + static int dwarf2_flag_true_p (struct die_info *die, unsigned name, struct dwarf2_cu *cu); @@ -1761,6 +1766,8 @@ static const char *dwarf_tag_name (unsigned int); static const char *dwarf_attr_name (unsigned int); +static const char *dwarf_unit_type_name(int unit_type); + static const char *dwarf_form_name (unsigned int); static const char *dwarf_bool_name (unsigned int); @@ -3105,7 +3112,7 @@ create_signatured_type_table_from_debug_names sig_type = OBSTACK_ZALLOC (&objfile->objfile_obstack, struct signatured_type); - sig_type->signature = cu_header.signature; + sig_type->signature = cu_header.signature_or_dwo_id; sig_type->type_offset_in_tu = cu_header.type_cu_offset_in_tu; sig_type->per_cu.is_debug_types = 1; sig_type->per_cu.section = section; @@ -6390,18 +6397,27 @@ read_comp_unit_head (struct comp_unit_head *cu_header, switch (cu_header->unit_type) { case DW_UT_compile: + case DW_UT_partial: + case DW_UT_skeleton: + case DW_UT_split_compile: if (section_kind != rcuh_kind::COMPILE) error (_("Dwarf Error: wrong unit_type in compilation unit header " - "(is DW_UT_compile, should be DW_UT_type) [in module %s]"), - filename); + "(is %s, should be DW_UT_type) [in module %s]"), + dwarf_unit_type_name(cu_header->unit_type), filename); break; case DW_UT_type: + case DW_UT_split_type: section_kind = rcuh_kind::TYPE; break; default: error (_("Dwarf Error: wrong unit_type in compilation unit header " - "(is %d, should be %d or %d) [in module %s]"), - cu_header->unit_type, DW_UT_compile, DW_UT_type, filename); + "(is %#04x, should be one of: %s, %s, %s, %s or %s) " + "[in module %s]"), cu_header->unit_type, + dwarf_unit_type_name(DW_UT_compile), + dwarf_unit_type_name(DW_UT_skeleton), + dwarf_unit_type_name(DW_UT_split_compile), + dwarf_unit_type_name(DW_UT_type), + dwarf_unit_type_name(DW_UT_split_type), filename); } cu_header->addr_size = read_1_byte (abfd, info_ptr); @@ -6422,13 +6438,19 @@ read_comp_unit_head (struct comp_unit_head *cu_header, _("read_comp_unit_head: dwarf from non elf file")); cu_header->signed_addr_p = signed_addr; - if (section_kind == rcuh_kind::TYPE) - { - LONGEST type_offset; + bool header_has_signature_or_dwo_id = section_kind == rcuh_kind::TYPE + || cu_header->unit_type == DW_UT_skeleton + || cu_header->unit_type == DW_UT_split_compile; - cu_header->signature = read_8_bytes (abfd, info_ptr); + if (header_has_signature_or_dwo_id) + { + cu_header->signature_or_dwo_id = read_8_bytes (abfd, info_ptr); info_ptr += 8; + } + if (section_kind == rcuh_kind::TYPE) + { + LONGEST type_offset; type_offset = read_offset (abfd, info_ptr, cu_header, &bytes_read); info_ptr += bytes_read; cu_header->type_cu_offset_in_tu = (cu_offset) type_offset; @@ -6694,7 +6716,7 @@ create_debug_type_hash_table (struct dwarf2_per_objfile *dwarf2_per_objfile, sect_offset sect_off = (sect_offset) (ptr - section->buffer); /* Initialize it due to a false compiler warning. */ - header.signature = -1; + header.signature_or_dwo_id = -1; header.type_cu_offset_in_tu = (cu_offset) -1; /* We need to read the type's signature in order to build the hash @@ -6728,7 +6750,7 @@ create_debug_type_hash_table (struct dwarf2_per_objfile *dwarf2_per_objfile, dwo_tu = OBSTACK_ZALLOC (&objfile->objfile_obstack, struct dwo_unit); dwo_tu->dwo_file = dwo_file; - dwo_tu->signature = header.signature; + dwo_tu->signature = header.signature_or_dwo_id; dwo_tu->type_offset_in_tu = header.type_cu_offset_in_tu; dwo_tu->section = section; dwo_tu->sect_off = sect_off; @@ -6741,7 +6763,7 @@ create_debug_type_hash_table (struct dwarf2_per_objfile *dwarf2_per_objfile, dwo_tu = NULL; sig_type = OBSTACK_ZALLOC (&objfile->objfile_obstack, struct signatured_type); - sig_type->signature = header.signature; + sig_type->signature = header.signature_or_dwo_id; sig_type->type_offset_in_tu = header.type_cu_offset_in_tu; sig_type->per_cu.dwarf2_per_objfile = dwarf2_per_objfile; sig_type->per_cu.is_debug_types = 1; @@ -6776,14 +6798,14 @@ create_debug_type_hash_table (struct dwarf2_per_objfile *dwarf2_per_objfile, complaint (_("debug type entry at offset %s is duplicate to" " the entry at offset %s, signature %s"), sect_offset_str (sect_off), sect_offset_str (dup_sect_off), - hex_string (header.signature)); + hex_string (header.signature_or_dwo_id)); } *slot = dwo_file ? (void *) dwo_tu : (void *) sig_type; if (dwarf_read_debug > 1) fprintf_unfiltered (gdb_stdlog, " offset %s, signature %s\n", sect_offset_str (sect_off), - hex_string (header.signature)); + hex_string (header.signature_or_dwo_id)); info_ptr += length; } @@ -7206,12 +7228,12 @@ read_cutu_die_from_dwo (struct dwarf2_per_cu_data *this_cu, dwo_abbrev_section, info_ptr, rcuh_kind::TYPE); /* This is not an assert because it can be caused by bad debug info. */ - if (sig_type->signature != cu->header.signature) + if (sig_type->signature != cu->header.signature_or_dwo_id) { error (_("Dwarf Error: signature mismatch %s vs %s while reading" " TU at offset %s [in module %s]"), hex_string (sig_type->signature), - hex_string (cu->header.signature), + hex_string (cu->header.signature_or_dwo_id), sect_offset_str (dwo_unit->sect_off), bfd_get_filename (abfd)); } @@ -7297,47 +7319,58 @@ read_cutu_die_from_dwo (struct dwarf2_per_cu_data *this_cu, return 1; } +/* Return the signature of the compile unit. In DWARF 4 and before, the + signature is in the DW_AT_GNU_dwo_id attribute. In DWARF 5 and later, the + signature is part of the header. Returns 0 if the signature is not found. */ +static gdb::optional +lookup_dwo_id (struct dwarf2_cu *cu, struct die_info* comp_unit_die) +{ + if (cu->header.version >= 5) + return cu->header.signature_or_dwo_id; + struct attribute *attr; + attr = dwarf2_attr (comp_unit_die, DW_AT_GNU_dwo_id, cu); + if (!attr) + return gdb::optional(); + return DW_UNSND (attr); +} + /* Subroutine of init_cutu_and_read_dies to simplify it. Look up the DWO unit specified by COMP_UNIT_DIE of THIS_CU. Returns NULL if the specified DWO unit cannot be found. */ static struct dwo_unit * -lookup_dwo_unit (struct dwarf2_per_cu_data *this_cu, +lookup_dwo_unit (const struct die_reader_specs *reader, struct die_info *comp_unit_die) { - struct dwarf2_cu *cu = this_cu->cu; - ULONGEST signature; + struct dwarf2_cu *cu = reader->cu; + struct dwarf2_per_cu_data *per_cu = cu->per_cu; struct dwo_unit *dwo_unit; const char *comp_dir, *dwo_name; gdb_assert (cu != NULL); /* Yeah, we look dwo_name up again, but it simplifies the code. */ - dwo_name = dwarf2_string_attr (comp_unit_die, DW_AT_GNU_dwo_name, cu); + dwo_name = dwarf2_dwo_name (comp_unit_die, cu); comp_dir = dwarf2_string_attr (comp_unit_die, DW_AT_comp_dir, cu); - if (this_cu->is_debug_types) + if (per_cu->is_debug_types) { struct signatured_type *sig_type; - /* Since this_cu is the first member of struct signatured_type, + /* Since per_cu is the first member of struct signatured_type, we can go from a pointer to one to a pointer to the other. */ - sig_type = (struct signatured_type *) this_cu; - signature = sig_type->signature; + sig_type = (struct signatured_type *) per_cu; dwo_unit = lookup_dwo_type_unit (sig_type, dwo_name, comp_dir); } else { - struct attribute *attr; - - attr = dwarf2_attr (comp_unit_die, DW_AT_GNU_dwo_id, cu); - if (! attr) + gdb::optional signature = lookup_dwo_id(cu, comp_unit_die); + if (!signature.has_value()) error (_("Dwarf Error: missing dwo_id for dwo_name %s" " [in module %s]"), - dwo_name, objfile_name (this_cu->dwarf2_per_objfile->objfile)); - signature = DW_UNSND (attr); - dwo_unit = lookup_dwo_comp_unit (this_cu, dwo_name, comp_dir, - signature); + dwo_name, objfile_name (per_cu->dwarf2_per_objfile->objfile)); + dwo_unit = lookup_dwo_comp_unit (per_cu, dwo_name, comp_dir, + *signature); } return dwo_unit; @@ -7449,7 +7482,6 @@ init_cutu_and_read_dies (struct dwarf2_per_cu_data *this_cu, struct die_reader_specs reader; struct die_info *comp_unit_die; int has_children; - struct attribute *attr; struct signatured_type *sig_type = NULL; struct dwarf2_section_info *abbrev_section; /* Non-zero if CU currently points to a DWO file and we need to @@ -7523,7 +7555,7 @@ init_cutu_and_read_dies (struct dwarf2_per_cu_data *this_cu, /* Since per_cu is the first member of struct signatured_type, we can go from a pointer to one to a pointer to the other. */ sig_type = (struct signatured_type *) this_cu; - gdb_assert (sig_type->signature == cu->header.signature); + gdb_assert (sig_type->signature == cu->header.signature_or_dwo_id); gdb_assert (sig_type->type_offset_in_tu == cu->header.type_cu_offset_in_tu); gdb_assert (this_cu->sect_off == cu->header.sect_off); @@ -7586,9 +7618,9 @@ init_cutu_and_read_dies (struct dwarf2_per_cu_data *this_cu, Note that if USE_EXISTING_OK != 0, and THIS_CU->cu already contains a DWO CU, that this test will fail (the attribute will not be present). */ - attr = dwarf2_attr (comp_unit_die, DW_AT_GNU_dwo_name, cu); + const char *dwo_name = dwarf2_dwo_name (comp_unit_die, cu); abbrev_table_up dwo_abbrev_table; - if (attr) + if (dwo_name) { struct dwo_unit *dwo_unit; struct die_info *dwo_comp_unit_die; @@ -7600,7 +7632,7 @@ init_cutu_and_read_dies (struct dwarf2_per_cu_data *this_cu, sect_offset_str (this_cu->sect_off), bfd_get_filename (abfd)); } - dwo_unit = lookup_dwo_unit (this_cu, comp_unit_die); + dwo_unit = lookup_dwo_unit (&reader, comp_unit_die); if (dwo_unit != NULL) { if (read_cutu_die_from_dwo (this_cu, dwo_unit, @@ -8535,7 +8567,7 @@ read_comp_units_from_section (struct dwarf2_per_objfile *dwarf2_per_objfile, auto sig_type = XOBNEW (&objfile->objfile_obstack, struct signatured_type); memset (sig_type, 0, sizeof (*sig_type)); - sig_type->signature = cu_header.signature; + sig_type->signature = cu_header.signature_or_dwo_id; sig_type->type_offset_in_tu = cu_header.type_cu_offset_in_tu; this_cu = &sig_type->per_cu; } @@ -11839,10 +11871,9 @@ create_dwo_cu_reader (const struct die_reader_specs *reader, struct create_dwo_cu_data *data = (struct create_dwo_cu_data *) datap; struct dwo_file *dwo_file = data->dwo_file; struct dwo_unit *dwo_unit = &data->dwo_unit; - struct attribute *attr; - attr = dwarf2_attr (comp_unit_die, DW_AT_GNU_dwo_id, cu); - if (attr == NULL) + gdb::optional signature = lookup_dwo_id(cu, comp_unit_die); + if (!signature.has_value()) { complaint (_("Dwarf Error: debug entry at offset %s is missing" " its dwo_id [in module %s]"), @@ -11851,7 +11882,7 @@ create_dwo_cu_reader (const struct die_reader_specs *reader, } dwo_unit->dwo_file = dwo_file; - dwo_unit->signature = DW_UNSND (attr); + dwo_unit->signature = *signature; dwo_unit->section = section; dwo_unit->sect_off = sect_off; dwo_unit->length = cu->per_cu->length; @@ -20113,6 +20144,17 @@ dwarf2_string_attr (struct die_info *die, unsigned int name, struct dwarf2_cu *c return str; } +/* Return the dwo name or NULL if not present. If present, it is in either + DW_AT_GNU_dwo_name or DW_AT_dwo_name atrribute. */ +static const char * +dwarf2_dwo_name (struct die_info *die, struct dwarf2_cu *cu) +{ + const char *dwo_name = dwarf2_string_attr (die, DW_AT_GNU_dwo_name, cu); + if (!dwo_name) + dwo_name = dwarf2_string_attr (die, DW_AT_dwo_name, cu); + return dwo_name; +} + /* Return non-zero iff the attribute NAME is defined for the given DIE, and holds a non-zero value. This function should only be used for DW_FORM_flag or DW_FORM_flag_present attributes. */ @@ -22812,6 +22854,23 @@ dwarf_attr_name (unsigned attr) return name; } +/* Convert a unit type to corresponding DW_UT name. */ + +static const char * +dwarf_unit_type_name(int unit_type) { + switch (unit_type) { + case 0x01: return "DW_UT_compile (0x01)"; + case 0x02: return "DW_UT_type (0x02)"; + case 0x03: return "DW_UT_partial (0x03)"; + case 0x04: return "DW_UT_skeleton (0x04)"; + case 0x05: return "DW_UT_split_compile (0x05)"; + case 0x06: return "DW_UT_split_type (0x06)"; + case 0x80: return "DW_UT_lo_user (0x80)"; + case 0xff: return "DW_UT_hi_user (0xff)"; + default: return nullptr; + } +} + /* Convert a DWARF value form code into its string name. */ static const char *