* [PATCH] Add initializers to comp_unit_head @ 2022-12-24 22:44 Tom Tromey 2022-12-25 8:06 ` tdevries 0 siblings, 1 reply; 6+ messages in thread From: Tom Tromey @ 2022-12-24 22:44 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey PR symtab/29343 points out that it would be beneficial if comp_unit_head had a constructor and used initializers. This patch implements this. I'm unsure if this is sufficient to close the bug, but at least it's a step. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29343 --- gdb/dwarf2/comp-unit-head.h | 24 ++++++++++++------------ gdb/dwarf2/cu.h | 2 +- gdb/dwarf2/read.c | 2 -- gdb/dwarf2/read.h | 2 +- 4 files changed, 14 insertions(+), 16 deletions(-) diff --git a/gdb/dwarf2/comp-unit-head.h b/gdb/dwarf2/comp-unit-head.h index a7ee3e64546..7579fe7fa29 100644 --- a/gdb/dwarf2/comp-unit-head.h +++ b/gdb/dwarf2/comp-unit-head.h @@ -34,36 +34,36 @@ translation, looks like this. */ struct comp_unit_head { - unsigned int length; - unsigned char version; - unsigned char addr_size; - unsigned char signed_addr_p; - sect_offset abbrev_sect_off; + unsigned int length = 0; + unsigned char version = 0; + unsigned char addr_size = 0; + unsigned char signed_addr_p = 0; + sect_offset abbrev_sect_off {}; /* Size of file offsets; either 4 or 8. */ - unsigned int offset_size; + unsigned int offset_size = 0; /* Size of the length field; either 4 or 12. */ - unsigned int initial_length_size; + unsigned int initial_length_size = 0; - enum dwarf_unit_type unit_type; + enum dwarf_unit_type unit_type {}; /* Offset to first die in this cu from the start of the cu. This will be the first byte following the compilation unit header. */ - cu_offset first_die_cu_offset; + cu_offset first_die_cu_offset {}; /* Offset to the first byte of this compilation unit header in the .debug_info section, for resolving relative reference dies. */ - sect_offset sect_off; + sect_offset sect_off {}; /* For types, offset in the type's DIE of the type defined by this TU. */ - cu_offset type_cu_offset_in_tu; + cu_offset type_cu_offset_in_tu {}; /* 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; + ULONGEST signature = 0; /* Return the total length of the CU described by this header. */ unsigned int get_length () const diff --git a/gdb/dwarf2/cu.h b/gdb/dwarf2/cu.h index 52975095871..b126acab7b2 100644 --- a/gdb/dwarf2/cu.h +++ b/gdb/dwarf2/cu.h @@ -98,7 +98,7 @@ struct dwarf2_cu void add_dependence (struct dwarf2_per_cu_data *ref_per_cu); /* The header of the compilation unit. */ - struct comp_unit_head header {}; + struct comp_unit_head header; /* Base address of this compilation unit. */ gdb::optional<CORE_ADDR> base_address; diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 7bd12c107e2..28fadb886aa 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -23580,8 +23580,6 @@ dwarf2_per_cu_data::get_header () const const gdb_byte *info_ptr = this->section->buffer + to_underlying (this->sect_off); - memset (&m_header, 0, sizeof (m_header)); - read_comp_unit_head (&m_header, info_ptr, this->section, rcuh_kind::COMPILE); diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h index dd718266883..0dc4e2e885c 100644 --- a/gdb/dwarf2/read.h +++ b/gdb/dwarf2/read.h @@ -206,7 +206,7 @@ struct dwarf2_per_cu_data Don't access this field directly, use the get_header method instead. It should be private, but we can't make it private at the moment. */ - mutable comp_unit_head m_header {}; + mutable comp_unit_head m_header; /* The file and directory for this CU. This is cached so that we don't need to re-examine the DWO in some situations. This may be -- 2.38.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Add initializers to comp_unit_head 2022-12-24 22:44 [PATCH] Add initializers to comp_unit_head Tom Tromey @ 2022-12-25 8:06 ` tdevries 2022-12-25 15:33 ` Tom Tromey 0 siblings, 1 reply; 6+ messages in thread From: tdevries @ 2022-12-25 8:06 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches [-- Attachment #1: Type: text/plain, Size: 477 bytes --] On 2022-12-24 22:44, Tom Tromey wrote: > PR symtab/29343 points out that it would be beneficial if > comp_unit_head had a constructor and used initializers. This patch > implements this. LGTM. > I'm unsure if this is sufficient to close the bug, > but at least it's a step. > I think it's sufficient. FWIW, I've rebased the patch attached to the PR on your patch, and I'm left with the patch below, which makes the length field of comp_unit_head private. Thanks, - Tom [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-gdb-symtab-Make-comp_unit_head.length-private.patch --] [-- Type: text/x-patch; name=0001-gdb-symtab-Make-comp_unit_head.length-private.patch, Size: 3328 bytes --] From b04c791586da2916ca87521eca3c82ba71cd22fb Mon Sep 17 00:00:00 2001 From: Tom de Vries <tdevries@suse.de> Date: Sun, 25 Dec 2022 08:53:43 +0100 Subject: [PATCH] [gdb/symtab] Make comp_unit_head.length private Make comp_unit_head.length private, to enforce using accessor function get_length. Tested on x86_64-linux. PR symtab/29343 Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29343 --- gdb/dwarf2/comp-unit-head.c | 4 ++-- gdb/dwarf2/comp-unit-head.h | 16 +++++++++++++--- gdb/dwarf2/read.c | 4 ++-- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/gdb/dwarf2/comp-unit-head.c b/gdb/dwarf2/comp-unit-head.c index edb2a8aa55a..128d62fcbb1 100644 --- a/gdb/dwarf2/comp-unit-head.c +++ b/gdb/dwarf2/comp-unit-head.c @@ -44,7 +44,7 @@ read_comp_unit_head (struct comp_unit_head *cu_header, const char *filename = section->get_file_name (); bfd *abfd = section->get_bfd_owner (); - cu_header->length = read_initial_length (abfd, info_ptr, &bytes_read); + cu_header->set_length (read_initial_length (abfd, info_ptr, &bytes_read)); cu_header->initial_length_size = bytes_read; cu_header->offset_size = (bytes_read == 4) ? 4 : 8; info_ptr += bytes_read; @@ -166,7 +166,7 @@ error_check_comp_unit_head (dwarf2_per_objfile *per_objfile, > section->size) error (_("Dwarf Error: bad length (0x%x) in compilation unit header " "(offset %s + 0) [in module %s]"), - header->length, sect_offset_str (header->sect_off), + header->get_length (false), sect_offset_str (header->sect_off), filename); } diff --git a/gdb/dwarf2/comp-unit-head.h b/gdb/dwarf2/comp-unit-head.h index 7579fe7fa29..56b37ae537e 100644 --- a/gdb/dwarf2/comp-unit-head.h +++ b/gdb/dwarf2/comp-unit-head.h @@ -34,7 +34,9 @@ translation, looks like this. */ struct comp_unit_head { - unsigned int length = 0; +private: + unsigned int m_length = 0; +public: unsigned char version = 0; unsigned char addr_size = 0; unsigned char signed_addr_p = 0; @@ -65,10 +67,18 @@ struct comp_unit_head DW_UT_skeleton or DW_UT_split_compile. */ ULONGEST signature = 0; + void set_length (unsigned int length) + { + m_length = length; + } + /* Return the total length of the CU described by this header. */ - unsigned int get_length () const + unsigned int get_length (bool include_initial = true) const { - return initial_length_size + length; + unsigned int res = m_length; + if (include_initial) + res += initial_length_size; + return res; } /* Return TRUE if OFF is within this CU. */ diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 28fadb886aa..50881929d47 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -7744,7 +7744,7 @@ load_full_comp_unit (dwarf2_per_cu_data *this_cu, gdb_assert (cu->die_hash == NULL); cu->die_hash = - htab_create_alloc_ex (cu->header.length / 12, + htab_create_alloc_ex (cu->header.get_length (false) / 12, die_hash, die_eq, NULL, @@ -23026,7 +23026,7 @@ read_signatured_type (signatured_type *sig_type, gdb_assert (cu->die_hash == NULL); cu->die_hash = - htab_create_alloc_ex (cu->header.length / 12, + htab_create_alloc_ex (cu->header.get_length (false) / 12, die_hash, die_eq, NULL, -- 2.34.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Add initializers to comp_unit_head 2022-12-25 8:06 ` tdevries @ 2022-12-25 15:33 ` Tom Tromey 2022-12-25 17:56 ` tdevries 0 siblings, 1 reply; 6+ messages in thread From: Tom Tromey @ 2022-12-25 15:33 UTC (permalink / raw) To: tdevries via Gdb-patches; +Cc: Tom Tromey, tdevries >>>>> tdevries via Gdb-patches <gdb-patches@sourceware.org> writes: > FWIW, I've rebased the patch attached to the PR on your patch, and I'm > left with the patch below, which makes the length field of > comp_unit_head private. > /* Return the total length of the CU described by this header. */ > - unsigned int get_length () const > + unsigned int get_length (bool include_initial = true) const > { > - return initial_length_size + length; > + unsigned int res = m_length; > + if (include_initial) > + res += initial_length_size; > + return res; I think that since the callers all statically decide which length they want, there should probably just be two methods with different names. Tom ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Add initializers to comp_unit_head 2022-12-25 15:33 ` Tom Tromey @ 2022-12-25 17:56 ` tdevries 2022-12-26 17:59 ` Tom Tromey 0 siblings, 1 reply; 6+ messages in thread From: tdevries @ 2022-12-25 17:56 UTC (permalink / raw) To: Tom Tromey; +Cc: tdevries via Gdb-patches On 2022-12-25 15:33, Tom Tromey wrote: >>>>>> tdevries via Gdb-patches <gdb-patches@sourceware.org> writes: > >> FWIW, I've rebased the patch attached to the PR on your patch, and I'm >> left with the patch below, which makes the length field of >> comp_unit_head private. > >> /* Return the total length of the CU described by this header. */ >> - unsigned int get_length () const >> + unsigned int get_length (bool include_initial = true) const >> { >> - return initial_length_size + length; >> + unsigned int res = m_length; >> + if (include_initial) >> + res += initial_length_size; >> + return res; > > I think that since the callers all statically decide which length they > want, there should probably just be two methods with different names. > OK, so how about: - get_length_with_initial - get_length_without_initial ? Thanks, - Tom > Tom ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Add initializers to comp_unit_head 2022-12-25 17:56 ` tdevries @ 2022-12-26 17:59 ` Tom Tromey 2022-12-30 12:58 ` [pushed] [gdb/symtab] Make comp_unit_head.length private Tom de Vries 0 siblings, 1 reply; 6+ messages in thread From: Tom Tromey @ 2022-12-26 17:59 UTC (permalink / raw) To: tdevries; +Cc: Tom Tromey, tdevries via Gdb-patches > OK, so how about: > - get_length_with_initial > - get_length_without_initial > ? Works for me, thank you. Tom ^ permalink raw reply [flat|nested] 6+ messages in thread
* [pushed] [gdb/symtab] Make comp_unit_head.length private 2022-12-26 17:59 ` Tom Tromey @ 2022-12-30 12:58 ` Tom de Vries 0 siblings, 0 replies; 6+ messages in thread From: Tom de Vries @ 2022-12-30 12:58 UTC (permalink / raw) To: Tom Tromey; +Cc: tdevries via Gdb-patches [-- Attachment #1: Type: text/plain, Size: 267 bytes --] [was: Re: [PATCH] Add initializers to comp_unit_head ] On 12/26/22 18:59, Tom Tromey wrote: >> OK, so how about: >> - get_length_with_initial >> - get_length_without_initial >> ? > > Works for me, thank you. Committed with that approach, as attached. Thanks, - Tom [-- Attachment #2: 0001-gdb-symtab-Make-comp_unit_head.length-private.patch --] [-- Type: text/x-patch, Size: 7449 bytes --] From d8f52a9a9ccbf7411cf4ae487d2756826f5d0bd5 Mon Sep 17 00:00:00 2001 From: Tom de Vries <tdevries@suse.de> Date: Fri, 30 Dec 2022 13:55:22 +0100 Subject: [pushed] [gdb/symtab] Make comp_unit_head.length private Make comp_unit_head.length private, to enforce using accessor functions. Replace accessor function get_length with get_length_with_initial and get_length_without_initial, to make it explicit which variant we're using. Tested on x86_64-linux. PR symtab/29343 Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29343 --- gdb/dwarf2/comp-unit-head.c | 6 +++--- gdb/dwarf2/comp-unit-head.h | 25 ++++++++++++++++++++----- gdb/dwarf2/read.c | 20 ++++++++++---------- 3 files changed, 33 insertions(+), 18 deletions(-) diff --git a/gdb/dwarf2/comp-unit-head.c b/gdb/dwarf2/comp-unit-head.c index edb2a8aa55a..b618a674808 100644 --- a/gdb/dwarf2/comp-unit-head.c +++ b/gdb/dwarf2/comp-unit-head.c @@ -44,7 +44,7 @@ read_comp_unit_head (struct comp_unit_head *cu_header, const char *filename = section->get_file_name (); bfd *abfd = section->get_bfd_owner (); - cu_header->length = read_initial_length (abfd, info_ptr, &bytes_read); + cu_header->set_length (read_initial_length (abfd, info_ptr, &bytes_read)); cu_header->initial_length_size = bytes_read; cu_header->offset_size = (bytes_read == 4) ? 4 : 8; info_ptr += bytes_read; @@ -162,11 +162,11 @@ error_check_comp_unit_head (dwarf2_per_objfile *per_objfile, /* Cast to ULONGEST to use 64-bit arithmetic when possible to avoid potential 32-bit overflow. */ - if (((ULONGEST) header->sect_off + header->get_length ()) + if (((ULONGEST) header->sect_off + header->get_length_with_initial ()) > section->size) error (_("Dwarf Error: bad length (0x%x) in compilation unit header " "(offset %s + 0) [in module %s]"), - header->length, sect_offset_str (header->sect_off), + header->get_length_without_initial (), sect_offset_str (header->sect_off), filename); } diff --git a/gdb/dwarf2/comp-unit-head.h b/gdb/dwarf2/comp-unit-head.h index 7579fe7fa29..45384fac720 100644 --- a/gdb/dwarf2/comp-unit-head.h +++ b/gdb/dwarf2/comp-unit-head.h @@ -34,7 +34,9 @@ translation, looks like this. */ struct comp_unit_head { - unsigned int length = 0; +private: + unsigned int m_length = 0; +public: unsigned char version = 0; unsigned char addr_size = 0; unsigned char signed_addr_p = 0; @@ -65,17 +67,30 @@ struct comp_unit_head DW_UT_skeleton or DW_UT_split_compile. */ ULONGEST signature = 0; - /* Return the total length of the CU described by this header. */ - unsigned int get_length () const + void set_length (unsigned int length) { - return initial_length_size + length; + m_length = length; + } + + /* Return the total length of the CU described by this header, including the + initial length field. */ + unsigned int get_length_with_initial () const + { + return m_length + initial_length_size; + } + + /* Return the total length of the CU described by this header, excluding the + initial length field. */ + unsigned int get_length_without_initial () const + { + return m_length; } /* Return TRUE if OFF is within this CU. */ bool offset_in_cu_p (sect_offset off) const { sect_offset bottom = sect_off; - sect_offset top = sect_off + get_length (); + sect_offset top = sect_off + get_length_with_initial (); return off >= bottom && off < top; } diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 28fadb886aa..b33ea6847e0 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -5598,7 +5598,7 @@ create_debug_type_hash_table (dwarf2_per_objfile *per_objfile, ptr = read_and_check_comp_unit_head (per_objfile, &header, section, abbrev_section, ptr, section_kind); - length = header.get_length (); + length = header.get_length_with_initial (); /* Skip dummy type units. */ if (ptr >= info_ptr + length @@ -6005,7 +6005,7 @@ read_cutu_die_from_dwo (dwarf2_cu *cu, gdb_assert (dwo_unit->sect_off == cu->header.sect_off); /* For DWOs coming from DWP files, we don't know the CU length nor the type's offset in the TU until now. */ - dwo_unit->length = cu->header.get_length (); + dwo_unit->length = cu->header.get_length_with_initial (); dwo_unit->type_offset_in_tu = cu->header.type_cu_offset_in_tu; /* Establish the type offset that can be used to lookup the type. @@ -6021,7 +6021,7 @@ read_cutu_die_from_dwo (dwarf2_cu *cu, gdb_assert (dwo_unit->sect_off == cu->header.sect_off); /* For DWOs coming from DWP files, we don't know the CU length until now. */ - dwo_unit->length = cu->header.get_length (); + dwo_unit->length = cu->header.get_length_with_initial (); } dwo_abbrev_section->read (objfile); @@ -6289,7 +6289,7 @@ cutu_reader::cutu_reader (dwarf2_per_cu_data *this_cu, /* LENGTH has not been set yet for type units if we're using .gdb_index. */ - this_cu->set_length (cu->header.get_length ()); + this_cu->set_length (cu->header.get_length_with_initial ()); /* Establish the type offset that can be used to lookup the type. */ sig_type->type_offset_in_section = @@ -6305,7 +6305,7 @@ cutu_reader::cutu_reader (dwarf2_per_cu_data *this_cu, rcuh_kind::COMPILE); gdb_assert (this_cu->sect_off == cu->header.sect_off); - this_cu->set_length (cu->header.get_length ()); + this_cu->set_length (cu->header.get_length_with_initial ()); this_cu->set_version (cu->header.version); } } @@ -6465,7 +6465,7 @@ cutu_reader::cutu_reader (dwarf2_per_cu_data *this_cu, m_new_cu->str_offsets_base = parent_cu->str_offsets_base; m_new_cu->addr_base = parent_cu->addr_base; } - this_cu->set_length (m_new_cu->header.get_length ()); + this_cu->set_length (m_new_cu->header.get_length_with_initial ()); /* Skip dummy compilation units. */ if (info_ptr >= begin_info_ptr + this_cu->length () @@ -7249,7 +7249,7 @@ read_comp_units_from_section (dwarf2_per_objfile *per_objfile, *slot = sig_ptr; } this_cu->sect_off = sect_off; - this_cu->set_length (cu_header.get_length ()); + this_cu->set_length (cu_header.get_length_with_initial ()); this_cu->is_dwz = is_dwz; this_cu->section = section; /* Init this asap, to avoid a data race in the set_version in @@ -7744,7 +7744,7 @@ load_full_comp_unit (dwarf2_per_cu_data *this_cu, gdb_assert (cu->die_hash == NULL); cu->die_hash = - htab_create_alloc_ex (cu->header.length / 12, + htab_create_alloc_ex (cu->header.get_length_without_initial () / 12, die_hash, die_eq, NULL, @@ -18409,7 +18409,7 @@ cooked_indexer::index_dies (cutu_reader *reader, { const gdb_byte *end_ptr = (reader->buffer + to_underlying (reader->cu->header.sect_off) - + reader->cu->header.get_length ()); + + reader->cu->header.get_length_with_initial ()); while (info_ptr < end_ptr) { @@ -23026,7 +23026,7 @@ read_signatured_type (signatured_type *sig_type, gdb_assert (cu->die_hash == NULL); cu->die_hash = - htab_create_alloc_ex (cu->header.length / 12, + htab_create_alloc_ex (cu->header.get_length_without_initial () / 12, die_hash, die_eq, NULL, base-commit: a984f112b015b7d33c3c91230eb4c35695926539 -- 2.35.3 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-12-30 12:58 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-12-24 22:44 [PATCH] Add initializers to comp_unit_head Tom Tromey 2022-12-25 8:06 ` tdevries 2022-12-25 15:33 ` Tom Tromey 2022-12-25 17:56 ` tdevries 2022-12-26 17:59 ` Tom Tromey 2022-12-30 12:58 ` [pushed] [gdb/symtab] Make comp_unit_head.length private Tom de Vries
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).