From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 115837 invoked by alias); 15 Oct 2019 20:19:36 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 115827 invoked by uid 89); 15 Oct 2019 20:19:35 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-24.5 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=respectfully, fan, HX-Received:a63 X-HELO: mail-pf1-f202.google.com Received: from mail-pf1-f202.google.com (HELO mail-pf1-f202.google.com) (209.85.210.202) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 15 Oct 2019 20:19:33 +0000 Received: by mail-pf1-f202.google.com with SMTP id 22so16813624pfx.8 for ; Tue, 15 Oct 2019 13:19:32 -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=+Yav+WknRDoHdv1oJOwMb6dkSP9+YRIb2nnMk2UezsY=; b=GexJdIwn4ZEMWEkQwjR9q1zvozQcWCuLShBgwylrDLHa7dBoPTZgpChu1p0/4QEwY3 lJZO5ajjXuxA+1rJxFgQSu5oKXtemZHRHIm+BfTnjmajzWUgCm45cI+OtgbYtOSfnFet PmgTmdgA1vYnJBvYKOdEcPfGHo0y82Tswbmgjgn6S/EFp1SxMyIa4Phcg7gb4dg2Hy1V iDUieKGI+B2JKqmRO+etGCh+b31QfQQaJC5cHVj448czjjBB3tDLQWhhc0RLeBqrYZiz XQnPs8PivFctyxOLlbwBNtW8NdAtQ3Xm3WXA7VrWJgY3L58KBaiY74yUVhh3jJqGev9E AuJw== Date: Tue, 15 Oct 2019 20:19:00 -0000 In-Reply-To: Message-Id: <20191015201927.60124-1-tamur@google.com> Mime-Version: 1.0 References: Subject: [PATCH] DWARF 5 support: Handle line table and file indexes From: "Ali Tamur via gdb-patches" Reply-To: Ali Tamur To: Simon Marchi Cc: gdb-patches@sourceware.org, Ali Tamur Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2019-10/txt/msg00454.txt.bz2 Hi Simon, Thank you for the review. I did all the changes you asked except for one, which I explain below. Please take another look. Ali > > Make file_names field private to abstract this detail from the clients.= =20 > > I think it's a good idea to encapsulate this. =C2=A0Should include_dirs be > private as well, since it is in the same situation? Done. > > * =C2=A0Pass whether the file index is zero or one based to a few metho= ds. > > Hmm, I am not a big fan of that, .. I got rid of is_zero_indexed parameter and introduced a file_names method. > > -enum class file_name_index : unsigned int {}; > > +typedef int file_name_index; > > Is there a reason you changed these? =C2=A0The reason why these were of e= num > class type (or "strong typedef") still seems valid to me. =C2=A0We don't = want > some code accessing the vectors blindy using these indices, so it > encourages people to use the right accessor method instead. I respectfully disagree. We do "index - 1" calculation in file_name_at and include_dir_at methods. If we are going to cast back and forth to integers anyways, having an enum loses its appeal. > Remove one indentation level. > Format it like this: > differ -> differs > Two spaces after period. > Unnecessary change? > Hmm it's not great to have to lose the const here. > This function should be static. Done. --- * Fix handling of file and directory indexes in line tables; in DWARF 5 the indexes are zero-based. Make file_names field private to abstract this deta= il from the clients. Introduce file_names and file_names_size methods. Reflect these changes in clients. * Handle DW_FORM_data16 in read_formatted_entries; it is used to record MD5 of the file entries in DWARF 5. * Fix a bug in line header parsing that calculates the length of the header incorrectly. (Seemingly this manifests itself only in DWARF 5). * Introduce a new method, is_valid_file_index that takes into account whet= her it is DWARF 5. Use it in related clients. Tested with CC=3D/usr/bin/gcc (version 8.3.0) against master branch (also w= ith -gsplit-dwarf and -gdwarf-4 flags) and there was no increase in the set of tests that fails. (gdb still cannot debug a 'hello world' program with DWAR= F 5, so for the time being, this is all we care about). This is part of an effort to support DWARF 5 in gdb. --- gdb/dwarf2read.c | 174 ++++++++++++++++++++++++++++------------------- 1 file changed, 104 insertions(+), 70 deletions(-) diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index ee9df34ed2..8e272b1da3 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -922,13 +922,13 @@ typedef void (die_reader_func_ftype) (const struct di= e_reader_specs *reader, int has_children, void *data); =20 -/* A 1-based directory index. This is a strong typedef to prevent - accidentally using a directory index as a 0-based index into an - array/vector. */ -enum class dir_index : unsigned int {}; +/* dir_index is 1-based in DWARF 4 and before, and is 0-based in DWARF 5 a= nd + later. */ +typedef int dir_index; =20 -/* Likewise, a 1-based file name index. */ -enum class file_name_index : unsigned int {}; +/* file_name_index is 1-based in DWARF 4 and before, and is 0-based in DWA= RF 5 + and later. */ +typedef int file_name_index; =20 struct file_entry { @@ -980,32 +980,40 @@ struct line_header void add_file_name (const char *name, dir_index d_index, unsigned int mod_time, unsigned int length); =20 - /* Return the include dir at INDEX (1-based). Returns NULL if INDEX - is out of bounds. */ + /* Return the include dir at INDEX (0-based in DWARF 5 and 1-based befor= e). + Returns NULL if INDEX is out of bounds. */ const char *include_dir_at (dir_index index) const { - /* Convert directory index number (1-based) to vector index - (0-based). */ - size_t vec_index =3D to_underlying (index) - 1; - - if (vec_index >=3D include_dirs.size ()) + size_t vec_index; + if (version >=3D 5) + vec_index =3D index; + else + vec_index =3D index - 1; + if (vec_index >=3D m_include_dirs.size ()) return NULL; - return include_dirs[vec_index]; + return m_include_dirs[vec_index]; } =20 - /* Return the file name at INDEX (1-based). Returns NULL if INDEX - is out of bounds. */ + /* Return the file name at INDEX (0-based in DWARF 5 and 1-based before). + Returns NULL if INDEX is out of bounds. */ file_entry *file_name_at (file_name_index index) { - /* Convert file name index number (1-based) to vector index - (0-based). */ - size_t vec_index =3D to_underlying (index) - 1; - - if (vec_index >=3D file_names.size ()) + size_t vec_index; + if (version >=3D 5) + vec_index =3D index; + else + vec_index =3D index - 1; + if (vec_index >=3D m_file_names.size ()) return NULL; - return &file_names[vec_index]; + return &m_file_names[vec_index]; } =20 + /* The indexes are 0-based in DWARF 5 and 1-based in DWARF 4. Therefore, + this method should only be used to iterate through all file entries i= n an + index-agnostic manner. */ + std::vector &file_names () + { return m_file_names; } + /* Offset of line number information in .debug_line section. */ sect_offset sect_off {}; =20 @@ -1032,12 +1040,23 @@ struct line_header pointers. The memory is owned by debug_line_buffer. */ std::vector include_dirs; =20 - /* The file_names table. */ - std::vector file_names; + int file_names_size () + { return m_file_names.size(); } =20 /* The start and end of the statement program following this header. These point into dwarf2_per_objfile->line_buffer. */ const gdb_byte *statement_program_start {}, *statement_program_end {}; + + private: + /* The include_directories table. Note these are observing + pointers. The memory is owned by debug_line_buffer. */ + std::vector m_include_dirs; + + /* The file_names table. This is private because the meaning of indexes + differs among DWARF versions (The first valid index is 1 in DWARF 4 a= nd + before, and is 0 in DWARF 5 and later). So the client should use + file_name_at method for access. */ + std::vector m_file_names; }; =20 typedef std::unique_ptr line_header_up; @@ -3644,7 +3663,6 @@ dw2_get_file_names_reader (const struct die_reader_sp= ecs *reader, struct objfile *objfile =3D dwarf2_per_objfile->objfile; struct dwarf2_per_cu_data *lh_cu; struct attribute *attr; - int i; void **slot; struct quick_file_names *qfn; =20 @@ -3703,12 +3721,12 @@ dw2_get_file_names_reader (const struct die_reader_= specs *reader, if (strcmp (fnd.name, "") !=3D 0) ++offset; =20 - qfn->num_file_names =3D offset + lh->file_names.size (); + qfn->num_file_names =3D offset + lh->file_names_size (); qfn->file_names =3D XOBNEWVEC (&objfile->objfile_obstack, const char *, qfn->num_file_name= s); if (offset !=3D 0) qfn->file_names[0] =3D xstrdup (fnd.name); - for (i =3D 0; i < lh->file_names.size (); ++i) + for (int i =3D 0; i < lh->file_names_size (); ++i) qfn->file_names[i + offset] =3D file_full_name (i + 1, lh.get (), fnd.= comp_dir); qfn->real_names =3D NULL; =20 @@ -11717,16 +11735,16 @@ dwarf2_cu::setup_type_unit_groups (struct die_inf= o *die) process_full_type_unit still needs to know if this is the first time. */ =20 - tu_group->num_symtabs =3D line_header->file_names.size (); + tu_group->num_symtabs =3D line_header->file_names_size (); tu_group->symtabs =3D XNEWVEC (struct symtab *, - line_header->file_names.size ()); + line_header->file_names_size ()); =20 - for (i =3D 0; i < line_header->file_names.size (); ++i) + auto &file_names =3D line_header->file_names (); + for (i =3D 0; i < file_names.size (); ++i) { - file_entry &fe =3D line_header->file_names[i]; - - dwarf2_start_subfile (this, fe.name, - fe.include_dir (line_header)); + file_entry *fe =3D &file_names[i]; + dwarf2_start_subfile (this, fe->name, + fe->include_dir (line_header)); buildsym_compunit *b =3D get_builder (); if (b->get_current_subfile ()->symtab =3D=3D NULL) { @@ -11739,8 +11757,8 @@ dwarf2_cu::setup_type_unit_groups (struct die_info = *die) =3D allocate_symtab (cust, b->get_current_subfile ()->name); } =20 - fe.symtab =3D b->get_current_subfile ()->symtab; - tu_group->symtabs[i] =3D fe.symtab; + fe->symtab =3D b->get_current_subfile ()->symtab; + tu_group->symtabs[i] =3D fe->symtab; } } else @@ -11753,11 +11771,11 @@ dwarf2_cu::setup_type_unit_groups (struct die_inf= o *die) compunit_language (cust), 0, cust)); =20 - for (i =3D 0; i < line_header->file_names.size (); ++i) + auto &file_names =3D line_header->file_names (); + for (i =3D 0; i < file_names.size (); ++i) { - file_entry &fe =3D line_header->file_names[i]; - - fe.symtab =3D tu_group->symtabs[i]; + file_entry *fe =3D &file_names[i]; + fe->symtab =3D tu_group->symtabs[i]; } } =20 @@ -16230,7 +16248,7 @@ process_structure_scope (struct die_info *die, stru= ct dwarf2_cu *cu) { /* Any related symtab will do. */ symtab - =3D cu->line_header->file_name_at (file_name_index (1))->symtab; + =3D cu->line_header->file_names ()[0].symtab; } else { @@ -20286,9 +20304,9 @@ line_header::add_include_dir (const char *include_d= ir) { if (dwarf_line_debug >=3D 2) fprintf_unfiltered (gdb_stdlog, "Adding dir %zu: %s\n", - include_dirs.size () + 1, include_dir); + m_include_dirs.size () + 1, include_dir); =20 - include_dirs.push_back (include_dir); + m_include_dirs.push_back (include_dir); } =20 void @@ -20299,9 +20317,9 @@ line_header::add_file_name (const char *name, { if (dwarf_line_debug >=3D 2) fprintf_unfiltered (gdb_stdlog, "Adding file %u: %s\n", - (unsigned) file_names.size () + 1, name); + (unsigned) file_names_size () + 1, name); =20 - file_names.emplace_back (name, d_index, mod_time, length); + m_file_names.emplace_back (name, d_index, mod_time, length); } =20 /* A convenience function to find the proper .debug_line section for a CU.= */ @@ -20415,6 +20433,11 @@ read_formatted_entries (struct dwarf2_per_objfile = *dwarf2_per_objfile, buf +=3D 8; break; =20 + case DW_FORM_data16: + /* This is used for MD5, but file_entry does not record MD5s. */ + buf +=3D 16; + break; + case DW_FORM_udata: uint.emplace (read_unsigned_leb128 (abfd, buf, &bytes_read)); buf +=3D bytes_read; @@ -20515,12 +20538,15 @@ dwarf_decode_line_header (sect_offset sect_off, s= truct dwarf2_cu *cu) read_checked_initial_length_and_offset (abfd, line_ptr, &cu->header, &bytes_read, &offset_size); line_ptr +=3D bytes_read; + + const gdb_byte *start_here =3D line_ptr; + if (line_ptr + lh->total_length > (section->buffer + section->size)) { dwarf2_statement_list_fits_in_line_number_section_complaint (); return 0; } - lh->statement_program_end =3D line_ptr + lh->total_length; + lh->statement_program_end =3D start_here + lh->total_length; lh->version =3D read_2_bytes (abfd, line_ptr); line_ptr +=3D 2; if (lh->version > 5) @@ -20550,6 +20576,7 @@ dwarf_decode_line_header (sect_offset sect_off, str= uct dwarf2_cu *cu) } lh->header_length =3D read_offset_1 (abfd, line_ptr, offset_size); line_ptr +=3D offset_size; + lh->statement_program_start =3D line_ptr + lh->header_length; lh->minimum_instruction_length =3D read_1_byte (abfd, line_ptr); line_ptr +=3D 1; if (lh->version >=3D 4) @@ -20634,7 +20661,6 @@ dwarf_decode_line_header (sect_offset sect_off, str= uct dwarf2_cu *cu) } line_ptr +=3D bytes_read; } - lh->statement_program_start =3D line_ptr; =20 if (line_ptr > (section->buffer + section->size)) complaint (_("line number info header doesn't " @@ -20651,18 +20677,17 @@ dwarf_decode_line_header (sect_offset sect_off, s= truct dwarf2_cu *cu) Returns NULL if FILE_INDEX should be ignored, i.e., it is pst->filename= . */ =20 static const char * -psymtab_include_file_name (const struct line_header *lh, int file_index, +psymtab_include_file_name (const struct line_header *lh, const file_entry = *fe, const struct partial_symtab *pst, const char *comp_dir, gdb::unique_xmalloc_ptr *name_holder) { - const file_entry &fe =3D lh->file_names[file_index]; - const char *include_name =3D fe.name; + const char *include_name =3D fe->name; const char *include_name_to_compare =3D include_name; const char *pst_filename; int file_is_pst; =20 - const char *dir_name =3D fe.include_dir (lh); + const char *dir_name =3D fe->include_dir (lh); =20 gdb::unique_xmalloc_ptr hold_compare; if (!IS_ABSOLUTE_PATH (include_name) @@ -20831,8 +20856,8 @@ private: and initialized according to the DWARF spec. */ =20 unsigned char m_op_index =3D 0; - /* The line table index (1-based) of the current file. */ - file_name_index m_file =3D (file_name_index) 1; + /* The line table index of the current file. */ + file_name_index m_file =3D 1; unsigned int m_line =3D 1; =20 /* These are initialized in the constructor. */ @@ -21024,7 +21049,7 @@ lnp_state_machine::record_line (bool end_sequence) fprintf_unfiltered (gdb_stdlog, "Processing actual line %u: file %u," " address %s, is_stmt %u, discrim %u\n", - m_line, to_underlying (m_file), + m_line, m_file, paddress (m_gdbarch, m_address), m_is_stmt, m_discriminator); } @@ -21367,13 +21392,14 @@ dwarf_decode_lines (struct line_header *lh, const= char *comp_dir, =20 /* Now that we're done scanning the Line Header Program, we can create the psymtab of each included file. */ - for (file_index =3D 0; file_index < lh->file_names.size (); file_ind= ex++) - if (lh->file_names[file_index].included_p =3D=3D 1) + auto &file_names =3D lh->file_names (); + for (file_index =3D 0; file_index < file_names.size (); file_index++) + if (file_names[file_index].included_p =3D=3D 1) { gdb::unique_xmalloc_ptr name_holder; const char *include_name =3D - psymtab_include_file_name (lh, file_index, pst, comp_dir, - &name_holder); + psymtab_include_file_name (lh, &file_names[file_index], pst, + comp_dir, &name_holder); if (include_name !=3D NULL) dwarf2_create_include_psymtab (include_name, pst, objfile); } @@ -21385,13 +21411,13 @@ dwarf_decode_lines (struct line_header *lh, const= char *comp_dir, line numbers). */ buildsym_compunit *builder =3D cu->get_builder (); struct compunit_symtab *cust =3D builder->get_compunit_symtab (); - int i; =20 - for (i =3D 0; i < lh->file_names.size (); i++) + auto &file_names =3D lh->file_names (); + for (int i =3D 0; i < file_names.size (); i++) { - file_entry &fe =3D lh->file_names[i]; + file_entry *fe =3D &file_names[i]; =20 - dwarf2_start_subfile (cu, fe.name, fe.include_dir (lh)); + dwarf2_start_subfile (cu, fe->name, fe->include_dir (lh)); =20 if (builder->get_current_subfile ()->symtab =3D=3D NULL) { @@ -21399,7 +21425,7 @@ dwarf_decode_lines (struct line_header *lh, const c= har *comp_dir, =3D allocate_symtab (cust, builder->get_current_subfile ()->name); } - fe.symtab =3D builder->get_current_subfile ()->symtab; + fe->symtab =3D builder->get_current_subfile ()->symtab; } } } @@ -24190,6 +24216,14 @@ dwarf_alloc_die (struct dwarf2_cu *cu, int num_att= rs) =0C /* Macro support. */ =20 +static bool +is_valid_file_index (int file_index, struct line_header *lh) +{ + if (lh->version >=3D 5) + return 0 <=3D file_index && file_index < lh->file_names_size (); + return 1 <=3D file_index && file_index <=3D lh->file_names_size (); +} + /* Return file name relative to the compilation directory of file number I= in *LH's file name table. The result is allocated using xmalloc; the call= er is responsible for freeing it. */ @@ -24199,17 +24233,17 @@ file_file_name (int file, struct line_header *lh) { /* Is the file number a valid index into the line header's file name table? Remember that file numbers start with one, not zero. */ - if (1 <=3D file && file <=3D lh->file_names.size ()) + if (is_valid_file_index (file, lh)) { - const file_entry &fe =3D lh->file_names[file - 1]; + const file_entry *fe =3D lh->file_name_at (file); =20 - if (!IS_ABSOLUTE_PATH (fe.name)) + if (!IS_ABSOLUTE_PATH (fe->name)) { - const char *dir =3D fe.include_dir (lh); + const char *dir =3D fe->include_dir (lh); if (dir !=3D NULL) - return concat (dir, SLASH_STRING, fe.name, (char *) NULL); + return concat (dir, SLASH_STRING, fe->name, (char *) NULL); } - return xstrdup (fe.name); + return xstrdup (fe->name); } else { @@ -24237,7 +24271,7 @@ file_full_name (int file, struct line_header *lh, c= onst char *comp_dir) { /* Is the file number a valid index into the line header's file name table? Remember that file numbers start with one, not zero. */ - if (1 <=3D file && file <=3D lh->file_names.size ()) + if (is_valid_file_index (file, lh)) { char *relative =3D file_file_name (file, lh); =20 --=20 2.23.0.700.g56cf767bdb-goog