* [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).