* [PATCH 01/13] gdb/dwarf: change read_loclist_index complaints into errors
2021-01-20 5:39 [PATCH 00/13] DWARF 5 rnglists & loclists fixes (PR 26813) Simon Marchi
@ 2021-01-20 5:39 ` Simon Marchi
2021-01-28 15:17 ` Zoran Zaric
2021-01-20 5:39 ` [PATCH 02/13] gdb/dwarf: fix bound check in read_rnglist_index Simon Marchi
` (12 subsequent siblings)
13 siblings, 1 reply; 34+ messages in thread
From: Simon Marchi @ 2021-01-20 5:39 UTC (permalink / raw)
To: gdb-patches; +Cc: Zoran Zaric, Simon Marchi
From: Simon Marchi <simon.marchi@efficios.com>
Unlike read_rnglists_index, read_loclist_index uses complaints when it
detects an inconsistency (a DW_FORM_loclistx value without a
.debug_loclists section or an offset outside of the section). I really
think they should be errors, since there's no point in continuing if
this situation happens, we will likely segfault or read garbage.
gdb/ChangeLog:
* dwarf2/read.c (read_loclist_index): Change complaints into
errors.
Change-Id: Ic3a1cf6e682d47cb6e739dd76fd7ca5be2637e10
---
gdb/dwarf2/read.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 9032186ef89e..2b76ed001616 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -20190,19 +20190,22 @@ read_loclist_index (struct dwarf2_cu *cu, ULONGEST loclist_index)
section->read (objfile);
if (section->buffer == NULL)
- complaint (_("DW_FORM_loclistx used without .debug_loclists "
- "section [in module %s]"), objfile_name (objfile));
+ error (_("DW_FORM_loclistx used without .debug_loclists "
+ "section [in module %s]"), objfile_name (objfile));
+
struct loclists_rnglists_header header;
read_loclists_rnglists_header (&header, section);
if (loclist_index >= header.offset_entry_count)
- complaint (_("DW_FORM_loclistx pointing outside of "
- ".debug_loclists offset array [in module %s]"),
- objfile_name (objfile));
+ error (_("DW_FORM_loclistx pointing outside of "
+ ".debug_loclists offset array [in module %s]"),
+ objfile_name (objfile));
+
if (loclist_base + loclist_index * cu->header.offset_size
>= section->size)
- complaint (_("DW_FORM_loclistx pointing outside of "
- ".debug_loclists section [in module %s]"),
- objfile_name (objfile));
+ error (_("DW_FORM_loclistx pointing outside of "
+ ".debug_loclists section [in module %s]"),
+ objfile_name (objfile));
+
const gdb_byte *info_ptr
= section->buffer + loclist_base + loclist_index * cu->header.offset_size;
--
2.30.0
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 01/13] gdb/dwarf: change read_loclist_index complaints into errors
2021-01-20 5:39 ` [PATCH 01/13] gdb/dwarf: change read_loclist_index complaints into errors Simon Marchi
@ 2021-01-28 15:17 ` Zoran Zaric
2021-01-28 15:42 ` Simon Marchi
0 siblings, 1 reply; 34+ messages in thread
From: Zoran Zaric @ 2021-01-28 15:17 UTC (permalink / raw)
To: Simon Marchi, gdb-patches; +Cc: Simon Marchi
> From: Simon Marchi <simon.marchi@efficios.com>
>
> Unlike read_rnglists_index, read_loclist_index uses complaints when it
> detects an inconsistency (a DW_FORM_loclistx value without a
> .debug_loclists section or an offset outside of the section). I really
> think they should be errors, since there's no point in continuing if
> this situation happens, we will likely segfault or read garbage.
>
Makes sense.
Although unless I misunderstand something, isn't the difference here
that the error will actually throw an exception and therefore stop
reading of the compilation unit if not the whole object file debug
information?
If this is the case, then considering the difference in usage between
the two, having a wrong loclist information can still provide a correct
line table information, but having a wrong rnglist information in my
mind creates a more serious issue.
On the other hand, how much can one trust in the information correctness
if either of those are wrong.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 01/13] gdb/dwarf: change read_loclist_index complaints into errors
2021-01-28 15:17 ` Zoran Zaric
@ 2021-01-28 15:42 ` Simon Marchi
2021-02-25 19:20 ` Tom Tromey
0 siblings, 1 reply; 34+ messages in thread
From: Simon Marchi @ 2021-01-28 15:42 UTC (permalink / raw)
To: Zoran Zaric, Simon Marchi, gdb-patches
On 2021-01-28 10:17 a.m., Zoran Zaric wrote:
>> From: Simon Marchi <simon.marchi@efficios.com>
>>
>> Unlike read_rnglists_index, read_loclist_index uses complaints when it
>> detects an inconsistency (a DW_FORM_loclistx value without a
>> .debug_loclists section or an offset outside of the section). I really
>> think they should be errors, since there's no point in continuing if
>> this situation happens, we will likely segfault or read garbage.
>>
>
> Makes sense.
>
> Although unless I misunderstand something, isn't the difference here that the error will actually throw an exception and therefore stop reading of the compilation unit if not the whole object file debug information?
>
> If this is the case, then considering the difference in usage between the two, having a wrong loclist information can still provide a correct line table information, but having a wrong rnglist information in my mind creates a more serious issue.
>
> On the other hand, how much can one trust in the information correctness if either of those are wrong.
Indeed, `error` throws an exception that gets handled... I don't know
where. If you are reading partial symtabs, it probably goes up to
dwarf2_build_psymtabs, so it stops the processing for the whole object
file.
I think it's correct from the read_rnglists_index and read_loclist_index
functions point of view to throw if they encounter invalid data and
can't return something meaningful. If we want to make error handling a
bit more granular, we could catch the error in the caller
(read_attribute_reprocess), make it display a complaint, and continue as
if the attribute wasn't present.
We won't have location or range information for this entity (or perhaps
for all entities, if we fail to read all of them), but perhaps other
parts of the debug info will be read fine. And those other parts could
maybe be useful to some user, versus aborting completely and having no
debug info at all.
However, I don't intend to do this at the moment, because it would be
quite a lot of work to do and test properly, in the end to accomodate an
hypothetical buggy compiler. Maybe if/when we have a concrete instance
of a widely available compiler producing such buggy debug info, we can
revisit this idea.
Thanks for the review!
Simon
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 01/13] gdb/dwarf: change read_loclist_index complaints into errors
2021-01-28 15:42 ` Simon Marchi
@ 2021-02-25 19:20 ` Tom Tromey
0 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2021-02-25 19:20 UTC (permalink / raw)
To: Simon Marchi via Gdb-patches; +Cc: Zoran Zaric, Simon Marchi, Simon Marchi
>> If this is the case, then considering the difference in usage between
>> the two, having a wrong loclist information can still provide a
>> correct line table information, but having a wrong rnglist
>> information in my mind creates a more serious issue.
>> On the other hand, how much can one trust in the information
>> correctness if either of those are wrong.
Simon> Indeed, `error` throws an exception that gets handled... I don't know
Simon> where. If you are reading partial symtabs, it probably goes up to
Simon> dwarf2_build_psymtabs, so it stops the processing for the whole object
Simon> file.
FWIW there is at least one bug open about this behavior.
It's not completely clear what would be best. One idea I consider
sometimes is to throw out just the known-bad CU, but try to read the
remaining ones. This might not be too difficult to implement.
Simon> I think it's correct from the read_rnglists_index and read_loclist_index
Simon> functions point of view to throw if they encounter invalid data and
Simon> can't return something meaningful. If we want to make error handling a
Simon> bit more granular, we could catch the error in the caller
Simon> (read_attribute_reprocess), make it display a complaint, and continue as
Simon> if the attribute wasn't present.
Things like this could be considered on a case-by-case basis as well.
Simon> However, I don't intend to do this at the moment, because it would be
Simon> quite a lot of work to do and test properly, in the end to accomodate an
Simon> hypothetical buggy compiler. Maybe if/when we have a concrete instance
Simon> of a widely available compiler producing such buggy debug info, we can
Simon> revisit this idea.
For the plan of just rejecting an individual bad CU, the problem isn't
so much consistently bad compilers as that, when one does trip across
bad DWARF, it makes the entire result unusable -- even if the bug is
localized somehow.
Tom
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 02/13] gdb/dwarf: fix bound check in read_rnglist_index
2021-01-20 5:39 [PATCH 00/13] DWARF 5 rnglists & loclists fixes (PR 26813) Simon Marchi
2021-01-20 5:39 ` [PATCH 01/13] gdb/dwarf: change read_loclist_index complaints into errors Simon Marchi
@ 2021-01-20 5:39 ` Simon Marchi
2021-01-28 15:22 ` Zoran Zaric
2021-01-20 5:39 ` [PATCH 03/13] gdb/dwarf: add missing bound check to read_loclist_index Simon Marchi
` (11 subsequent siblings)
13 siblings, 1 reply; 34+ messages in thread
From: Simon Marchi @ 2021-01-20 5:39 UTC (permalink / raw)
To: gdb-patches; +Cc: Zoran Zaric, Simon Marchi
From: Simon Marchi <simon.marchi@efficios.com>
I think this check in read_rnglist_index is wrong:
/* Validate that reading won't go beyond the end of the section. */
if (start_offset + cu->header.offset_size > rnglist_base + section->size)
error (_("Reading DW_FORM_rnglistx index beyond end of"
".debug_rnglists section [in module %s]"),
objfile_name (objfile));
The addition `rnglist_base + section->size` doesn't make sense.
rnglist_base is an offset into `section`, so it doesn't make sense to
add it to `section`'s size. `start_offset` also is an offset into
`section`, so we should just compare it to just `section->size`.
gdb/ChangeLog:
* dwarf2/read.c (read_rnglist_index): Fix bound check.
Change-Id: If0ff7c73f4f80f79aac447518f4e8f131f2db8f2
---
gdb/dwarf2/read.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 2b76ed001616..f3bc35644c8a 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -20229,6 +20229,8 @@ read_rnglist_index (struct dwarf2_cu *cu, ULONGEST rnglist_index,
: RNGLIST_HEADER_SIZE64);
ULONGEST rnglist_base =
(cu->dwo_unit != nullptr) ? rnglist_header_size : cu->ranges_base;
+
+ /* Offset in .debug_rnglists of the offset for RNGLIST_INDEX. */
ULONGEST start_offset =
rnglist_base + rnglist_index * cu->header.offset_size;
@@ -20257,7 +20259,7 @@ read_rnglist_index (struct dwarf2_cu *cu, ULONGEST rnglist_index,
objfile_name (objfile));
/* Validate that reading won't go beyond the end of the section. */
- if (start_offset + cu->header.offset_size > rnglist_base + section->size)
+ if (start_offset + cu->header.offset_size > section->size)
error (_("Reading DW_FORM_rnglistx index beyond end of"
".debug_rnglists section [in module %s]"),
objfile_name (objfile));
--
2.30.0
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 02/13] gdb/dwarf: fix bound check in read_rnglist_index
2021-01-20 5:39 ` [PATCH 02/13] gdb/dwarf: fix bound check in read_rnglist_index Simon Marchi
@ 2021-01-28 15:22 ` Zoran Zaric
0 siblings, 0 replies; 34+ messages in thread
From: Zoran Zaric @ 2021-01-28 15:22 UTC (permalink / raw)
To: Simon Marchi, gdb-patches; +Cc: Simon Marchi
> From: Simon Marchi <simon.marchi@efficios.com>
>
> I think this check in read_rnglist_index is wrong:
>
> /* Validate that reading won't go beyond the end of the section. */
> if (start_offset + cu->header.offset_size > rnglist_base + section->size)
> error (_("Reading DW_FORM_rnglistx index beyond end of"
> ".debug_rnglists section [in module %s]"),
> objfile_name (objfile));
>
> The addition `rnglist_base + section->size` doesn't make sense.
> rnglist_base is an offset into `section`, so it doesn't make sense to
> add it to `section`'s size. `start_offset` also is an offset into
> `section`, so we should just compare it to just `section->size`.
Completely agree with this change.
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 03/13] gdb/dwarf: add missing bound check to read_loclist_index
2021-01-20 5:39 [PATCH 00/13] DWARF 5 rnglists & loclists fixes (PR 26813) Simon Marchi
2021-01-20 5:39 ` [PATCH 01/13] gdb/dwarf: change read_loclist_index complaints into errors Simon Marchi
2021-01-20 5:39 ` [PATCH 02/13] gdb/dwarf: fix bound check in read_rnglist_index Simon Marchi
@ 2021-01-20 5:39 ` Simon Marchi
2021-01-20 5:39 ` [PATCH 04/13] gdb/dwarf: remove unnecessary check in read_{rng, loc}list_index Simon Marchi
` (10 subsequent siblings)
13 siblings, 0 replies; 34+ messages in thread
From: Simon Marchi @ 2021-01-20 5:39 UTC (permalink / raw)
To: gdb-patches; +Cc: Zoran Zaric, Simon Marchi
From: Simon Marchi <simon.marchi@efficios.com>
read_rnglist_index has a bound check to make sure that we don't go past
the end of the section while reading the offset, but read_loclist_index
doesn't. Add it to read_loclist_index.
gdb/ChangeLog:
* dwarf2/read.c (read_loclist_index): Add bound check for the end
of the offset.
Change-Id: Ic4b55c88860fdc3e007740949c78ec84cdb4da60
---
gdb/dwarf2/read.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index f3bc35644c8a..848c15330435 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -20186,6 +20186,11 @@ read_loclist_index (struct dwarf2_cu *cu, ULONGEST loclist_index)
struct objfile *objfile = per_objfile->objfile;
bfd *abfd = objfile->obfd;
ULONGEST loclist_base = lookup_loclist_base (cu);
+
+ /* Offset in .debug_loclists of the offset for LOCLIST_INDEX. */
+ ULONGEST start_offset =
+ loclist_base + loclist_index * cu->header.offset_size;
+
struct dwarf2_section_info *section = cu_debug_loc_section (cu);
section->read (objfile);
@@ -20200,14 +20205,18 @@ read_loclist_index (struct dwarf2_cu *cu, ULONGEST loclist_index)
".debug_loclists offset array [in module %s]"),
objfile_name (objfile));
- if (loclist_base + loclist_index * cu->header.offset_size
- >= section->size)
+ if (start_offset >= section->size)
error (_("DW_FORM_loclistx pointing outside of "
".debug_loclists section [in module %s]"),
objfile_name (objfile));
- const gdb_byte *info_ptr
- = section->buffer + loclist_base + loclist_index * cu->header.offset_size;
+ /* Validate that reading won't go beyond the end of the section. */
+ if (start_offset + cu->header.offset_size > section->size)
+ error (_("Reading DW_FORM_loclistx index beyond end of"
+ ".debug_loclists section [in module %s]"),
+ objfile_name (objfile));
+
+ const gdb_byte *info_ptr = section->buffer + start_offset;
if (cu->header.offset_size == 4)
return bfd_get_32 (abfd, info_ptr) + loclist_base;
--
2.30.0
^ permalink raw reply [flat|nested] 34+ messages in thread* [PATCH 04/13] gdb/dwarf: remove unnecessary check in read_{rng, loc}list_index
2021-01-20 5:39 [PATCH 00/13] DWARF 5 rnglists & loclists fixes (PR 26813) Simon Marchi
` (2 preceding siblings ...)
2021-01-20 5:39 ` [PATCH 03/13] gdb/dwarf: add missing bound check to read_loclist_index Simon Marchi
@ 2021-01-20 5:39 ` Simon Marchi
2021-01-20 5:39 ` [PATCH 05/13] gdb/dwarf: few fixes for handling DW_FORM_{rng, loc}listx Simon Marchi
` (9 subsequent siblings)
13 siblings, 0 replies; 34+ messages in thread
From: Simon Marchi @ 2021-01-20 5:39 UTC (permalink / raw)
To: gdb-patches; +Cc: Zoran Zaric, Simon Marchi
From: Simon Marchi <simon.marchi@efficios.com>
In read_rnglist_index and read_loclist_index, we check that both the
start and end of the offset that we read from the offset table are
within the section. I think it's unecessary to do both: if the end of
the offset is within the section, then surely the start of the offset is
within it.
Remove the check for the start of the offset in both functions.
gdb/ChangeLog:
* dwarf2/read.c (read_loclist_index): Remove bound check for
start of offset.
(read_rnglist_index): Likewise.
Change-Id: I7b57ddf4f8a8a28971738f0e3f3af62108f9e19a
---
gdb/dwarf2/read.c | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 848c15330435..76044187bf76 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -20205,11 +20205,6 @@ read_loclist_index (struct dwarf2_cu *cu, ULONGEST loclist_index)
".debug_loclists offset array [in module %s]"),
objfile_name (objfile));
- if (start_offset >= section->size)
- error (_("DW_FORM_loclistx pointing outside of "
- ".debug_loclists section [in module %s]"),
- objfile_name (objfile));
-
/* Validate that reading won't go beyond the end of the section. */
if (start_offset + cu->header.offset_size > section->size)
error (_("Reading DW_FORM_loclistx index beyond end of"
@@ -20261,12 +20256,6 @@ read_rnglist_index (struct dwarf2_cu *cu, ULONGEST rnglist_index,
".debug_rnglists offset array [in module %s]"),
objfile_name (objfile));
- /* Validate that the offset is within the section's range. */
- if (start_offset >= section->size)
- error (_("DW_FORM_rnglistx pointing outside of "
- ".debug_rnglists section [in module %s]"),
- objfile_name (objfile));
-
/* Validate that reading won't go beyond the end of the section. */
if (start_offset + cu->header.offset_size > section->size)
error (_("Reading DW_FORM_rnglistx index beyond end of"
--
2.30.0
^ permalink raw reply [flat|nested] 34+ messages in thread* [PATCH 05/13] gdb/dwarf: few fixes for handling DW_FORM_{rng, loc}listx
2021-01-20 5:39 [PATCH 00/13] DWARF 5 rnglists & loclists fixes (PR 26813) Simon Marchi
` (3 preceding siblings ...)
2021-01-20 5:39 ` [PATCH 04/13] gdb/dwarf: remove unnecessary check in read_{rng, loc}list_index Simon Marchi
@ 2021-01-20 5:39 ` Simon Marchi
2021-01-28 15:30 ` [PATCH 05/13] gdb/dwarf: few fixes for handling DW_FORM_{rng,loc}listx Zoran Zaric
2021-01-20 5:39 ` [PATCH 06/13] gdb/dwarf: read correct rnglist/loclist header in read_{rng, loc}list_index Simon Marchi
` (8 subsequent siblings)
13 siblings, 1 reply; 34+ messages in thread
From: Simon Marchi @ 2021-01-20 5:39 UTC (permalink / raw)
To: gdb-patches; +Cc: Zoran Zaric, Simon Marchi
From: Simon Marchi <simon.marchi@efficios.com>
We hit an assertion when loading the binary from PR 26813. When fixing
it, execution goes a up bit further but then hits another assert, and
another, and another. With these fours fixes, I am able to load the
binary and get to the prompt. An error is shown (index pointing outside
of the section), because the DW_FORM_rnglistx attribute is not read
correctly, but that one is taken care of by the next patch.
The four fixes are:
- attribute::form_requires_reprocessing needs to handle forms
DW_FORM_rnglistx and DW_FORM_loclistx, because set_unsigned_reprocess
is called for them in read_attribute_value.
- read_attribute_reprocess must call set_unsigned for them, not
set_address. The parameter of set_address is a CORE_ADDR, meaning
it's for program addresses. Post-reprocess, DW_FORM_rnglistx and
DW_FORM_loclistx are offsets into their respective sections
(.debug_rnglists and .debug_loclists). set_unsigned is the current
attribute value setter that fits the best. But perhaps we should have
a setter that takes a sect_offset?
- read_attribute_process must call as_unsigned_reprocess instead of
as_unsigned to get the pre-reprocess value, otherwise we hit the
assert inside as_unsigned that makes sure the attribute doesn't need
reprocessing.
- attribute::set_unsigned needs to clear the requires_reprocessing flag,
otherwise it stays set when reprocessing DW_FORM_rnglistx and
DW_FORM_loclistx attributes.
There's another assert that we hit once the next patch is applied, but
since it's in the same vein as the changes in this patch, I included it
in this patch:
- attribute::form_is_unsigned must handle form DW_FORM_loclistx,
otherwise we hit the assert when trying to call set_unsigned for an
attribute of this form. DW_FORM_rnglistx is already handled.
gdb/ChangeLog:
PR gdb/26813
* dwarf2/attribute.h (struct attribute) <set_unsigned>: Clear
requires_reprocessing flag.
* dwarf2/attribute.c (attribute::form_is_unsigned): Handle
DW_FORM_loclistx.
(attribute::form_requires_reprocessing): Handle DW_FORM_rnglistx
and DW_FORM_loclistx.
* dwarf2/read.c (read_attribute_reprocess): Use set_unsigned
instead of set_address for DW_FORM_loclistx and
DW_FORM_rnglistx.
Change-Id: I06c156fa3913ca98e4e39085f4ef171645b4bc1e
---
gdb/dwarf2/attribute.c | 5 ++++-
gdb/dwarf2/attribute.h | 1 +
gdb/dwarf2/read.c | 16 +++++++++++++---
3 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/gdb/dwarf2/attribute.c b/gdb/dwarf2/attribute.c
index 479261030c5d..b4f188a096e1 100644
--- a/gdb/dwarf2/attribute.c
+++ b/gdb/dwarf2/attribute.c
@@ -179,6 +179,7 @@ attribute::form_is_unsigned () const
|| form == DW_FORM_flag_present
|| form == DW_FORM_udata
|| form == DW_FORM_rnglistx
+ || form == DW_FORM_loclistx
|| form == DW_FORM_ref1
|| form == DW_FORM_ref2
|| form == DW_FORM_ref4
@@ -197,7 +198,9 @@ attribute::form_requires_reprocessing () const
|| form == DW_FORM_strx4
|| form == DW_FORM_GNU_str_index
|| form == DW_FORM_addrx
- || form == DW_FORM_GNU_addr_index);
+ || form == DW_FORM_GNU_addr_index
+ || form == DW_FORM_rnglistx
+ || form == DW_FORM_loclistx);
}
/* See attribute.h. */
diff --git a/gdb/dwarf2/attribute.h b/gdb/dwarf2/attribute.h
index a3ff9b0eb9c6..56776d64ed34 100644
--- a/gdb/dwarf2/attribute.h
+++ b/gdb/dwarf2/attribute.h
@@ -223,6 +223,7 @@ struct attribute
{
gdb_assert (form_is_unsigned ());
u.unsnd = unsnd;
+ requires_reprocessing = 0;
}
/* Temporarily set this attribute to an unsigned integer. This is
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 76044187bf76..4146694247ea 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -20287,10 +20287,20 @@ read_attribute_reprocess (const struct die_reader_specs *reader,
attr->as_unsigned_reprocess ()));
break;
case DW_FORM_loclistx:
- attr->set_address (read_loclist_index (cu, attr->as_unsigned ()));
- break;
+ {
+ CORE_ADDR loclists_sect_off
+ = read_loclist_index (cu, attr->as_unsigned_reprocess ());
+
+ attr->set_unsigned (loclists_sect_off);
+ }
+ break;
case DW_FORM_rnglistx:
- attr->set_address (read_rnglist_index (cu, attr->as_unsigned (), tag));
+ {
+ CORE_ADDR rnglists_sect_off
+ = read_rnglist_index (cu, attr->as_unsigned_reprocess (), tag);
+
+ attr->set_unsigned (rnglists_sect_off);
+ }
break;
case DW_FORM_strx:
case DW_FORM_strx1:
--
2.30.0
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 05/13] gdb/dwarf: few fixes for handling DW_FORM_{rng,loc}listx
2021-01-20 5:39 ` [PATCH 05/13] gdb/dwarf: few fixes for handling DW_FORM_{rng, loc}listx Simon Marchi
@ 2021-01-28 15:30 ` Zoran Zaric
0 siblings, 0 replies; 34+ messages in thread
From: Zoran Zaric @ 2021-01-28 15:30 UTC (permalink / raw)
To: Simon Marchi, gdb-patches; +Cc: Simon Marchi
> From: Simon Marchi <simon.marchi@efficios.com>
>
> We hit an assertion when loading the binary from PR 26813. When fixing
> it, execution goes a up bit further but then hits another assert, and
> another, and another. With these fours fixes, I am able to load the
> binary and get to the prompt. An error is shown (index pointing outside
> of the section), because the DW_FORM_rnglistx attribute is not read
> correctly, but that one is taken care of by the next patch.
>
> The four fixes are:
>
> - attribute::form_requires_reprocessing needs to handle forms
> DW_FORM_rnglistx and DW_FORM_loclistx, because set_unsigned_reprocess
> is called for them in read_attribute_value.
>
> - read_attribute_reprocess must call set_unsigned for them, not
> set_address. The parameter of set_address is a CORE_ADDR, meaning
> it's for program addresses. Post-reprocess, DW_FORM_rnglistx and
> DW_FORM_loclistx are offsets into their respective sections
> (.debug_rnglists and .debug_loclists). set_unsigned is the current
> attribute value setter that fits the best. But perhaps we should have
> a setter that takes a sect_offset?
>
> - read_attribute_process must call as_unsigned_reprocess instead of
> as_unsigned to get the pre-reprocess value, otherwise we hit the
> assert inside as_unsigned that makes sure the attribute doesn't need
> reprocessing.
>
> - attribute::set_unsigned needs to clear the requires_reprocessing flag,
> otherwise it stays set when reprocessing DW_FORM_rnglistx and
> DW_FORM_loclistx attributes.
>
> There's another assert that we hit once the next patch is applied, but
> since it's in the same vein as the changes in this patch, I included it
> in this patch:
>
> - attribute::form_is_unsigned must handle form DW_FORM_loclistx,
> otherwise we hit the assert when trying to call set_unsigned for an
> attribute of this form. DW_FORM_rnglistx is already handled.
>
Agreed.
I came to the same conclusion while investigating this issue, but
unfortunately, didn't have the time to figure out how to fix the
following asserts.
Zoran
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 06/13] gdb/dwarf: read correct rnglist/loclist header in read_{rng, loc}list_index
2021-01-20 5:39 [PATCH 00/13] DWARF 5 rnglists & loclists fixes (PR 26813) Simon Marchi
` (4 preceding siblings ...)
2021-01-20 5:39 ` [PATCH 05/13] gdb/dwarf: few fixes for handling DW_FORM_{rng, loc}listx Simon Marchi
@ 2021-01-20 5:39 ` Simon Marchi
2021-01-28 15:39 ` [PATCH 06/13] gdb/dwarf: read correct rnglist/loclist header in read_{rng,loc}list_index Zoran Zaric
2021-01-20 5:39 ` [PATCH 07/13] gdb/dwarf: read DW_AT_ranges value as unsigned in partial_die_info::read Simon Marchi
` (7 subsequent siblings)
13 siblings, 1 reply; 34+ messages in thread
From: Simon Marchi @ 2021-01-20 5:39 UTC (permalink / raw)
To: gdb-patches; +Cc: Zoran Zaric, Simon Marchi
From: Simon Marchi <simon.marchi@efficios.com>
When loading the binary from PR 26813 in GDB, we get:
DW_FORM_rnglistx index pointing outside of .debug_rnglists offset array [in module /home/simark/build/binutils-gdb/gdb/MagicPurse]
... and the symbols fail to load.
In read_rnglist_index and read_loclist_index, we read the header
(documented in sections 7.28 and 7.29 of DWARF 5) of the CU's
contribution to the .debug_rnglists / .debug_loclists sections to
validate that the index we want to read makes sense. However, we always
read the header at the beginning of the section, rather than the header
for the contribution from which we want to read the index.
To illustrate, here's what the binary from PR 26813 contains. There are
two compile units:
0x0000000c: DW_TAG_compile_unit 1
DW_AT_ranges [DW_FORM_rnglistx]: 0x0
DW_AT_rnglists_base [DW_FORM_sec_offset]: 0xC
0x00003ec9: DW_TAG_compile_unit 2
DW_AT_ranges [DW_FORM_rnglistx]: 0xB
DW_AT_rnglists_base [DW_FORM_sec_offset]: 0x85
The layout of the .debug_rnglists is the following:
[0x00, 0x0B]: header for CU 1's contribution
[0x0C, 0x0F]: list of offsets for CU 1 (1 element)
[0x10, 0x78]: range lists data for CU 1
[0x79, 0x84]: header for CU 2's contribution
[0x85, 0xB4]: list of offsets for CU 2 (12 elements)
[0xB5, 0xBD7]: range lists data for CU 2
The DW_AT_rnglists_base attrbute points to the beginning of the list of
offsets for that CU, relative to the start of the .debug_rnglists
section. That's right after the header for that contribution.
When we try to read the DW_AT_ranges attribute for CU 2,
read_rnglist_index reads the header for CU 1 instead of the one for CU
2. Since there's only one element in CU 1's offset list, it believes
(wrongfully) that the index 0xB is out of range.
Fix it by reading the header just before where DW_AT_rnglists_base
points to. With this patch, I am able to load GDB built with clang-11
and -gdwarf-5 in itself, with and without -readnow.
gdb/ChangeLog:
PR gdb/26813
* dwarf2/read.c (read_loclists_rnglists_header): Add
header_offset parameter and use it.
(read_loclist_index): Read header of the current contribution,
not the one at the beginning of the section.
(read_rnglist_index): Likewise.
Change-Id: Ie53ff8251af8c1556f0a83a31aa8572044b79e3d
---
gdb/dwarf2/read.c | 44 ++++++++++++++++++++++++++++++++++++++------
1 file changed, 38 insertions(+), 6 deletions(-)
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 4146694247ea..9701d422a338 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -20136,22 +20136,30 @@ partial_die_info::fixup (struct dwarf2_cu *cu)
}
/* Read the .debug_loclists or .debug_rnglists header (they are the same format)
- contents from the given SECTION in the HEADER. */
+ contents from the given SECTION in the HEADER.
+
+ HEADER_OFFSET is the offset of the header in the section. */
static void
read_loclists_rnglists_header (struct loclists_rnglists_header *header,
- struct dwarf2_section_info *section)
+ struct dwarf2_section_info *section,
+ sect_offset header_offset)
{
unsigned int bytes_read;
bfd *abfd = section->get_bfd_owner ();
- const gdb_byte *info_ptr = section->buffer;
+ const gdb_byte *info_ptr = section->buffer + to_underlying (header_offset);
+
header->length = read_initial_length (abfd, info_ptr, &bytes_read);
info_ptr += bytes_read;
+
header->version = read_2_bytes (abfd, info_ptr);
info_ptr += 2;
+
header->addr_size = read_1_byte (abfd, info_ptr);
info_ptr += 1;
+
header->segment_collector_size = read_1_byte (abfd, info_ptr);
info_ptr += 1;
+
header->offset_entry_count = read_4_bytes (abfd, info_ptr);
}
@@ -20185,21 +20193,36 @@ read_loclist_index (struct dwarf2_cu *cu, ULONGEST loclist_index)
dwarf2_per_objfile *per_objfile = cu->per_objfile;
struct objfile *objfile = per_objfile->objfile;
bfd *abfd = objfile->obfd;
+ ULONGEST loclist_header_size =
+ (cu->header.initial_length_size == 4 ? LOCLIST_HEADER_SIZE32
+ : LOCLIST_HEADER_SIZE64);
ULONGEST loclist_base = lookup_loclist_base (cu);
/* Offset in .debug_loclists of the offset for LOCLIST_INDEX. */
ULONGEST start_offset =
loclist_base + loclist_index * cu->header.offset_size;
+ /* Get loclists section. */
struct dwarf2_section_info *section = cu_debug_loc_section (cu);
+ /* Read the loclists section content. */
section->read (objfile);
if (section->buffer == NULL)
error (_("DW_FORM_loclistx used without .debug_loclists "
"section [in module %s]"), objfile_name (objfile));
+ /* DW_AT_loclists_base points after the .debug_loclists contribution header,
+ so if loclist_base is smaller than the header size, we have a problem. */
+ if (loclist_base < loclist_header_size)
+ error (_("DW_AT_loclists_base is smaller than header size [in module %s]"),
+ objfile_name (objfile));
+
+ /* Read the header of the loclists contribution. */
struct loclists_rnglists_header header;
- read_loclists_rnglists_header (&header, section);
+ read_loclists_rnglists_header (&header, section,
+ (sect_offset) (loclist_base - loclist_header_size));
+
+ /* Verify the loclist index is valid. */
if (loclist_index >= header.offset_entry_count)
error (_("DW_FORM_loclistx pointing outside of "
".debug_loclists offset array [in module %s]"),
@@ -20248,9 +20271,18 @@ read_rnglist_index (struct dwarf2_cu *cu, ULONGEST rnglist_index,
"[in module %s]"),
objfile_name (objfile));
- /* Verify the rnglist index is valid. */
+ /* DW_AT_rnglists_base points after the .debug_rnglists contribution header,
+ so if rnglist_base is smaller than the header size, we have a problem. */
+ if (rnglist_base < rnglist_header_size)
+ error (_("DW_AT_rnglists_base is smaller than header size [in module %s]"),
+ objfile_name (objfile));
+
+ /* Read the header of the rnglists contribution. */
struct loclists_rnglists_header header;
- read_loclists_rnglists_header (&header, section);
+ read_loclists_rnglists_header (&header, section,
+ (sect_offset) (rnglist_base - rnglist_header_size));
+
+ /* Verify the rnglist index is valid. */
if (rnglist_index >= header.offset_entry_count)
error (_("DW_FORM_rnglistx index pointing outside of "
".debug_rnglists offset array [in module %s]"),
--
2.30.0
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 06/13] gdb/dwarf: read correct rnglist/loclist header in read_{rng,loc}list_index
2021-01-20 5:39 ` [PATCH 06/13] gdb/dwarf: read correct rnglist/loclist header in read_{rng, loc}list_index Simon Marchi
@ 2021-01-28 15:39 ` Zoran Zaric
2021-01-28 15:49 ` Simon Marchi
0 siblings, 1 reply; 34+ messages in thread
From: Zoran Zaric @ 2021-01-28 15:39 UTC (permalink / raw)
To: Simon Marchi, gdb-patches; +Cc: Simon Marchi
> From: Simon Marchi <simon.marchi@efficios.com>
>
> When loading the binary from PR 26813 in GDB, we get:
>
> DW_FORM_rnglistx index pointing outside of .debug_rnglists offset array [in module /home/simark/build/binutils-gdb/gdb/MagicPurse]
>
> ... and the symbols fail to load.
>
> In read_rnglist_index and read_loclist_index, we read the header
> (documented in sections 7.28 and 7.29 of DWARF 5) of the CU's
> contribution to the .debug_rnglists / .debug_loclists sections to
> validate that the index we want to read makes sense. However, we always
> read the header at the beginning of the section, rather than the header
> for the contribution from which we want to read the index.
>
> To illustrate, here's what the binary from PR 26813 contains. There are
> two compile units:
>
> 0x0000000c: DW_TAG_compile_unit 1
> DW_AT_ranges [DW_FORM_rnglistx]: 0x0
> DW_AT_rnglists_base [DW_FORM_sec_offset]: 0xC
>
> 0x00003ec9: DW_TAG_compile_unit 2
> DW_AT_ranges [DW_FORM_rnglistx]: 0xB
> DW_AT_rnglists_base [DW_FORM_sec_offset]: 0x85
>
> The layout of the .debug_rnglists is the following:
>
> [0x00, 0x0B]: header for CU 1's contribution
> [0x0C, 0x0F]: list of offsets for CU 1 (1 element)
> [0x10, 0x78]: range lists data for CU 1
>
> [0x79, 0x84]: header for CU 2's contribution
> [0x85, 0xB4]: list of offsets for CU 2 (12 elements)
> [0xB5, 0xBD7]: range lists data for CU 2
>
> The DW_AT_rnglists_base attrbute points to the beginning of the list of
> offsets for that CU, relative to the start of the .debug_rnglists
> section. That's right after the header for that contribution.
>
> When we try to read the DW_AT_ranges attribute for CU 2,
> read_rnglist_index reads the header for CU 1 instead of the one for CU
> 2. Since there's only one element in CU 1's offset list, it believes
> (wrongfully) that the index 0xB is out of range.
>
> Fix it by reading the header just before where DW_AT_rnglists_base
> points to. With this patch, I am able to load GDB built with clang-11
> and -gdwarf-5 in itself, with and without -readnow.
>
I came to the same conclusion, the issue was probably never noticed
because the tested compilers only ever generated one rangelist entry.
Even LLVM didn't seem to generate it before version 11, or at least I
haven't noticed the issue when I was previously testing the DWARF 5 support.
>
> Change-Id: Ie53ff8251af8c1556f0a83a31aa8572044b79e3d
>
Are these Gerrit change ID's? They shouldn't be part of the patches right?
Zoran
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 06/13] gdb/dwarf: read correct rnglist/loclist header in read_{rng,loc}list_index
2021-01-28 15:39 ` [PATCH 06/13] gdb/dwarf: read correct rnglist/loclist header in read_{rng,loc}list_index Zoran Zaric
@ 2021-01-28 15:49 ` Simon Marchi
2021-01-28 15:54 ` Zoran Zaric
0 siblings, 1 reply; 34+ messages in thread
From: Simon Marchi @ 2021-01-28 15:49 UTC (permalink / raw)
To: Zoran Zaric, gdb-patches; +Cc: Simon Marchi
On 2021-01-28 10:39 a.m., Zoran Zaric wrote:>> From: Simon Marchi <simon.marchi@efficios.com>
>>
>> When loading the binary from PR 26813 in GDB, we get:
>>
>> DW_FORM_rnglistx index pointing outside of .debug_rnglists offset array [in module /home/simark/build/binutils-gdb/gdb/MagicPurse]
>>
>> ... and the symbols fail to load.
>>
>> In read_rnglist_index and read_loclist_index, we read the header
>> (documented in sections 7.28 and 7.29 of DWARF 5) of the CU's
>> contribution to the .debug_rnglists / .debug_loclists sections to
>> validate that the index we want to read makes sense. However, we always
>> read the header at the beginning of the section, rather than the header
>> for the contribution from which we want to read the index.
>>
>> To illustrate, here's what the binary from PR 26813 contains. There are
>> two compile units:
>>
>> 0x0000000c: DW_TAG_compile_unit 1
>> DW_AT_ranges [DW_FORM_rnglistx]: 0x0
>> DW_AT_rnglists_base [DW_FORM_sec_offset]: 0xC
>>
>> 0x00003ec9: DW_TAG_compile_unit 2
>> DW_AT_ranges [DW_FORM_rnglistx]: 0xB
>> DW_AT_rnglists_base [DW_FORM_sec_offset]: 0x85
>>
>> The layout of the .debug_rnglists is the following:
>>
>> [0x00, 0x0B]: header for CU 1's contribution
>> [0x0C, 0x0F]: list of offsets for CU 1 (1 element)
>> [0x10, 0x78]: range lists data for CU 1
>>
>> [0x79, 0x84]: header for CU 2's contribution
>> [0x85, 0xB4]: list of offsets for CU 2 (12 elements)
>> [0xB5, 0xBD7]: range lists data for CU 2
>>
>> The DW_AT_rnglists_base attrbute points to the beginning of the list of
>> offsets for that CU, relative to the start of the .debug_rnglists
>> section. That's right after the header for that contribution.
>>
>> When we try to read the DW_AT_ranges attribute for CU 2,
>> read_rnglist_index reads the header for CU 1 instead of the one for CU
>> 2. Since there's only one element in CU 1's offset list, it believes
>> (wrongfully) that the index 0xB is out of range.
>>
>> Fix it by reading the header just before where DW_AT_rnglists_base
>> points to. With this patch, I am able to load GDB built with clang-11
>> and -gdwarf-5 in itself, with and without -readnow.
>>
>
> I came to the same conclusion, the issue was probably never noticed because the tested compilers only ever generated one rangelist entry.
>
> Even LLVM didn't seem to generate it before version 11, or at least I haven't noticed the issue when I was previously testing the DWARF 5 support.
Indeed, and I think it was only used on the compilation unit's DIE?
>>
>> Change-Id: Ie53ff8251af8c1556f0a83a31aa8572044b79e3d
>>
>
> Are these Gerrit change ID's? They shouldn't be part of the patches right?
>
> Zoran
I do use Gerrit personally to track the patches I work on, their
versions, which ones have been merged and which ones haven't. When I
push the patches upstream and then sync my Gerrit instance's master
branch, it automatically sees which patches have been merged in the
master branch and closes the corresponding reviews. So keeping the
Gerrit Change-Id really helps me, and I don't think it bothers anyone,
so I leave them there.
Simon
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 06/13] gdb/dwarf: read correct rnglist/loclist header in read_{rng,loc}list_index
2021-01-28 15:49 ` Simon Marchi
@ 2021-01-28 15:54 ` Zoran Zaric
0 siblings, 0 replies; 34+ messages in thread
From: Zoran Zaric @ 2021-01-28 15:54 UTC (permalink / raw)
To: Simon Marchi, gdb-patches; +Cc: Simon Marchi
> On 2021-01-28 10:39 a.m., Zoran Zaric wrote:>> From: Simon Marchi <simon.marchi@efficios.com>
>>>
>>> When loading the binary from PR 26813 in GDB, we get:
>>>
>>> DW_FORM_rnglistx index pointing outside of .debug_rnglists offset array [in module /home/simark/build/binutils-gdb/gdb/MagicPurse]
>>>
>>> ... and the symbols fail to load.
>>>
>>> In read_rnglist_index and read_loclist_index, we read the header
>>> (documented in sections 7.28 and 7.29 of DWARF 5) of the CU's
>>> contribution to the .debug_rnglists / .debug_loclists sections to
>>> validate that the index we want to read makes sense. However, we always
>>> read the header at the beginning of the section, rather than the header
>>> for the contribution from which we want to read the index.
>>>
>>> To illustrate, here's what the binary from PR 26813 contains. There are
>>> two compile units:
>>>
>>> 0x0000000c: DW_TAG_compile_unit 1
>>> DW_AT_ranges [DW_FORM_rnglistx]: 0x0
>>> DW_AT_rnglists_base [DW_FORM_sec_offset]: 0xC
>>>
>>> 0x00003ec9: DW_TAG_compile_unit 2
>>> DW_AT_ranges [DW_FORM_rnglistx]: 0xB
>>> DW_AT_rnglists_base [DW_FORM_sec_offset]: 0x85
>>>
>>> The layout of the .debug_rnglists is the following:
>>>
>>> [0x00, 0x0B]: header for CU 1's contribution
>>> [0x0C, 0x0F]: list of offsets for CU 1 (1 element)
>>> [0x10, 0x78]: range lists data for CU 1
>>>
>>> [0x79, 0x84]: header for CU 2's contribution
>>> [0x85, 0xB4]: list of offsets for CU 2 (12 elements)
>>> [0xB5, 0xBD7]: range lists data for CU 2
>>>
>>> The DW_AT_rnglists_base attrbute points to the beginning of the list of
>>> offsets for that CU, relative to the start of the .debug_rnglists
>>> section. That's right after the header for that contribution.
>>>
>>> When we try to read the DW_AT_ranges attribute for CU 2,
>>> read_rnglist_index reads the header for CU 1 instead of the one for CU
>>> 2. Since there's only one element in CU 1's offset list, it believes
>>> (wrongfully) that the index 0xB is out of range.
>>>
>>> Fix it by reading the header just before where DW_AT_rnglists_base
>>> points to. With this patch, I am able to load GDB built with clang-11
>>> and -gdwarf-5 in itself, with and without -readnow.
>>>
>>
>> I came to the same conclusion, the issue was probably never noticed because the tested compilers only ever generated one rangelist entry.
>>
>> Even LLVM didn't seem to generate it before version 11, or at least I haven't noticed the issue when I was previously testing the DWARF 5 support.
>
> Indeed, and I think it was only used on the compilation unit's DIE?
I think you are right.
>>> Change-Id: Ie53ff8251af8c1556f0a83a31aa8572044b79e3d
>>>
>>
>> Are these Gerrit change ID's? They shouldn't be part of the patches right?
>>
>> Zoran
>
> I do use Gerrit personally to track the patches I work on, their
> versions, which ones have been merged and which ones haven't. When I
> push the patches upstream and then sync my Gerrit instance's master
> branch, it automatically sees which patches have been merged in the
> master branch and closes the corresponding reviews. So keeping the
> Gerrit Change-Id really helps me, and I don't think it bothers anyone,
> so I leave them there.
>
> Simon
>
Makes sense. I didn't notice change ID's on some other reviews so I
wasn't sure what is the policy about those.
Zoran
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 07/13] gdb/dwarf: read DW_AT_ranges value as unsigned in partial_die_info::read
2021-01-20 5:39 [PATCH 00/13] DWARF 5 rnglists & loclists fixes (PR 26813) Simon Marchi
` (5 preceding siblings ...)
2021-01-20 5:39 ` [PATCH 06/13] gdb/dwarf: read correct rnglist/loclist header in read_{rng, loc}list_index Simon Marchi
@ 2021-01-20 5:39 ` Simon Marchi
2021-01-28 15:41 ` Zoran Zaric
2021-01-20 5:39 ` [PATCH 08/13] gdb/testsuite: add .debug_rnglists tests Simon Marchi
` (6 subsequent siblings)
13 siblings, 1 reply; 34+ messages in thread
From: Simon Marchi @ 2021-01-20 5:39 UTC (permalink / raw)
To: gdb-patches; +Cc: Zoran Zaric, Simon Marchi
From: Simon Marchi <simon.marchi@efficios.com>
While writing a test for this series, I made a function
(DW_AT_subprogram) with a DW_AT_ranges attribute using the
DW_FORM_rnglistx form:
0x00000012: DW_TAG_subprogram
DW_AT_name [DW_FORM_string] ("foo")
DW_AT_ranges [DW_FORM_rnglistx] (indexed (0x0) rangelist = 0x00000036
[0x0000000000004000, 0x0000000000005000))
And strangely I couldn't print it:
(gdb) p foo
No symbol "foo" in current context.
This is because of the `attr.constant_value (0)` in the DW_AT_ranges
handling of partial_die_info::read. Since DW_FORM_rnglistx is not
recognized as a constant value by attribute::constant_value, the default
value (0) is returned. Down the line, this causes
dwarf2_rnglists_process to try read a range list at offset 0 in the
.debug_rnglists section, which is obviously wrong. In the end, no
symbol is created for foo because we didn't find an address range.
Use attr->as_unsigned instead. This is what is done for the equivalent
code in dwarf2_get_pc_bounds. With this, GDB processes the subprogram
DIE and we are able to print the function symbol:
(gdb) p foo
$1 = {void (void)} 0x4000
Note that I didn't see an actual compiler use DW_FORM_rnglistx for a
subprogram's range. However, in the binary attached to PR 26813, there
are some lexical blocks with it:
0x0000d34a: DW_TAG_lexical_block
DW_AT_ranges [DW_FORM_rnglistx] (indexed (0x2) rangelist = 0x000000d4
[0x0000000000005db1, 0x0000000000005e36)
[0x0000000000005e48, 0x0000000000005f3c)
[0x0000000000006045, 0x0000000000006053))
Their ranges are read incorrectly just like the ranges of the
subprograms. With this patch applied, they are read correctly.
gdb/ChangeLog:
* dwarf2/read.c (partial_die_info::read): Use
attribute::as_unsigned instead of attribute::constant_value
for DW_AT_ranges.
Change-Id: I285381a15588574058d934bbc88f06029e8a3bd1
---
gdb/dwarf2/read.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 9701d422a338..5277e2a09e3f 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -19805,7 +19805,7 @@ partial_die_info::read (const struct die_reader_specs *reader,
/* It would be nice to reuse dwarf2_get_pc_bounds here,
but that requires a full DIE, so instead we just
reimplement it. */
- unsigned int ranges_offset = (attr.constant_value (0)
+ unsigned int ranges_offset = (attr.as_unsigned ()
+ (need_ranges_base
? cu->ranges_base
: 0));
--
2.30.0
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 07/13] gdb/dwarf: read DW_AT_ranges value as unsigned in partial_die_info::read
2021-01-20 5:39 ` [PATCH 07/13] gdb/dwarf: read DW_AT_ranges value as unsigned in partial_die_info::read Simon Marchi
@ 2021-01-28 15:41 ` Zoran Zaric
2021-01-28 15:51 ` Simon Marchi
0 siblings, 1 reply; 34+ messages in thread
From: Zoran Zaric @ 2021-01-28 15:41 UTC (permalink / raw)
To: Simon Marchi, gdb-patches; +Cc: Simon Marchi
> From: Simon Marchi <simon.marchi@efficios.com>
>
> While writing a test for this series, I made a function
> (DW_AT_subprogram) with a DW_AT_ranges attribute using the
> DW_FORM_rnglistx form:
>
> 0x00000012: DW_TAG_subprogram
> DW_AT_name [DW_FORM_string] ("foo")
> DW_AT_ranges [DW_FORM_rnglistx] (indexed (0x0) rangelist = 0x00000036
> [0x0000000000004000, 0x0000000000005000))
>
> And strangely I couldn't print it:
>
> (gdb) p foo
> No symbol "foo" in current context.
>
> This is because of the `attr.constant_value (0)` in the DW_AT_ranges
> handling of partial_die_info::read. Since DW_FORM_rnglistx is not
> recognized as a constant value by attribute::constant_value, the default
> value (0) is returned. Down the line, this causes
> dwarf2_rnglists_process to try read a range list at offset 0 in the
> .debug_rnglists section, which is obviously wrong. In the end, no
> symbol is created for foo because we didn't find an address range.
>
> Use attr->as_unsigned instead. This is what is done for the equivalent
> code in dwarf2_get_pc_bounds. With this, GDB processes the subprogram
> DIE and we are able to print the function symbol:
>
> (gdb) p foo
> $1 = {void (void)} 0x4000
>
> Note that I didn't see an actual compiler use DW_FORM_rnglistx for a
> subprogram's range. However, in the binary attached to PR 26813, there
> are some lexical blocks with it:
>
> 0x0000d34a: DW_TAG_lexical_block
> DW_AT_ranges [DW_FORM_rnglistx] (indexed (0x2) rangelist = 0x000000d4
> [0x0000000000005db1, 0x0000000000005e36)
> [0x0000000000005e48, 0x0000000000005f3c)
> [0x0000000000006045, 0x0000000000006053))
>
> Their ranges are read incorrectly just like the ranges of the
> subprograms. With this patch applied, they are read correctly.
>
Good find.
Zoran
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 07/13] gdb/dwarf: read DW_AT_ranges value as unsigned in partial_die_info::read
2021-01-28 15:41 ` Zoran Zaric
@ 2021-01-28 15:51 ` Simon Marchi
0 siblings, 0 replies; 34+ messages in thread
From: Simon Marchi @ 2021-01-28 15:51 UTC (permalink / raw)
To: Zoran Zaric, gdb-patches; +Cc: Simon Marchi
On 2021-01-28 10:41 a.m., Zoran Zaric wrote:
>> From: Simon Marchi <simon.marchi@efficios.com>
>>
>> While writing a test for this series, I made a function
>> (DW_AT_subprogram) with a DW_AT_ranges attribute using the
>> DW_FORM_rnglistx form:
>>
>> 0x00000012: DW_TAG_subprogram
>> DW_AT_name [DW_FORM_string] ("foo")
>> DW_AT_ranges [DW_FORM_rnglistx] (indexed (0x0) rangelist = 0x00000036
>> [0x0000000000004000, 0x0000000000005000))
>>
>> And strangely I couldn't print it:
>>
>> (gdb) p foo
>> No symbol "foo" in current context.
>>
>> This is because of the `attr.constant_value (0)` in the DW_AT_ranges
>> handling of partial_die_info::read. Since DW_FORM_rnglistx is not
>> recognized as a constant value by attribute::constant_value, the default
>> value (0) is returned. Down the line, this causes
>> dwarf2_rnglists_process to try read a range list at offset 0 in the
>> .debug_rnglists section, which is obviously wrong. In the end, no
>> symbol is created for foo because we didn't find an address range.
>>
>> Use attr->as_unsigned instead. This is what is done for the equivalent
>> code in dwarf2_get_pc_bounds. With this, GDB processes the subprogram
>> DIE and we are able to print the function symbol:
>>
>> (gdb) p foo
>> $1 = {void (void)} 0x4000
>>
>> Note that I didn't see an actual compiler use DW_FORM_rnglistx for a
>> subprogram's range. However, in the binary attached to PR 26813, there
>> are some lexical blocks with it:
>>
>> 0x0000d34a: DW_TAG_lexical_block
>> DW_AT_ranges [DW_FORM_rnglistx] (indexed (0x2) rangelist = 0x000000d4
>> [0x0000000000005db1, 0x0000000000005e36)
>> [0x0000000000005e48, 0x0000000000005f3c)
>> [0x0000000000006045, 0x0000000000006053))
>>
>> Their ranges are read incorrectly just like the ranges of the
>> subprograms. With this patch applied, they are read correctly.
>>
>
> Good find.
>
> Zoran
Note that an identical change was already merged:
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=9f6c202e573b796a9e7328cc5fdab7c0c63776bf
So this patch will probably disappear when I rebase.
Simon
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 08/13] gdb/testsuite: add .debug_rnglists tests
2021-01-20 5:39 [PATCH 00/13] DWARF 5 rnglists & loclists fixes (PR 26813) Simon Marchi
` (6 preceding siblings ...)
2021-01-20 5:39 ` [PATCH 07/13] gdb/dwarf: read DW_AT_ranges value as unsigned in partial_die_info::read Simon Marchi
@ 2021-01-20 5:39 ` Simon Marchi
2021-01-28 16:24 ` Zoran Zaric
2021-01-20 5:39 ` [PATCH 09/13] gdb/testsuite: DWARF assembler: add context parameters to _location Simon Marchi
` (5 subsequent siblings)
13 siblings, 1 reply; 34+ messages in thread
From: Simon Marchi @ 2021-01-20 5:39 UTC (permalink / raw)
To: gdb-patches; +Cc: Zoran Zaric, Simon Marchi
From: Simon Marchi <simon.marchi@efficios.com>
Add tests for the various issues fixed in the previous patches.
Add a new "rnglists" procedure to the DWARF assembler, to allow
generating .debug_rnglists sections. A trivial change is required to
support the DWARF 5 CU header layout.
gdb/testsuite/ChangeLog:
* lib/dwarf.exp (_handle_DW_FORM): Handle DW_FORM_rnglistx.
(cu): Generate header for DWARF 5.
(rnglists): New proc.
* gdb.dwarf2/rnglists-multiple-cus.exp: New.
* gdb.dwarf2/rnglists-sec-offset.exp: New.
Change-Id: I5b297e59c370c60cf671dec19796a6c3b9a9f632
---
.../gdb.dwarf2/rnglists-multiple-cus.exp | 102 ++++++++++
.../gdb.dwarf2/rnglists-sec-offset.exp | 80 ++++++++
gdb/testsuite/lib/dwarf.exp | 187 +++++++++++++++++-
3 files changed, 366 insertions(+), 3 deletions(-)
create mode 100644 gdb/testsuite/gdb.dwarf2/rnglists-multiple-cus.exp
create mode 100644 gdb/testsuite/gdb.dwarf2/rnglists-sec-offset.exp
diff --git a/gdb/testsuite/gdb.dwarf2/rnglists-multiple-cus.exp b/gdb/testsuite/gdb.dwarf2/rnglists-multiple-cus.exp
new file mode 100644
index 000000000000..e09cd4e8fe73
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/rnglists-multiple-cus.exp
@@ -0,0 +1,102 @@
+# Copyright 2020 Free Software Foundation, Inc.
+
+# 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 <http://www.gnu.org/licenses/>.
+
+# Test to reproduce the crash described in PR 26813.
+#
+# When reading a list in any table in the .debug_rnglists section, GDB would
+# read the header at offset 0 in the section (the header of the first table).
+# When the index of the list we read was greater than the number of lists of
+# the first table, GDB would erroneously report that the index is invalid.
+#
+# So this test creates a .debug_rnglists section with two tables. The second
+# table has more lists than the first one and we try to read a high index in
+# the second table.
+
+load_lib dwarf.exp
+
+if {![dwarf2_support]} {
+ return 0
+}
+
+# Test with 32-bit and 64-bit DWARF.
+foreach_with_prefix is_64 {false true} {
+ if { $is_64 } {
+ standard_testfile main.c -dw64.S
+ set testfile ${testfile}-dw64
+ } else {
+ standard_testfile main.c -dw32.S
+ set testfile ${testfile}-dw32
+ }
+
+ set asm_file [standard_output_file $srcfile2]
+ Dwarf::assemble $asm_file {
+ global is_64
+
+ # The CU uses the DW_FORM_rnglistx form to refer to the .debug_rnglists
+ # section.
+ cu {
+ version 5
+ is_64 $is_64
+ } {
+ DW_TAG_compile_unit {
+ {DW_AT_ranges 1 DW_FORM_rnglistx}
+ {DW_AT_rnglists_base cu_table DW_FORM_sec_offset}
+ } {
+ # This tests a DW_AT_ranges attribute of form DW_FORM_rnglistx on a
+ # function, which was buggy at some point.
+ DW_TAG_subprogram {
+ {DW_AT_name "foo"}
+ {DW_AT_ranges 2 DW_FORM_rnglistx}
+ }
+ }
+ }
+
+ rnglists -is-64 $is_64 {
+ # This table is unused, but exists so that the used table is not at
+ # the beginning of the section.
+ table {
+ list_ {
+ start_end 0x1000 0x2000
+ }
+ }
+
+ # The lists in this table are accessed by index (DW_FORM_rnglistx).
+ table -post-header-label cu_table {
+ # This list is unused, but exists to offset the next ones.
+ list_ {
+ start_end 0x2000 0x3000
+ }
+
+ # For the CU.
+ list_ {
+ start_end 0x3000 0x4000
+ }
+
+ # For function foo.
+ list_ {
+ start_end 0x3000 0x3010
+ }
+ }
+ }
+ }
+
+ if { [prepare_for_testing "failed to prepare" ${testfile} \
+ [list $srcfile $asm_file] {nodebug}] } {
+ return -1
+ }
+
+ # Sanity checks to make sure GDB slurped the symbols correctly.
+ gdb_test "p/x &foo" " = 0x3000"
+}
diff --git a/gdb/testsuite/gdb.dwarf2/rnglists-sec-offset.exp b/gdb/testsuite/gdb.dwarf2/rnglists-sec-offset.exp
new file mode 100644
index 000000000000..d898d11c0dc9
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/rnglists-sec-offset.exp
@@ -0,0 +1,80 @@
+# Copyright 2020 Free Software Foundation, Inc.
+
+# 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 <http://www.gnu.org/licenses/>.
+
+# Test DW_AT_ranges attributes referencing the .debug_rnglists section using the
+# DW_FORM_sec_offset form.
+
+load_lib dwarf.exp
+
+if {![dwarf2_support]} {
+ return 0
+}
+
+foreach_with_prefix is_64 {false true} {
+ if { $is_64 } {
+ standard_testfile main.c -dw64.S
+ set testfile ${testfile}-dw64
+ } else {
+ standard_testfile main.c -dw32.S
+ set testfile ${testfile}-dw32
+ }
+
+ set asm_file [standard_output_file $srcfile2]
+ Dwarf::assemble $asm_file {
+ global is_64
+
+ declare_labels cu_range_list foo_range_list
+
+ # This CU uses the DW_FORM_sec_offset form to refer to the .debug_rnglists
+ # section.
+ cu {
+ version 5
+ is_64 $is_64
+ } {
+ DW_TAG_compile_unit {
+ {DW_AT_ranges $cu_range_list DW_FORM_sec_offset}
+ } {
+ DW_TAG_subprogram {
+ {DW_AT_name "foo"}
+ {DW_AT_ranges $foo_range_list DW_FORM_sec_offset}
+ }
+ }
+ }
+
+ rnglists -is-64 $is_64 {
+ # The lists in this table are accessed by direct offset
+ # (DW_FORM_sec_offset).
+ table {
+ # For the CU.
+ cu_range_list: list_ {
+ start_end 0x4000 0x5000
+ }
+
+ # For function foo.
+ foo_range_list: list_ {
+ start_end 0x4000 0x4010
+ }
+ }
+ }
+ }
+
+ if { [prepare_for_testing "failed to prepare" ${testfile} \
+ [list $srcfile $asm_file] {nodebug}] } {
+ return -1
+ }
+
+ # Sanity checks to make sure GDB slurped the symbols correctly.
+ gdb_test "p/x &foo" " = 0x4000"
+}
diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
index 7462890ef9d9..aba4afba2249 100644
--- a/gdb/testsuite/lib/dwarf.exp
+++ b/gdb/testsuite/lib/dwarf.exp
@@ -473,7 +473,8 @@ namespace eval Dwarf {
}
DW_FORM_ref_udata -
- DW_FORM_udata {
+ DW_FORM_udata -
+ DW_FORM_rnglistx {
_op .uleb128 $value
}
@@ -1115,8 +1116,16 @@ namespace eval Dwarf {
}
define_label $start_label
_op .2byte $_cu_version Version
- _op .${_cu_offset_size}byte $my_abbrevs Abbrevs
- _op .byte $_cu_addr_size "Pointer size"
+
+ # The CU header for DWARF 4 and 5 are slightly different.
+ if { $_cu_version == 5 } {
+ _op .byte 0x1 "DW_UT_compile"
+ _op .byte $_cu_addr_size "Pointer size"
+ _op .${_cu_offset_size}byte $my_abbrevs Abbrevs
+ } else {
+ _op .${_cu_offset_size}byte $my_abbrevs Abbrevs
+ _op .byte $_cu_addr_size "Pointer size"
+ }
_defer_output $_abbrev_section {
define_label $my_abbrevs
@@ -1306,6 +1315,178 @@ namespace eval Dwarf {
uplevel $body
}
+ # Emit a DWARF .debug_rnglists section.
+ #
+ # The target address size is based on the current target's address size.
+ #
+ # There is one mandatory positional argument, BODY, which must be Tcl code
+ # that emits the content of the section. It is evaluated in the caller's
+ # context.
+ #
+ # The following option can be used:
+ #
+ # - -is-64 true|false: Whether to use 64-bit DWARF instead of 32-bit DWARF.
+ # The default is 32-bit.
+
+ proc rnglists { args } {
+ variable _debug_rnglists_addr_size
+ variable _debug_rnglists_offset_size
+ variable _debug_rnglists_is_64_dwarf
+
+ parse_args {{"is-64" "false"}}
+
+ if { [llength $args] != 1 } {
+ error "rnglists proc expects one positional argument (body)"
+ }
+
+ lassign $args body
+
+ if [is_64_target] {
+ set _debug_rnglists_addr_size 8
+ } else {
+ set _debug_rnglists_addr_size 4
+ }
+
+ if { ${is-64} } {
+ set _debug_rnglists_offset_size 8
+ set _debug_rnglists_is_64_dwarf true
+ } else {
+ set _debug_rnglists_offset_size 4
+ set _debug_rnglists_is_64_dwarf false
+ }
+
+ _section ".debug_rnglists"
+
+ # Count of tables in the section.
+ variable _debug_rnglists_table_count 0
+
+ # Compute the label name for list at index LIST_IDX, for the current
+ # table.
+
+ proc _compute_list_label { list_idx } {
+ variable _debug_rnglists_table_count
+
+ return ".Lrnglists_table_${_debug_rnglists_table_count}_list_${list_idx}"
+ }
+
+ # Generate one table (header + offset array + range lists).
+ #
+ # Accepts one positional argument, BODY. BODY may call the LIST_
+ # procedure to generate rnglists.
+ #
+ # The -post-header-label option can be used to define a label just after
+ # the header of the table. This is the label that a DW_AT_rnglists_base
+ # attribute will usually refer to.
+
+ proc table { args } {
+ variable _debug_rnglists_table_count
+ variable _debug_rnglists_addr_size
+ variable _debug_rnglists_offset_size
+ variable _debug_rnglists_is_64_dwarf
+
+ parse_args {{post-header-label ""}}
+
+ if { [llength $args] != 1 } {
+ error "table proc expects one positional argument (body)"
+ }
+
+ lassign $args body
+
+ # Generate one range list.
+ #
+ # BODY may call the various procs defined below to generate list entries.
+ # They correspond to the range list entry kinds described in section 2.17.3
+ # of the DWARF 5 spec.
+ #
+ # To define a label pointing to the beginning of the list, use
+ # the conventional way of declaring and defining labels:
+ #
+ # declare_labels the_list
+ #
+ # the_list: list_ {
+ # ...
+ # }
+
+ proc list_ { body } {
+ variable _debug_rnglists_list_count
+
+ # Define a label for this list. It is used to build the offset
+ # array later.
+ set list_label [_compute_list_label $_debug_rnglists_list_count]
+ define_label $list_label
+
+ # Emit a DW_RLE_start_end entry.
+
+ proc start_end { start end } {
+ variable _debug_rnglists_addr_size
+
+ _op .byte 0x06 "DW_RLE_start_end"
+ _op .${_debug_rnglists_addr_size}byte $start "start"
+ _op .${_debug_rnglists_addr_size}byte $end "end"
+ }
+
+ uplevel $body
+
+ # Emit end of list.
+ _op .byte 0x00 "DW_RLE_end_of_list"
+
+ incr _debug_rnglists_list_count
+ }
+
+ # Count of lists in the table.
+ variable _debug_rnglists_list_count 0
+
+ # Generate the lists ops first, because we need to know how many
+ # lists there are to generate the header and offset table.
+ set lists_ops [_defer_to_string {
+ uplevel $body
+ }]
+
+ set post_unit_len_label \
+ [_compute_label "rnglists_table_${_debug_rnglists_table_count}_post_unit_len"]
+ set post_header_label \
+ [_compute_label "rnglists_table_${_debug_rnglists_table_count}_post_header"]
+ set table_end_label \
+ [_compute_label "rnglists_table_${_debug_rnglists_table_count}_end"]
+
+ # Emit the table header.
+ if { $_debug_rnglists_is_64_dwarf } {
+ _op .4byte 0xffffffff "unit length 1/2"
+ _op .8byte "$table_end_label - $post_unit_len_label" "unit length 2/2"
+ } else {
+ _op .4byte "$table_end_label - $post_unit_len_label" "unit length"
+ }
+
+ define_label $post_unit_len_label
+
+ _op .2byte 5 "dwarf version"
+ _op .byte $_debug_rnglists_addr_size "address size"
+ _op .byte 0 "segment selector size"
+ _op .4byte "$_debug_rnglists_list_count" "offset entry count"
+
+ define_label $post_header_label
+
+ # Define the user post-header label, if provided.
+ if { ${post-header-label} != "" } {
+ define_label ${post-header-label}
+ }
+
+ # Emit the offset array.
+ for {set list_idx 0} {$list_idx < $_debug_rnglists_list_count} {incr list_idx} {
+ set list_label [_compute_list_label $list_idx]
+ _op .${_debug_rnglists_offset_size}byte "$list_label - $post_header_label" "offset of list $list_idx"
+ }
+
+ # Emit the actual list data.
+ _emit "$lists_ops"
+
+ define_label $table_end_label
+
+ incr _debug_rnglists_table_count
+ }
+
+ uplevel $body
+ }
# Emit a DWARF .debug_line unit.
# OPTIONS is a list with an even number of elements containing
--
2.30.0
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 08/13] gdb/testsuite: add .debug_rnglists tests
2021-01-20 5:39 ` [PATCH 08/13] gdb/testsuite: add .debug_rnglists tests Simon Marchi
@ 2021-01-28 16:24 ` Zoran Zaric
0 siblings, 0 replies; 34+ messages in thread
From: Zoran Zaric @ 2021-01-28 16:24 UTC (permalink / raw)
To: Simon Marchi, gdb-patches; +Cc: Simon Marchi
> From: Simon Marchi <simon.marchi@efficios.com>
>
> Add tests for the various issues fixed in the previous patches.
>
> Add a new "rnglists" procedure to the DWARF assembler, to allow
> generating .debug_rnglists sections. A trivial change is required to
> support the DWARF 5 CU header layout.
>
I am not a TCL expert and it took me a while to understand the ranglist
support in dwarf.exp and the code makes sense to me.
I also think that the output format should be compatible with the LLVM
assembler.
Thank you for adding this functionality to the DWARF assembler.
Finding ways to force a compiler to generate a specific cases that use
the rangelist constructs with any certainty is almost impossible. Even
the case that I've found could not be reproducible with the next LLVM
release.
Zoran
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 09/13] gdb/testsuite: DWARF assembler: add context parameters to _location
2021-01-20 5:39 [PATCH 00/13] DWARF 5 rnglists & loclists fixes (PR 26813) Simon Marchi
` (7 preceding siblings ...)
2021-01-20 5:39 ` [PATCH 08/13] gdb/testsuite: add .debug_rnglists tests Simon Marchi
@ 2021-01-20 5:39 ` Simon Marchi
2021-01-28 16:30 ` Zoran Zaric
2021-01-20 5:39 ` [PATCH 10/13] gdb/testsuite: add .debug_loclists tests Simon Marchi
` (4 subsequent siblings)
13 siblings, 1 reply; 34+ messages in thread
From: Simon Marchi @ 2021-01-20 5:39 UTC (permalink / raw)
To: gdb-patches; +Cc: Zoran Zaric, Simon Marchi
From: Simon Marchi <simon.marchi@efficios.com>
The _location proc is used to assemble a location description. It needs
to know some contextual information:
- size of an address
- size of an offset (into another DWARF section)
- DWARF version
It currently get all this directly from global variables holding the
compilation unit information. This is fine because as of now, all
location descriptions are generated in the context of creating a
compilation unit. However, a subsequent patch will generate location
descriptions while generating a .debug_loclists section. _location
should therefore no longer rely on the current compilation unit's
properties.
Change it to accept these values as parameters instead of accessing the
values for the CU.
No functional changes intended.
gdb/testsuite/ChangeLog:
* lib/dwarf.exp (_location): Add parameters.
(_handle_DW_FORM): Adjust.
Change-Id: Ib94981979c83ffbebac838081d645ad71c221637
---
gdb/testsuite/lib/dwarf.exp | 40 +++++++++++++++++++++++++------------
1 file changed, 27 insertions(+), 13 deletions(-)
diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
index aba4afba2249..f4f1cab05e68 100644
--- a/gdb/testsuite/lib/dwarf.exp
+++ b/gdb/testsuite/lib/dwarf.exp
@@ -515,11 +515,15 @@ namespace eval Dwarf {
}
SPECIAL_expr {
+ variable _cu_version
+ variable _cu_addr_size
+ variable _cu_offset_size
+
set l1 [new_label "expr_start"]
set l2 [new_label "expr_end"]
_op .uleb128 "$l2 - $l1" "expression"
define_label $l1
- _location $value
+ _location $value $_cu_version $_cu_addr_size $_cu_offset_size
define_label $l2
}
@@ -889,18 +893,28 @@ namespace eval Dwarf {
# This is a miniature assembler for location expressions. It is
# suitable for use in the attributes to a DIE. Its output is
# prefixed with "=" to make it automatically use DW_FORM_block.
+ #
# BODY is split by lines, and each line is taken to be a list.
+ #
+ # DWARF_VERSION is the DWARF version for the section where the location
+ # description is found.
+ #
+ # ADDR_SIZE is the length in bytes (4 or 8) of an address on the target
+ # machine (typically found in the header of the section where the location
+ # description is found).
+ #
+ # OFFSET_SIZE is the length in bytes (4 or 8) of an offset into a DWARF
+ # section. This typically depends on whether 32-bit or 64-bit DWARF is
+ # used, as indicated in the header of the section where the location
+ # description is found.
+ #
# (FIXME should use 'info complete' here.)
# Each list's first element is the opcode, either short or long
# forms are accepted.
# FIXME argument handling
# FIXME move docs
- proc _location {body} {
+ proc _location { body dwarf_version addr_size offset_size } {
variable _constants
- variable _cu_label
- variable _cu_version
- variable _cu_addr_size
- variable _cu_offset_size
foreach line [split $body \n] {
# Ignore blank lines, and allow embedded comments.
@@ -912,7 +926,7 @@ namespace eval Dwarf {
switch -exact -- $opcode {
DW_OP_addr {
- _op .${_cu_addr_size}byte [lindex $line 1]
+ _op .${addr_size}byte [lindex $line 1]
}
DW_OP_regx {
@@ -992,10 +1006,10 @@ namespace eval Dwarf {
# Here label is a section offset.
set label [lindex $line 1]
- if { $_cu_version == 2 } {
- _op .${_cu_addr_size}byte $label
+ if { $dwarf_version == 2 } {
+ _op .${addr_size}byte $label
} else {
- _op .${_cu_offset_size}byte $label
+ _op .${offset_size}byte $label
}
_op .sleb128 [lindex $line 2]
}
@@ -1007,10 +1021,10 @@ namespace eval Dwarf {
# Here label is a section offset.
set label [lindex $line 1]
- if { $_cu_version == 2 } {
- _op .${_cu_addr_size}byte $label
+ if { $dwarf_version == 2 } {
+ _op .${addr_size}byte $label
} else {
- _op .${_cu_offset_size}byte $label
+ _op .${offset_size}byte $label
}
}
--
2.30.0
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 09/13] gdb/testsuite: DWARF assembler: add context parameters to _location
2021-01-20 5:39 ` [PATCH 09/13] gdb/testsuite: DWARF assembler: add context parameters to _location Simon Marchi
@ 2021-01-28 16:30 ` Zoran Zaric
0 siblings, 0 replies; 34+ messages in thread
From: Zoran Zaric @ 2021-01-28 16:30 UTC (permalink / raw)
To: Simon Marchi, gdb-patches; +Cc: Simon Marchi
> From: Simon Marchi <simon.marchi@efficios.com>
>
> The _location proc is used to assemble a location description. It needs
> to know some contextual information:
>
> - size of an address
> - size of an offset (into another DWARF section)
> - DWARF version
>
> It currently get all this directly from global variables holding the
> compilation unit information. This is fine because as of now, all
> location descriptions are generated in the context of creating a
> compilation unit. However, a subsequent patch will generate location
> descriptions while generating a .debug_loclists section. _location
> should therefore no longer rely on the current compilation unit's
> properties.
>
> Change it to accept these values as parameters instead of accessing the
> values for the CU.
>
> No functional changes intended.
>
Agreed.
This seems like a necessary step for the new conditions that the DWARF 5
standard introduces.
Zoran
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 10/13] gdb/testsuite: add .debug_loclists tests
2021-01-20 5:39 [PATCH 00/13] DWARF 5 rnglists & loclists fixes (PR 26813) Simon Marchi
` (8 preceding siblings ...)
2021-01-20 5:39 ` [PATCH 09/13] gdb/testsuite: DWARF assembler: add context parameters to _location Simon Marchi
@ 2021-01-20 5:39 ` Simon Marchi
2021-01-28 16:52 ` Zoran Zaric
2021-01-20 5:39 ` [PATCH 11/13] gdb/dwarf: split dwarf2_cu::ranges_base in two Simon Marchi
` (3 subsequent siblings)
13 siblings, 1 reply; 34+ messages in thread
From: Simon Marchi @ 2021-01-20 5:39 UTC (permalink / raw)
To: gdb-patches; +Cc: Zoran Zaric, Simon Marchi
From: Simon Marchi <simon.marchi@efficios.com>
Add tests for the various issues fixed in the previous patches.
Add a new "loclists" procedure to the DWARF assembler, to allow
generating .debug_loclists sections.
gdb/testsuite/ChangeLog:
* lib/dwarf.exp (_handle_DW_FORM): Handle DW_FORM_loclistx.
(loclists): New proc.
* gdb.dwarf2/loclists-multiple-cus.c: New.
* gdb.dwarf2/loclists-multiple-cus.exp: New.
* gdb.dwarf2/loclists-sec-offset.c: New.
* gdb.dwarf2/loclists-sec-offset.exp: New.
Change-Id: I209bcb2a9482762ae943e518998d1f7761f76928
---
.../gdb.dwarf2/loclists-multiple-cus.c | 37 ++++
.../gdb.dwarf2/loclists-multiple-cus.exp | 146 +++++++++++++
.../gdb.dwarf2/loclists-sec-offset.c | 37 ++++
.../gdb.dwarf2/loclists-sec-offset.exp | 125 +++++++++++
gdb/testsuite/lib/dwarf.exp | 196 ++++++++++++++++++
5 files changed, 541 insertions(+)
create mode 100644 gdb/testsuite/gdb.dwarf2/loclists-multiple-cus.c
create mode 100644 gdb/testsuite/gdb.dwarf2/loclists-multiple-cus.exp
create mode 100644 gdb/testsuite/gdb.dwarf2/loclists-sec-offset.c
create mode 100644 gdb/testsuite/gdb.dwarf2/loclists-sec-offset.exp
diff --git a/gdb/testsuite/gdb.dwarf2/loclists-multiple-cus.c b/gdb/testsuite/gdb.dwarf2/loclists-multiple-cus.c
new file mode 100644
index 000000000000..2bffbf2ac4c0
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/loclists-multiple-cus.c
@@ -0,0 +1,37 @@
+/* Copyright (C) 2021 Free Software Foundation, Inc.
+
+ 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 <http://www.gnu.org/licenses/>. */
+
+static int
+func1 (void)
+{
+ asm ("func1_label: .global func1_label\n");
+ return 1;
+}
+
+static int
+func2 (void)
+{
+ asm ("func2_label: .global func2_label\n");
+ return 2;
+}
+
+int
+main (void)
+{
+ func1 ();
+ func2 ();
+}
diff --git a/gdb/testsuite/gdb.dwarf2/loclists-multiple-cus.exp b/gdb/testsuite/gdb.dwarf2/loclists-multiple-cus.exp
new file mode 100644
index 000000000000..6b4f5c8cbb87
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/loclists-multiple-cus.exp
@@ -0,0 +1,146 @@
+# Copyright 2020 Free Software Foundation, Inc.
+
+# 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 <http://www.gnu.org/licenses/>.
+
+# Test to reproduce the crash described in PR 26813.
+#
+# When reading a list in any table in the .debug_loclists section, GDB would
+# read the header at offset 0 in the section (the header of the first table).
+# When the index of the list we read was greater than the number of lists of
+# the first table, GDB would erroneously report that the index is invalid.
+#
+# So this test creates a .debug_loclists section with two tables. The second
+# table has more lists than the first one and we try to read a high index in
+# the second table.
+
+load_lib dwarf.exp
+
+if {![dwarf2_support]} {
+ return 0
+}
+
+# Test with 32-bit and 64-bit DWARF.
+foreach_with_prefix is_64 {false true} {
+ if { $is_64 } {
+ standard_testfile .c -dw64.S
+ set testfile ${testfile}-dw64
+ } else {
+ standard_testfile .c -dw32.S
+ set testfile ${testfile}-dw32
+ }
+
+ # Get the addresses / lengths of func1 and func2.
+ lassign [function_range func1 $srcdir/$subdir/$srcfile] func1_addr func1_len
+ lassign [function_range func2 $srcdir/$subdir/$srcfile] func2_addr func2_len
+
+ set asm_file [standard_output_file $srcfile2]
+ Dwarf::assemble $asm_file {
+ global func1_addr func1_len
+ global func2_addr func2_len
+ global is_64
+
+ # The CU uses the DW_FORM_loclistx form to refer to the .debug_loclists
+ # section.
+ cu {
+ version 5
+ is_64 $is_64
+ } {
+ declare_labels int_type
+
+ DW_TAG_compile_unit {
+ {DW_AT_loclists_base cu_table DW_FORM_sec_offset}
+ } {
+ int_type: DW_TAG_base_type {
+ {DW_AT_byte_size 4 DW_FORM_data1}
+ {DW_AT_encoding @DW_ATE_signed}
+ {DW_AT_name "int"}
+ }
+
+ DW_TAG_variable {
+ {DW_AT_name "foo"}
+ {DW_AT_location 1 DW_FORM_loclistx}
+ {DW_AT_type :$int_type}
+ }
+
+ DW_TAG_subprogram {
+ {DW_AT_name "func1"}
+ {DW_AT_low_pc $func1_addr}
+ {DW_AT_high_pc $func1_len DW_FORM_udata}
+ }
+
+ DW_TAG_subprogram {
+ {DW_AT_name "func2"}
+ {DW_AT_low_pc $func2_addr}
+ {DW_AT_high_pc $func2_len DW_FORM_udata}
+ }
+ }
+ }
+
+ loclists -is-64 $is_64 {
+ # This table is unused, but exists so that the used table is not at
+ # the beginning of the section.
+ table {
+ list_ {
+ start_length 0x1000 0x1000 { DW_OP_addr 0x100000 }
+ }
+ }
+
+ # The lists in this table are accessed by index (DW_FORM_rnglistx).
+ table -post-header-label cu_table {
+ # This list is unused, but exists to offset the next ones.
+ list_ {
+ start_length 0x1000 0x1000 { DW_OP_addr 0x100000 }
+ }
+
+ # For variable foo.
+ list_ {
+ # When in func1.
+ start_length $func1_addr $func1_len {
+ DW_OP_constu 0x123456
+ DW_OP_stack_value
+ }
+
+ # When in func2.
+ start_length $func2_addr $func2_len {
+ DW_OP_constu 0x234567
+ DW_OP_stack_value
+ }
+ }
+ }
+ }
+ }
+
+ if { [prepare_for_testing "failed to prepare" ${testfile} \
+ [list $srcfile $asm_file] {nodebug}] } {
+ return -1
+ }
+
+ if { ![runto_main] } {
+ fail "can't run to main"
+ return
+ }
+
+ gdb_breakpoint "func1"
+ gdb_breakpoint "func2"
+
+ gdb_continue_to_breakpoint "func1"
+ with_test_prefix "at func1" {
+ gdb_test "print /x foo" " = 0x123456"
+ }
+
+ gdb_continue_to_breakpoint "func2"
+ with_test_prefix "at func2" {
+ gdb_test "print /x foo" " = 0x234567"
+ }
+}
diff --git a/gdb/testsuite/gdb.dwarf2/loclists-sec-offset.c b/gdb/testsuite/gdb.dwarf2/loclists-sec-offset.c
new file mode 100644
index 000000000000..2bffbf2ac4c0
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/loclists-sec-offset.c
@@ -0,0 +1,37 @@
+/* Copyright (C) 2021 Free Software Foundation, Inc.
+
+ 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 <http://www.gnu.org/licenses/>. */
+
+static int
+func1 (void)
+{
+ asm ("func1_label: .global func1_label\n");
+ return 1;
+}
+
+static int
+func2 (void)
+{
+ asm ("func2_label: .global func2_label\n");
+ return 2;
+}
+
+int
+main (void)
+{
+ func1 ();
+ func2 ();
+}
diff --git a/gdb/testsuite/gdb.dwarf2/loclists-sec-offset.exp b/gdb/testsuite/gdb.dwarf2/loclists-sec-offset.exp
new file mode 100644
index 000000000000..9a9188b10744
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/loclists-sec-offset.exp
@@ -0,0 +1,125 @@
+# Copyright 2020 Free Software Foundation, Inc.
+
+# 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 <http://www.gnu.org/licenses/>.
+
+# Test DW_AT_location attributes referencing the .debug_loclists section using
+# the DW_FORM_sec_offset form.
+
+load_lib dwarf.exp
+
+if {![dwarf2_support]} {
+ return 0
+}
+
+# Test with 32-bit and 64-bit DWARF.
+foreach_with_prefix is_64 {false true} {
+ if { $is_64 } {
+ standard_testfile .c -dw64.S
+ set testfile ${testfile}-dw64
+ } else {
+ standard_testfile .c -dw32.S
+ set testfile ${testfile}-dw32
+ }
+
+ # Get the addresses / lengths of func1 and func2.
+ lassign [function_range func1 $srcdir/$subdir/$srcfile] func1_addr func1_len
+ lassign [function_range func2 $srcdir/$subdir/$srcfile] func2_addr func2_len
+
+ set asm_file [standard_output_file $srcfile2]
+ Dwarf::assemble $asm_file {
+ global func1_addr func1_len
+ global func2_addr func2_len
+ global is_64
+
+ declare_labels cu_range_list foo_range_list
+
+ # This CU uses the DW_FORM_sec_offset form to refer to the .debug_rnglists
+ # section.
+ cu {
+ version 5
+ is_64 $is_64
+ } {
+ declare_labels int_type
+ declare_labels foo_location_list
+
+ DW_TAG_compile_unit {
+ } {
+ int_type: DW_TAG_base_type {
+ {DW_AT_byte_size 4 DW_FORM_data1}
+ {DW_AT_encoding @DW_ATE_signed}
+ {DW_AT_name "int"}
+ }
+
+ DW_TAG_variable {
+ {DW_AT_name "foo"}
+ {DW_AT_location $foo_location_list DW_FORM_sec_offset}
+ {DW_AT_type :$int_type}
+ }
+
+ DW_TAG_subprogram {
+ {DW_AT_name "func1"}
+ {DW_AT_low_pc $func1_addr}
+ {DW_AT_high_pc $func1_len DW_FORM_udata}
+ }
+
+ DW_TAG_subprogram {
+ {DW_AT_name "func2"}
+ {DW_AT_low_pc $func2_addr}
+ {DW_AT_high_pc $func2_len DW_FORM_udata}
+ }
+ }
+ }
+
+ loclists -is-64 $is_64 {
+ # The lists in this table are accessed by direct offset
+ # (DW_FORM_sec_offset).
+ table {
+ foo_location_list: list_ {
+ start_length $func1_addr $func1_len {
+ DW_OP_constu 0x123456
+ DW_OP_stack_value
+ }
+
+ start_length $func2_addr $func2_len {
+ DW_OP_constu 0x234567
+ DW_OP_stack_value
+ }
+ }
+ }
+ }
+ }
+
+ if { [prepare_for_testing "failed to prepare" ${testfile} \
+ [list $srcfile $asm_file] {nodebug}] } {
+ return -1
+ }
+
+ if { ![runto_main] } {
+ fail "can't run to main"
+ return
+ }
+
+ gdb_breakpoint "func1"
+ gdb_breakpoint "func2"
+
+ gdb_continue_to_breakpoint "func1"
+ with_test_prefix "at func1" {
+ gdb_test "print /x foo" " = 0x123456"
+ }
+
+ gdb_continue_to_breakpoint "func2"
+ with_test_prefix "at func2" {
+ gdb_test "print /x foo" " = 0x234567"
+ }
+}
diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
index f4f1cab05e68..b444ef36778a 100644
--- a/gdb/testsuite/lib/dwarf.exp
+++ b/gdb/testsuite/lib/dwarf.exp
@@ -474,6 +474,7 @@ namespace eval Dwarf {
DW_FORM_ref_udata -
DW_FORM_udata -
+ DW_FORM_loclistx -
DW_FORM_rnglistx {
_op .uleb128 $value
}
@@ -1502,6 +1503,201 @@ namespace eval Dwarf {
uplevel $body
}
+ # Emit a DWARF .debug_loclists section.
+ #
+ # The target address size is based on the current target's address size.
+ #
+ # There is one mandatory positional argument, BODY, which must be Tcl code
+ # that emits the content of the section. It is evaluated in the caller's
+ # context.
+ #
+ # The following option can be used:
+ #
+ # - -is-64 true|false: Whether to use 64-bit DWARF instead of 32-bit DWARF.
+ # The default is 32-bit.
+
+ proc loclists { args } {
+ variable _debug_loclists_addr_size
+ variable _debug_loclists_offset_size
+ variable _debug_loclists_is_64_dwarf
+
+ parse_args {{"is-64" "false"}}
+
+ if { [llength $args] != 1 } {
+ error "loclists proc expects one positional argument (body)"
+ }
+
+ lassign $args body
+
+ if [is_64_target] {
+ set _debug_loclists_addr_size 8
+ } else {
+ set _debug_loclists_addr_size 4
+ }
+
+ if { ${is-64} } {
+ set _debug_loclists_offset_size 8
+ set _debug_loclists_is_64_dwarf true
+ } else {
+ set _debug_loclists_offset_size 4
+ set _debug_loclists_is_64_dwarf false
+ }
+
+ _section ".debug_loclists"
+
+ # Count of tables in the section.
+ variable _debug_loclists_table_count 0
+
+ # Compute the label name for list at index LIST_IDX, for the current
+ # table.
+
+ proc _compute_list_label { list_idx } {
+ variable _debug_loclists_table_count
+
+ return ".Lloclists_table_${_debug_loclists_table_count}_list_${list_idx}"
+ }
+
+ # Generate one table (header + offset array + location lists).
+ #
+ # Accepts one position argument, BODY. BODY may call the LIST_
+ # procedure to generate loclists.
+ #
+ # The -post-header-label option can be used to define a label just after the
+ # header of the table. This is the label that a DW_AT_loclists_base
+ # attribute will usually refer to.
+
+ proc table { args } {
+ variable _debug_loclists_table_count
+ variable _debug_loclists_addr_size
+ variable _debug_loclists_offset_size
+ variable _debug_loclists_is_64_dwarf
+
+ parse_args {{post-header-label ""}}
+
+ if { [llength $args] != 1 } {
+ error "table proc expects one positional argument (body)"
+ }
+
+ lassign $args body
+
+ # Generate one location list.
+ #
+ # BODY may call the various procs defined below to generate list
+ # entries. They correspond to the location list entry kinds
+ # described in section 2.6.2 of the DWARF 5 spec.
+ #
+ # To define a label pointing to the beginning of the list, use
+ # the conventional way of declaring and defining labels:
+ #
+ # declare_labels the_list
+ #
+ # the_list: list_ {
+ # ...
+ # }
+
+ proc list_ { body } {
+ variable _debug_loclists_list_count
+
+ # Count the location descriptions in this list.
+ variable _debug_loclists_locdesc_count 0
+
+ # Define a label for this list. It is used to build the offset
+ # array later.
+ set list_label [_compute_list_label $_debug_loclists_list_count]
+ define_label $list_label
+
+ # Emit a DW_LLE_start_length entry.
+
+ proc start_length { start length locdesc } {
+ variable _debug_loclists_is_64_dwarf
+ variable _debug_loclists_addr_size
+ variable _debug_loclists_offset_size
+ variable _debug_loclists_table_count
+ variable _debug_loclists_list_count
+ variable _debug_loclists_locdesc_count
+
+ _op .byte 0x08 "DW_LLE_start_length"
+
+ # Start and end of the address range.
+ _op .${_debug_loclists_addr_size}byte $start "start"
+ _op .uleb128 $length "length"
+
+ # Length of location description.
+ set locdesc_start_label ".Lloclists_table_${_debug_loclists_table_count}_list_${_debug_loclists_list_count}_locdesc_${_debug_loclists_locdesc_count}_start"
+ set locdesc_end_label ".Lloclists_table_${_debug_loclists_table_count}_list_${_debug_loclists_list_count}_locdesc_${_debug_loclists_locdesc_count}_end"
+ _op .uleb128 "$locdesc_end_label - $locdesc_start_label" "locdesc length"
+
+ define_label $locdesc_start_label
+ set dwarf_version 5
+ _location $locdesc $dwarf_version $_debug_loclists_addr_size $_debug_loclists_offset_size
+ define_label $locdesc_end_label
+
+ incr _debug_loclists_locdesc_count
+ }
+
+ uplevel $body
+
+ # Emit end of list.
+ _op .byte 0x00 "DW_LLE_end_of_list"
+
+ incr _debug_loclists_list_count
+ }
+
+ # Count of lists in the table.
+ variable _debug_loclists_list_count 0
+
+ # Generate the lists ops first, because we need to know how many
+ # lists there are to generate the header and offset table.
+ set lists_ops [_defer_to_string {
+ uplevel $body
+ }]
+
+ set post_unit_len_label \
+ [_compute_label "loclists_table_${_debug_loclists_table_count}_post_unit_len"]
+ set post_header_label \
+ [_compute_label "loclists_table_${_debug_loclists_table_count}_post_header"]
+ set table_end_label \
+ [_compute_label "loclists_table_${_debug_loclists_table_count}_end"]
+
+ # Emit the table header.
+ if { $_debug_loclists_is_64_dwarf } {
+ _op .4byte 0xffffffff "unit length 1/2"
+ _op .8byte "$table_end_label - $post_unit_len_label" "unit length 2/2"
+ } else {
+ _op .4byte "$table_end_label - $post_unit_len_label" "unit length"
+ }
+
+ define_label $post_unit_len_label
+
+ _op .2byte 5 "DWARF version"
+ _op .byte $_debug_loclists_addr_size "address size"
+ _op .byte 0 "segment selector size"
+ _op .4byte $_debug_loclists_list_count "offset entry count"
+
+ define_label $post_header_label
+
+ # Define the user post-header label, if provided.
+ if { ${post-header-label} != "" } {
+ define_label ${post-header-label}
+ }
+
+ # Emit the offset array.
+ for {set list_idx 0} {$list_idx < $_debug_loclists_list_count} {incr list_idx} {
+ set list_label [_compute_list_label $list_idx]
+ _op .${_debug_loclists_offset_size}byte "$list_label - $post_header_label" "offset of list $list_idx"
+ }
+
+ # Emit the actual list data.
+ _emit "$lists_ops"
+
+ define_label $table_end_label
+
+ incr _debug_loclists_table_count
+ }
+
+ uplevel $body
+ }
+
# Emit a DWARF .debug_line unit.
# OPTIONS is a list with an even number of elements containing
# option-name and option-value pairs.
--
2.30.0
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 10/13] gdb/testsuite: add .debug_loclists tests
2021-01-20 5:39 ` [PATCH 10/13] gdb/testsuite: add .debug_loclists tests Simon Marchi
@ 2021-01-28 16:52 ` Zoran Zaric
2021-01-28 17:47 ` Simon Marchi
0 siblings, 1 reply; 34+ messages in thread
From: Zoran Zaric @ 2021-01-28 16:52 UTC (permalink / raw)
To: Simon Marchi, gdb-patches; +Cc: Simon Marchi
> From: Simon Marchi <simon.marchi@efficios.com>
>
> Add tests for the various issues fixed in the previous patches.
>
> Add a new "loclists" procedure to the DWARF assembler, to allow
> generating .debug_loclists sections.
>
Thank you for this contribution.
Having a loclists support in DWARF assembler gives us so considerable
testing flexibility and decouples the gdb testing even more from what
compiler is expected to generate.
The only thing missing now (at least in my mind) is the CFI support but
that is a big project for the future.
> +
> + proc loclists { args } {
> + variable _debug_loclists_addr_size
> + variable _debug_loclists_offset_size
> + variable _debug_loclists_is_64_dwarf
> +
> + parse_args {{"is-64" "false"}}
> +
> + if { [llength $args] != 1 } {
> + error "loclists proc expects one positional argument (body)"
> + }
> +
> + lassign $args body
> +
> + if [is_64_target] {
> + set _debug_loclists_addr_size 8
> + } else {
> + set _debug_loclists_addr_size 4
> + }
> +
> + if { ${is-64} } {
> + set _debug_loclists_offset_size 8
> + set _debug_loclists_is_64_dwarf true
> + } else {
> + set _debug_loclists_offset_size 4
> + set _debug_loclists_is_64_dwarf false
> + }
> +
> + _section ".debug_loclists"
> +
> + # Count of tables in the section.
> + variable _debug_loclists_table_count 0
> +
> + # Compute the label name for list at index LIST_IDX, for the current
> + # table.
> +
> + proc _compute_list_label { list_idx } {
> + variable _debug_loclists_table_count
> +
> + return ".Lloclists_table_${_debug_loclists_table_count}_list_${list_idx}"
> + }
> +
> + # Generate one table (header + offset array + location lists).
> + #
> + # Accepts one position argument, BODY. BODY may call the LIST_
> + # procedure to generate loclists.
> + #
> + # The -post-header-label option can be used to define a label just after the
> + # header of the table. This is the label that a DW_AT_loclists_base
> + # attribute will usually refer to.
> +
> + proc table { args } {
> + variable _debug_loclists_table_count
> + variable _debug_loclists_addr_size
> + variable _debug_loclists_offset_size
> + variable _debug_loclists_is_64_dwarf
> +
> + parse_args {{post-header-label ""}}
> +
> + if { [llength $args] != 1 } {
> + error "table proc expects one positional argument (body)"
> + }
> +
> + lassign $args body
> +
> + # Generate one location list.
> + #
> + # BODY may call the various procs defined below to generate list
> + # entries. They correspond to the location list entry kinds
> + # described in section 2.6.2 of the DWARF 5 spec.
> + #
> + # To define a label pointing to the beginning of the list, use
> + # the conventional way of declaring and defining labels:
> + #
> + # declare_labels the_list
> + #
> + # the_list: list_ {
> + # ...
> + # }
> +
> + proc list_ { body } {
> + variable _debug_loclists_list_count
> +
I guess that the table and list procedures from loclist and ranglist
support couldn't be commonized in some way?
They do seem considerably different.
Zoran
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 10/13] gdb/testsuite: add .debug_loclists tests
2021-01-28 16:52 ` Zoran Zaric
@ 2021-01-28 17:47 ` Simon Marchi
2021-01-29 10:13 ` Zoran Zaric
0 siblings, 1 reply; 34+ messages in thread
From: Simon Marchi @ 2021-01-28 17:47 UTC (permalink / raw)
To: Zoran Zaric, gdb-patches; +Cc: Simon Marchi
On 2021-01-28 11:52 a.m., Zoran Zaric wrote:>> From: Simon Marchi <simon.marchi@efficios.com>
>>
>> Add tests for the various issues fixed in the previous patches.
>>
>> Add a new "loclists" procedure to the DWARF assembler, to allow
>> generating .debug_loclists sections.
>>
>
> Thank you for this contribution.
>
> Having a loclists support in DWARF assembler gives us so considerable testing flexibility and decouples the gdb testing even more from what compiler is expected to generate.
>
> The only thing missing now (at least in my mind) is the CFI support but that is a big project for the future.
Given that:
- the actual assembler (GNU as or other) already has support for
specifying and generating CFI, and
- a test case that wants to use specific CFI would contain some
assembly code already, to control exactly which instructions are
generated
I don't think our assembler needs to know how to generate CFI, you'd
just write it in platform-specific assembly.
>> +
>> + proc loclists { args } {
>> + variable _debug_loclists_addr_size
>> + variable _debug_loclists_offset_size
>> + variable _debug_loclists_is_64_dwarf
>> +
>> + parse_args {{"is-64" "false"}}
>> +
>> + if { [llength $args] != 1 } {
>> + error "loclists proc expects one positional argument (body)"
>> + }
>> +
>> + lassign $args body
>> +
>> + if [is_64_target] {
>> + set _debug_loclists_addr_size 8
>> + } else {
>> + set _debug_loclists_addr_size 4
>> + }
>> +
>> + if { ${is-64} } {
>> + set _debug_loclists_offset_size 8
>> + set _debug_loclists_is_64_dwarf true
>> + } else {
>> + set _debug_loclists_offset_size 4
>> + set _debug_loclists_is_64_dwarf false
>> + }
>> +
>> + _section ".debug_loclists"
>> +
>> + # Count of tables in the section.
>> + variable _debug_loclists_table_count 0
>> +
>> + # Compute the label name for list at index LIST_IDX, for the current
>> + # table.
>> +
>> + proc _compute_list_label { list_idx } {
>> + variable _debug_loclists_table_count
>> +
>> + return ".Lloclists_table_${_debug_loclists_table_count}_list_${list_idx}"
>> + }
>> +
>> + # Generate one table (header + offset array + location lists).
>> + #
>> + # Accepts one position argument, BODY. BODY may call the LIST_
>> + # procedure to generate loclists.
>> + #
>> + # The -post-header-label option can be used to define a label just after the
>> + # header of the table. This is the label that a DW_AT_loclists_base
>> + # attribute will usually refer to.
>> +
>> + proc table { args } {
>> + variable _debug_loclists_table_count
>> + variable _debug_loclists_addr_size
>> + variable _debug_loclists_offset_size
>> + variable _debug_loclists_is_64_dwarf
>> +
>> + parse_args {{post-header-label ""}}
>> +
>> + if { [llength $args] != 1 } {
>> + error "table proc expects one positional argument (body)"
>> + }
>> +
>> + lassign $args body
>> +
>> + # Generate one location list.
>> + #
>> + # BODY may call the various procs defined below to generate list
>> + # entries. They correspond to the location list entry kinds
>> + # described in section 2.6.2 of the DWARF 5 spec.
>> + #
>> + # To define a label pointing to the beginning of the list, use
>> + # the conventional way of declaring and defining labels:
>> + #
>> + # declare_labels the_list
>> + #
>> + # the_list: list_ {
>> + # ...
>> + # }
>> +
>> + proc list_ { body } {
>> + variable _debug_loclists_list_count
>> +
>
> I guess that the table and list procedures from loclist and ranglist support couldn't be commonized in some way?
>
> They do seem considerably different.
Yeah, they are similar (once you understand one, it's very easy to
understand the other). But they are not identical. So given that we
have only two kinds, I think it would just make it more complex for
nothing to try to factor out the common parts. If we had 20 kinds, then
maybe it would be a different story :).
Simon
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 10/13] gdb/testsuite: add .debug_loclists tests
2021-01-28 17:47 ` Simon Marchi
@ 2021-01-29 10:13 ` Zoran Zaric
2021-01-29 15:57 ` Simon Marchi
0 siblings, 1 reply; 34+ messages in thread
From: Zoran Zaric @ 2021-01-29 10:13 UTC (permalink / raw)
To: Simon Marchi, gdb-patches; +Cc: Simon Marchi
>>>
>>> Add tests for the various issues fixed in the previous patches.
>>>
>>> Add a new "loclists" procedure to the DWARF assembler, to allow
>>> generating .debug_loclists sections.
>>>
>>
>> Thank you for this contribution.
>>
>> Having a loclists support in DWARF assembler gives us so considerable testing flexibility and decouples the gdb testing even more from what compiler is expected to generate.
>>
>> The only thing missing now (at least in my mind) is the CFI support but that is a big project for the future.
>
> Given that:
>
> - the actual assembler (GNU as or other) already has support for
> specifying and generating CFI, and
> - a test case that wants to use specific CFI would contain some
> assembly code already, to control exactly which instructions are
> generated
This is not exactly true, you can always define a CFI that doesn't need
any assembly code considering that register values can be set from the
the test itself or if only CFI register rules that target a memory
locations are used.
With the new extensions that I've contributed (and are currently under
the review) the register rules mechanism now supports any location
description to be part of the DWARF expression. With this extension, you
can imagine that a very complex DWARF expression that doesn't use any
potentially ABI sensitive resource, can still be written and tested in
the same way as any variable location.
Another option is to only hand write a CFI table for the top level
function (frame 0) and design a way to merge the original CFI generated
by the compiler for other functions with the hand written one.
Also, with a potential new operation DW_OP_LLVM_call_frame_entry_reg
described at length here:
https://llvm.org/docs/AMDGPUDwarfExtensionsForHeterogeneousDebugging.html
This would allow us to test any CFI expression of the frame we are
currently stopped in.
>
> I don't think our assembler needs to know how to generate CFI, you'd
> just write it in platform-specific assembly.
>
But, isn't this the case for any part of the DWARF assembler functionality?
Writing any complex DWARF expression fast in any assembly is hard and
time consuming.
I agree that adding a CFI support requires more thought and effort and
better understanding about how the CFI works and what is safe to use
from a test writer perspective, but I can definitely see the benefit of
adding that support in the future.
Zoran
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 10/13] gdb/testsuite: add .debug_loclists tests
2021-01-29 10:13 ` Zoran Zaric
@ 2021-01-29 15:57 ` Simon Marchi
2021-01-29 16:58 ` Zoran Zaric
0 siblings, 1 reply; 34+ messages in thread
From: Simon Marchi @ 2021-01-29 15:57 UTC (permalink / raw)
To: Zoran Zaric, gdb-patches; +Cc: Simon Marchi
On 2021-01-29 5:13 a.m., Zoran Zaric wrote:
>>>>
>>>> Add tests for the various issues fixed in the previous patches.
>>>>
>>>> Add a new "loclists" procedure to the DWARF assembler, to allow
>>>> generating .debug_loclists sections.
>>>>
>>>
>>> Thank you for this contribution.
>>>
>>> Having a loclists support in DWARF assembler gives us so considerable testing flexibility and decouples the gdb testing even more from what compiler is expected to generate.
>>>
>>> The only thing missing now (at least in my mind) is the CFI support but that is a big project for the future.
>>
>> Given that:
>>
>> - the actual assembler (GNU as or other) already has support for
>> specifying and generating CFI, and
>> - a test case that wants to use specific CFI would contain some
>> assembly code already, to control exactly which instructions are
>> generated
>
> This is not exactly true, you can always define a CFI that doesn't need any assembly code considering that register values can be set from the the test itself or if only CFI register rules that target a memory locations are used.
Isn't a CFI table essentially a big mapping that says:
- At address X + 0, here's how you'll find the saved registers
- At address X + 2, here's how you'll find the saved registers
- ... and so on
So ultimately you want in your test case to know exactly which
instructions are generated, and their addresses, to generate the CFI
table, don't you? And for that, won't you write assembly?
I'm not sure I know what "CFI register rules that target a memory
location" means. A register from a previous frame that is currently
saved in memory? I don't really understand what that changes.
> With the new extensions that I've contributed (and are currently under the review) the register rules mechanism now supports any location description to be part of the DWARF expression. With this extension, you can imagine that a very complex DWARF expression that doesn't use any potentially ABI sensitive resource, can still be written and tested in the same way as any variable location.
Hmm but in the end it's still just a sequence of opcodes, isn't it?
If a compiler is to ever emit such an expression, it would have to emit
it using CFI directives in the assembly code, so we could always write
the same expression by hand directly in assembly, couldn't we?
> Another option is to only hand write a CFI table for the top level function (frame 0) and design a way to merge the original CFI generated by the compiler for other functions with the hand written one.
I don't remember how exactly things are merged by the linker, but I
suppose that would work. But again, you'd probably want to write that
top-level frame 0 function in assembly - I think. Or if don't need to
write different rules for different instructions in the function, I
guess that function could be written in C, and you make CFI rules for
the whole function's range. But you'd need a way to tell the compiler
to not generate CFI for that particular function.
>
> Also, with a potential new operation DW_OP_LLVM_call_frame_entry_reg described at length here:
>
> https://llvm.org/docs/AMDGPUDwarfExtensionsForHeterogeneousDebugging.html
>
> This would allow us to test any CFI expression of the frame we are currently stopped in.
>
>>
>> I don't think our assembler needs to know how to generate CFI, you'd
>> just write it in platform-specific assembly.
>>
>
> But, isn't this the case for any part of the DWARF assembler functionality?
>
> Writing any complex DWARF expression fast in any assembly is hard and time consuming.
No, you can't express symbolically any DWARF expression at the assembly
level. For example, you can't describe the tree of DWARF DIEs using
directives in assembly. All you can do is output the raw bytes using
the .byte and friends directives, but that wouldn't be humanly feasible.
So instead we have this higher level language (our DWARF assembler)
where we describe the tree of DIEs and have it output the .byte
directives for us.
But in the case of CFI, the assembler has directives to express
symbolically everything we need to express today: the directives
".cfi_def_cfa_offset" and friends. You write them interleaved with the
assembly code, and the assembler generates the corresponding CFI tables.
And this is also how the compiler works, it emits CFI directives
interleaved with the assembly code. So if we want to test a particular
DWARF CFI construct, it's probably because the compiler generates it,
which means the assembler supports it, which means we can also write a
test case for it by hand (in assembly). Or maybe I'm mistaken and some
compilers generate CFI tables as bytes directly?
>
> I agree that adding a CFI support requires more thought and effort and better understanding about how the CFI works and what is safe to use from a test writer perspective, but I can definitely see the benefit of adding that support in the future.
In the event we want to test a CFI construct that isn't yet supported by
the assembler, then it could make sense to write own assembler that
emits the right bytes directly.
Simon
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 10/13] gdb/testsuite: add .debug_loclists tests
2021-01-29 15:57 ` Simon Marchi
@ 2021-01-29 16:58 ` Zoran Zaric
2021-01-29 17:37 ` Simon Marchi
0 siblings, 1 reply; 34+ messages in thread
From: Zoran Zaric @ 2021-01-29 16:58 UTC (permalink / raw)
To: Simon Marchi, gdb-patches; +Cc: Simon Marchi
>>>>>
>>>>> Add tests for the various issues fixed in the previous patches.
>>>>>
>>>>> Add a new "loclists" procedure to the DWARF assembler, to allow
>>>>> generating .debug_loclists sections.
>>>>>
>>>>
>>>> Thank you for this contribution.
>>>>
>>>> Having a loclists support in DWARF assembler gives us so considerable testing flexibility and decouples the gdb testing even more from what compiler is expected to generate.
>>>>
>>>> The only thing missing now (at least in my mind) is the CFI support but that is a big project for the future.
>>>
>>> Given that:
>>>
>>> - the actual assembler (GNU as or other) already has support for
>>> specifying and generating CFI, and
>>> - a test case that wants to use specific CFI would contain some
>>> assembly code already, to control exactly which instructions are
>>> generated
>>
>> This is not exactly true, you can always define a CFI that doesn't need any assembly code considering that register values can be set from the the test itself or if only CFI register rules that target a memory locations are used.
>
> Isn't a CFI table essentially a big mapping that says:
>
> - At address X + 0, here's how you'll find the saved registers
> - At address X + 2, here's how you'll find the saved registers
> - ... and so on
>
> So ultimately you want in your test case to know exactly which
> instructions are generated, and their addresses, to generate the CFI
> table, don't you? And for that, won't you write assembly?
You can still write a CFI information that covers the whole function
without going into details on a per instruction basis. In the same way
how you can write a single location description for a variable right now
without thinking if that variable exist in the actual code or is it
mapped to only one location.
If someone wants to do it per instruction basis later, more power to them.
It is true that one really needs to know what they are doing but that is
no different to the current symbolic information support.
>
> I'm not sure I know what "CFI register rules that target a memory
> location" means. A register from a previous frame that is currently
> saved in memory? I don't really understand what that changes.
My point was that in that case you can still write a fairly complex CFI
information that never touches ABI sensitive resources like a specific
register.
This will get even more useful with future addition of address spaces,
where pieces of a register might be spilled to different kinds of memory
like AMD has in the case of their large vector registers.
>
>> With the new extensions that I've contributed (and are currently under the review) the register rules mechanism now supports any location description to be part of the DWARF expression. With this extension, you can imagine that a very complex DWARF expression that doesn't use any potentially ABI sensitive resource, can still be written and tested in the same way as any variable location.
>
> Hmm but in the end it's still just a sequence of opcodes, isn't it?
Same goes for variable location descriptions right? And we still support
those in the DWARF assembler.
I have written full debug information in assembly in the past, and apart
from encoding the DIE relationship by hand, I didn't really see that
much difference between encoding variable location to a CFI entries.
Both equally unpleasant.
>> Another option is to only hand write a CFI table for the top level function (frame 0) and design a way to merge the original CFI generated by the compiler for other functions with the hand written one.
>
> I don't remember how exactly things are merged by the linker, but I
> suppose that would work. But again, you'd probably want to write that
> top-level frame 0 function in assembly - I think. Or if don't need to
> write different rules for different instructions in the function, I
> guess that function could be written in C, and you make CFI rules for
> the whole function's range. But you'd need a way to tell the compiler
> to not generate CFI for that particular function.
This function could be an empty function with a single asm statement
that could be compiled separately (with the instructions to the compiler
to not generate debug info for it).
We don't do that right now but it doesn't sound like a deal breaker.
>
>>
>> Also, with a potential new operation DW_OP_LLVM_call_frame_entry_reg described at length here:
>>
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fllvm.org%2Fdocs%2FAMDGPUDwarfExtensionsForHeterogeneousDebugging.html&data=04%7C01%7CZoran.Zaric%40amd.com%7C20c7017e3b7f47e5f39808d8c46e9356%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637475326499683800%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=Ww3TjCpYOjj7FtUuTSr93VqHEErO1LoLJFRJGK3hhFg%3D&reserved=0
>>
>> This would allow us to test any CFI expression of the frame we are currently stopped in.
>>
>>>
>>> I don't think our assembler needs to know how to generate CFI, you'd
>>> just write it in platform-specific assembly.
>>>
>>
>> But, isn't this the case for any part of the DWARF assembler functionality?
>>
>> Writing any complex DWARF expression fast in any assembly is hard and time consuming.
>
> No, you can't express symbolically any DWARF expression at the assembly
> level. For example, you can't describe the tree of DWARF DIEs using
> directives in assembly. All you can do is output the raw bytes using
> the .byte and friends directives, but that wouldn't be humanly feasible.
> So instead we have this higher level language (our DWARF assembler)
> where we describe the tree of DIEs and have it output the .byte
> directives for us.
This all depends of ones view of what is convenient and what is not.
From this discussion it sounds like the DIE relationship was the main
motivation for developing the DWARF assembler. I always though that it
was for general convenience of writing debug information in one test.
While I agree that writing DIE relationships in a raw byte format is
daunting, so is writing a really complex DWARF expression. This will get
even more daunting when lane masking, address spaces and large vector
registers come into play.
I have seen DWARF expression with over 15 operations and those are not
fun to write in a byte format.
And why not give the ability to mix and match symbolic variable
information over a frame change with hand written CFI rules in one test?
>
>>
>> I agree that adding a CFI support requires more thought and effort and better understanding about how the CFI works and what is safe to use from a test writer perspective, but I can definitely see the benefit of adding that support in the future.
>
> In the event we want to test a CFI construct that isn't yet supported by
> the assembler, then it could make sense to write own assembler that
> emits the right bytes directly.
I agree that what we have is good enough right now, but I wouldn't want
us to dismiss the idea completely.
I might be tempted to implement it in the future :)
Zoran
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 10/13] gdb/testsuite: add .debug_loclists tests
2021-01-29 16:58 ` Zoran Zaric
@ 2021-01-29 17:37 ` Simon Marchi
0 siblings, 0 replies; 34+ messages in thread
From: Simon Marchi @ 2021-01-29 17:37 UTC (permalink / raw)
To: Zoran Zaric, Simon Marchi, gdb-patches
On 2021-01-29 11:58 a.m., Zoran Zaric wrote:
>
> This all depends of ones view of what is convenient and what is not.
>
> From this discussion it sounds like the DIE relationship was the main
> motivation for developing the DWARF assembler. I always though that it
> was for general convenience of writing debug information in one test.
>
> While I agree that writing DIE relationships in a raw byte format is
> daunting, so is writing a really complex DWARF expression. This will
> get even more daunting when lane masking, address spaces and large
> vector registers come into play.
>
> I have seen DWARF expression with over 15 operations and those are not
> fun to write in a byte format.
So I imagined that if the compiler supports emitting CFI that uses
complex expressions, then the assembler would support a notation for
it. So you wouldn't write bytes, but something like:
.cfi_expr OP_CONST 1, OP_CONST 2, OP_ADD
where ".cfi_expr" is a directive I made up. However, that's perhaps not
true. I see that GAS has this ".cfi_escape" directive where you
directly pass bytes, perhaps the compiler would use that. If so, then
it indeed sounds desirable to be able to write the expressions using
a higher level notation, in TCL.
Simon
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 11/13] gdb/dwarf: split dwarf2_cu::ranges_base in two
2021-01-20 5:39 [PATCH 00/13] DWARF 5 rnglists & loclists fixes (PR 26813) Simon Marchi
` (9 preceding siblings ...)
2021-01-20 5:39 ` [PATCH 10/13] gdb/testsuite: add .debug_loclists tests Simon Marchi
@ 2021-01-20 5:39 ` Simon Marchi
2021-01-20 5:39 ` [PATCH 12/13] gdb/dwarf: make read_{loc, rng}list_index return sect_offset Simon Marchi
` (2 subsequent siblings)
13 siblings, 0 replies; 34+ messages in thread
From: Simon Marchi @ 2021-01-20 5:39 UTC (permalink / raw)
To: gdb-patches; +Cc: Zoran Zaric, Simon Marchi
From: Simon Marchi <simon.marchi@efficios.com>
Consider the test case added in this patch. It defines a compilation
unit with a DW_AT_rnglists_base attribute (used for attributes of form
DW_FORM_rnglistx), but also uses DW_AT_ranges of form
DW_FORM_sec_offset:
0x00000027: DW_TAG_compile_unit
DW_AT_ranges [DW_FORM_sec_offset] (0x0000004c
[0x0000000000005000, 0x0000000000006000))
DW_AT_rnglists_base [DW_FORM_sec_offset] (0x00000044)
The DW_AT_rnglists_base does not play a role in reading the DW_AT_ranges of
form DW_FORM_sec_offset, but it should also not do any harm.
This case is currently not handled correctly by GDB. This is not
something that a compiler is likely to emit, but in my opinion there's
no reason why GDB should fail reading it.
The problem is that in partial_die_info::read and a few other places
where the same logic is replicated, the cu->ranges_base value,
containing the DW_AT_rnglists_base value, is wrongfully added to the
DW_AT_ranges value.
It is quite messy how to decide whether cu->ranges_base should be added
to the attribute's value or not. But to summarize, the only time we
want to add it is when the attribute comes from a pre-DWARF 5 split unit
file (a .dwo) [1]. In this case, the DW_AT_ranges attribute from the
split unit file will have form DW_FORM_sec_offset, pointing somewhere in
the linked file's .debug_ranges section. *But* it's not a "true"
DW_FORM_sec_offset, in that it's an offset relative to the beginning of
that CU's contribution in the section, not relative to the beginning of
the section. So in that case, and only that case, do we want to add the
ranges base value, which we found from the DW_AT_GNU_ranges_base
attribute on the skeleton unit.
Almost all instances of the DW_AT_ranges attribute will be found in the
split unit (on DW_TAG_subprogram, for example), and therefore need to
have the ranges base added. However, the DW_TAG_compile_unit DIE in the
skeleton may also have a DW_AT_ranges attribute. For that one, the
ranges base must not be added. Once the DIEs have been loaded in GDB,
however, the distinction between what's coming from the skeleton and
what's coming from the split unit is not clear. It is all merged in one
big happy tree. So how do we know if a given attribute comes from the
split unit or not?
We use the fact that in pre-DWARF 5 split DWARF, DW_AT_ranges is found
on the skeleton's DW_TAG_compile_unit (in the linked file) and never in
the split unit's DW_TAG_compile_unit. This is why you have this in
partial_die_info::read:
int need_ranges_base = (tag != DW_TAG_compile_unit
&& attr.form != DW_FORM_rnglistx);
However, with the corner case described above (where we have a
DW_AT_rnglists_base attribute and a DW_AT_ranges attribute of form
DW_FORM_sec_offset) the condition gets it wrong when it encounters an
attribute like DW_TAG_subprogram with a DW_AT_ranges attribute of
DW_FORM_sec_offset form: it thinks that it is necessary to add the base,
when it reality it is not.
The problem boils down to failing to differentiate these cases:
- a DW_AT_ranges attribute of form DW_FORM_sec_offset in a
pre-DWARF 5 split unit (in which case we need to add the base)
- a DW_AT_ranges attribute of form DW_FORM_sec_offset in a DWARF 5
non-split unit (in which case we must not add the base)
What makes it unnecessarily complex is that the cu->ranges_base field is
overloaded, used to hold the pre-DWARF 5, non-standard
DW_AT_GNU_ranges_base and the DWARF 5 DW_AT_rnglists_base. In reality,
these two are called "bases" but are not the same thing. The result is
that we need twisted conditions to try to determine whether or not we
should add the base to the attribute's value.
To fix it, split the field in two distinct fields. I renamed everything
related to the "old" ranges base to "gnu_ranges_base", to make it clear
that it's about the non-standard, pre-DWARF 5 thing. And everything
related to the DWARF 5 thing gets renamed "rnglists". I think it
becomes much easier to reason this way.
The issue described above gets fixed by the fact that the
DW_AT_rnglists_base value does not end up in cu->gnu_ranges_base, so
cu->gnu_ranges_base stays 0. The condition to determine whether
gnu_ranges_base should be added can therefore be simplified back to:
tag != DW_TAG_compile_unit
... as it was before rnglistx support was added.
Extend the gdb.dwarf2/rnglists-sec-offset.exp to cover this case. I
also extended the test case for loclists similarly, just to see if there
would be some similar problem. There wasn't, but I think it's not a bad
idea to test that case for loclists as well, so I left it in the patch.
[1] https://gcc.gnu.org/wiki/DebugFission
gdb/ChangeLog:
* dwarf2/die.h (struct die_info) <ranges_base>: Split in...
<gnu_ranges_base>: ... this...
<rnglists_base>: ... and this.
* dwarf2/read.c (struct dwarf2_cu) <ranges_base>: Split in...
<gnu_ranges_base>: ... this...
<rnglists_base>: ... and this.
(read_cutu_die_from_dwo): Adjust
(dwarf2_get_pc_bounds): Adjust
(dwarf2_record_block_ranges): Adjust.
(read_full_die_1): Adjust
(partial_die_info::read): Adjust.
(read_rnglist_index): Adjust.
gdb/testsuite/ChangeLog:
* gdb.dwarf2/rnglists-sec-offset.exp: Add test for DW_AT_ranges
of DW_FORM_sec_offset form plus DW_AT_rnglists_base attribute.
* gdb.dwarf2/loclists-sec-offset.exp: Add test for
DW_AT_location of DW_FORM_sec_offset plus DW_AT_loclists_base
attribute
Change-Id: Icd109038634b75d0e6e9d7d1dcb62fb9eb951d83
---
gdb/dwarf2/die.h | 36 +++--
gdb/dwarf2/read.c | 142 ++++++++++--------
.../gdb.dwarf2/loclists-sec-offset.c | 16 ++
.../gdb.dwarf2/loclists-sec-offset.exp | 78 +++++++++-
.../gdb.dwarf2/rnglists-sec-offset.exp | 41 ++++-
5 files changed, 228 insertions(+), 85 deletions(-)
diff --git a/gdb/dwarf2/die.h b/gdb/dwarf2/die.h
index 587b9f66a439..792b7214023a 100644
--- a/gdb/dwarf2/die.h
+++ b/gdb/dwarf2/die.h
@@ -55,26 +55,40 @@ struct die_info
return gdb::optional<ULONGEST> ();
}
- /* Return range lists base of the compile unit, which, if exists, is
- stored either at the attribute DW_AT_rnglists_base or
- DW_AT_GNU_ranges_base. */
- ULONGEST ranges_base ()
+ /* Return the base address of the compile unit into the .debug_ranges section,
+ which, if exists, is stored in the DW_AT_GNU_ranges_base attribute. This
+ value is only relevant in pre-DWARF 5 split-unit scenarios. */
+ ULONGEST gnu_ranges_base ()
{
for (unsigned i = 0; i < num_attrs; ++i)
- if (attrs[i].name == DW_AT_rnglists_base
- || attrs[i].name == DW_AT_GNU_ranges_base)
+ if (attrs[i].name == DW_AT_GNU_ranges_base)
{
if (attrs[i].form_is_unsigned ())
- {
- /* If both exist, just use the first one. */
- return attrs[i].as_unsigned ();
- }
- complaint (_("ranges base attribute (offset %s) as wrong form"),
+ return attrs[i].as_unsigned ();
+
+ complaint (_("ranges base attribute (offset %s) has wrong form"),
sect_offset_str (sect_off));
}
+
return 0;
}
+ /* Return the rnglists base of the compile unit, which, if exists, is stored
+ in the DW_AT_rnglists_base attribute. */
+ ULONGEST rnglists_base ()
+ {
+ for (unsigned i = 0; i < num_attrs; ++i)
+ if (attrs[i].name == DW_AT_rnglists_base)
+ {
+ if (attrs[i].form_is_unsigned ())
+ return attrs[i].as_unsigned ();
+
+ complaint (_("rnglists base attribute (offset %s) has wrong form"),
+ sect_offset_str (sect_off));
+ }
+
+ return 0;
+ }
/* DWARF-2 tag for this DIE. */
ENUM_BITFIELD(dwarf_tag) tag : 16;
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 5277e2a09e3f..86a69efe8ecc 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -551,16 +551,41 @@ struct dwarf2_cu
Note this value comes from the Fission stub CU/TU's DIE. */
gdb::optional<ULONGEST> addr_base;
- /* The DW_AT_rnglists_base attribute if present.
- Note this value comes from the Fission stub CU/TU's DIE.
- Also note that the value is zero in the non-DWO case so this value can
- be used without needing to know whether DWO files are in use or not.
- N.B. This does not apply to DW_AT_ranges appearing in
- DW_TAG_compile_unit dies. This is a bit of a wart, consider if ever
- DW_AT_ranges appeared in the DW_TAG_compile_unit of DWO DIEs: then
- DW_AT_rnglists_base *would* have to be applied, and we'd have to care
- whether the DW_AT_ranges attribute came from the skeleton or DWO. */
- ULONGEST ranges_base = 0;
+ /* The DW_AT_GNU_ranges_base attribute, if present.
+
+ This is only relevant in the context of pre-DWARF 5 split units. In this
+ context, there is a .debug_ranges section in the linked executable,
+ containing all the ranges data for all the compilation units. Each
+ skeleton/stub unit has (if needed) a DW_AT_GNU_ranges_base attribute that
+ indicates the base of its contribution to that section. The DW_AT_ranges
+ attributes in the split-unit are of the form DW_FORM_sec_offset and point
+ into the .debug_ranges section of the linked file. However, they are not
+ "true" DW_FORM_sec_offset, because they are relative to the base of their
+ compilation unit's contribution, rather than relative to the beginning of
+ the section. The DW_AT_GNU_ranges_base value must be added to it to make
+ it relative to the beginning of the section.
+
+ Note that the value is zero when we are not in a pre-DWARF 5 split-unit
+ case, so this value can be added without needing to know whether we are in
+ this case or not.
+
+ N.B. If a DW_AT_ranges attribute is found on the DW_TAG_compile_unit in the
+ skeleton/stub, it must not have the base added, as it already points to the
+ right place. And since the DW_TAG_compile_unit DIE in the split-unit can't
+ have a DW_AT_ranges attribute, we can use the
+
+ die->tag != DW_AT_compile_unit
+
+ to determine whether the base should be added or not. */
+ ULONGEST gnu_ranges_base = 0;
+
+ /* The DW_AT_rnglists_base attribute, if present.
+
+ This is used when processing attributes of form DW_FORM_rnglistx in
+ non-split units. Attributes of this form found in a split unit don't
+ use it, as split-unit files have their own non-shared .debug_rnglists.dwo
+ section. */
+ ULONGEST rnglists_base = 0;
/* The DW_AT_loclists_base attribute if present. */
ULONGEST loclist_base = 0;
@@ -6967,10 +6992,17 @@ read_cutu_die_from_dwo (dwarf2_cu *cu,
cu->addr_base = stub_comp_unit_die->addr_base ();
- /* There should be a DW_AT_rnglists_base (DW_AT_GNU_ranges_base) attribute
- here (if needed). We need the value before we can process
- DW_AT_ranges. */
- cu->ranges_base = stub_comp_unit_die->ranges_base ();
+ /* There should be a DW_AT_GNU_ranges_base attribute here (if needed).
+ We need the value before we can process DW_AT_ranges values from the
+ DWO. */
+ cu->gnu_ranges_base = stub_comp_unit_die->gnu_ranges_base ();
+
+ /* For DWARF5: record the DW_AT_rnglists_base value from the skeleton. If
+ there are attributes of form DW_FORM_rnglistx in the skeleton, they'll
+ need the rnglists base. Attributes of form DW_FORM_rnglistx in the
+ split unit don't use it, as the DWO has its own .debug_rnglists.dwo
+ section. */
+ cu->rnglists_base = stub_comp_unit_die->rnglists_base ();
}
else if (stub_comp_dir != NULL)
{
@@ -14654,20 +14686,14 @@ dwarf2_get_pc_bounds (struct die_info *die, CORE_ADDR *lowpc,
attr = dwarf2_attr (die, DW_AT_ranges, cu);
if (attr != nullptr && attr->form_is_unsigned ())
{
- /* DW_AT_rnglists_base does not apply to DIEs from the DWO skeleton.
- We take advantage of the fact that DW_AT_ranges does not appear
- in DW_TAG_compile_unit of DWO files.
-
- Attributes of the form DW_FORM_rnglistx have already had their
- value changed by read_rnglist_index and already include
- DW_AT_rnglists_base, so don't need to add the ranges base,
- either. */
- int need_ranges_base = (die->tag != DW_TAG_compile_unit
- && attr->form != DW_FORM_rnglistx);
- unsigned int ranges_offset = (attr->as_unsigned ()
- + (need_ranges_base
- ? cu->ranges_base
- : 0));
+ /* Offset in the .debug_ranges or .debug_rnglist section (depending
+ on DWARF version). */
+ ULONGEST ranges_offset = attr->as_unsigned ();
+
+ /* See dwarf2_cu::gnu_ranges_base's doc for why we might want to add
+ this value. */
+ if (die->tag != DW_TAG_compile_unit)
+ ranges_offset += cu->gnu_ranges_base;
/* Value of the DW_AT_ranges attribute is the offset in the
.debug_ranges section. */
@@ -14832,24 +14858,17 @@ dwarf2_record_block_ranges (struct die_info *die, struct block *block,
attr = dwarf2_attr (die, DW_AT_ranges, cu);
if (attr != nullptr && attr->form_is_unsigned ())
{
- /* DW_AT_rnglists_base does not apply to DIEs from the DWO skeleton.
- We take advantage of the fact that DW_AT_ranges does not appear
- in DW_TAG_compile_unit of DWO files.
-
- Attributes of the form DW_FORM_rnglistx have already had their
- value changed by read_rnglist_index and already include
- DW_AT_rnglists_base, so don't need to add the ranges base,
- either. */
- int need_ranges_base = (die->tag != DW_TAG_compile_unit
- && attr->form != DW_FORM_rnglistx);
+ /* Offset in the .debug_ranges or .debug_rnglist section (depending
+ on DWARF version). */
+ ULONGEST ranges_offset = attr->as_unsigned ();
- /* The value of the DW_AT_ranges attribute is the offset of the
- address range list in the .debug_ranges section. */
- unsigned long offset = (attr->as_unsigned ()
- + (need_ranges_base ? cu->ranges_base : 0));
+ /* See dwarf2_cu::gnu_ranges_base's doc for why we might want to add
+ this value. */
+ if (die->tag != DW_TAG_compile_unit)
+ ranges_offset += cu->gnu_ranges_base;
std::vector<blockrange> blockvec;
- dwarf2_ranges_process (offset, cu, die->tag,
+ dwarf2_ranges_process (ranges_offset, cu, die->tag,
[&] (CORE_ADDR start, CORE_ADDR end)
{
start += baseaddr;
@@ -19264,7 +19283,7 @@ read_full_die_1 (const struct die_reader_specs *reader,
attr = die->attr (DW_AT_rnglists_base);
if (attr != nullptr)
- cu->ranges_base = attr->as_unsigned ();
+ cu->rnglists_base = attr->as_unsigned ();
if (any_need_reprocess)
{
@@ -19792,26 +19811,15 @@ partial_die_info::read (const struct die_reader_specs *reader,
case DW_AT_ranges:
{
- /* DW_AT_rnglists_base does not apply to DIEs from the DWO
- skeleton. We take advantage of the fact the DW_AT_ranges
- does not appear in DW_TAG_compile_unit of DWO files.
-
- Attributes of the form DW_FORM_rnglistx have already had
- their value changed by read_rnglist_index and already
- include DW_AT_rnglists_base, so don't need to add the ranges
- base, either. */
- int need_ranges_base = (tag != DW_TAG_compile_unit
- && attr.form != DW_FORM_rnglistx);
- /* It would be nice to reuse dwarf2_get_pc_bounds here,
- but that requires a full DIE, so instead we just
- reimplement it. */
- unsigned int ranges_offset = (attr.as_unsigned ()
- + (need_ranges_base
- ? cu->ranges_base
- : 0));
-
- /* Value of the DW_AT_ranges attribute is the offset in the
- .debug_ranges section. */
+ /* Offset in the .debug_ranges or .debug_rnglist section (depending
+ on DWARF version). */
+ ULONGEST ranges_offset = attr.as_unsigned ();
+
+ /* See dwarf2_cu::gnu_ranges_base's doc for why we might want to add
+ this value. */
+ if (tag != DW_TAG_compile_unit)
+ ranges_offset += cu->gnu_ranges_base;
+
if (dwarf2_ranges_read (ranges_offset, &lowpc, &highpc, cu,
nullptr, tag))
has_pc_info = 1;
@@ -20254,8 +20262,12 @@ read_rnglist_index (struct dwarf2_cu *cu, ULONGEST rnglist_index,
ULONGEST rnglist_header_size =
(cu->header.initial_length_size == 4 ? RNGLIST_HEADER_SIZE32
: RNGLIST_HEADER_SIZE64);
+
+ /* When reading a DW_FORM_rnglistx from a DWO, we read from the DWO's
+ .debug_rnglists.dwo section. The rnglists base given in the skeleton
+ doesn't apply. */
ULONGEST rnglist_base =
- (cu->dwo_unit != nullptr) ? rnglist_header_size : cu->ranges_base;
+ (cu->dwo_unit != nullptr) ? rnglist_header_size : cu->rnglists_base;
/* Offset in .debug_rnglists of the offset for RNGLIST_INDEX. */
ULONGEST start_offset =
diff --git a/gdb/testsuite/gdb.dwarf2/loclists-sec-offset.c b/gdb/testsuite/gdb.dwarf2/loclists-sec-offset.c
index 2bffbf2ac4c0..924c6ccd4126 100644
--- a/gdb/testsuite/gdb.dwarf2/loclists-sec-offset.c
+++ b/gdb/testsuite/gdb.dwarf2/loclists-sec-offset.c
@@ -29,9 +29,25 @@ func2 (void)
return 2;
}
+static int
+func3 (void)
+{
+ asm ("func3_label: .global func3_label\n");
+ return 3;
+}
+
+static int
+func4 (void)
+{
+ asm ("func4_label: .global func4_label\n");
+ return 4;
+}
+
int
main (void)
{
func1 ();
func2 ();
+ func3 ();
+ func4 ();
}
diff --git a/gdb/testsuite/gdb.dwarf2/loclists-sec-offset.exp b/gdb/testsuite/gdb.dwarf2/loclists-sec-offset.exp
index 9a9188b10744..ecd85ee116c1 100644
--- a/gdb/testsuite/gdb.dwarf2/loclists-sec-offset.exp
+++ b/gdb/testsuite/gdb.dwarf2/loclists-sec-offset.exp
@@ -32,14 +32,18 @@ foreach_with_prefix is_64 {false true} {
set testfile ${testfile}-dw32
}
- # Get the addresses / lengths of func1 and func2.
+ # Get the addresses / lengths of functions.
lassign [function_range func1 $srcdir/$subdir/$srcfile] func1_addr func1_len
lassign [function_range func2 $srcdir/$subdir/$srcfile] func2_addr func2_len
+ lassign [function_range func3 $srcdir/$subdir/$srcfile] func3_addr func3_len
+ lassign [function_range func4 $srcdir/$subdir/$srcfile] func4_addr func4_len
set asm_file [standard_output_file $srcfile2]
Dwarf::assemble $asm_file {
global func1_addr func1_len
global func2_addr func2_len
+ global func3_addr func3_len
+ global func4_addr func4_len
global is_64
declare_labels cu_range_list foo_range_list
@@ -50,12 +54,13 @@ foreach_with_prefix is_64 {false true} {
version 5
is_64 $is_64
} {
- declare_labels int_type
- declare_labels foo_location_list
+ declare_labels int_type1
+ declare_labels int_type2
+ declare_labels foo_location_list bar_location_list
DW_TAG_compile_unit {
} {
- int_type: DW_TAG_base_type {
+ int_type1: DW_TAG_base_type {
{DW_AT_byte_size 4 DW_FORM_data1}
{DW_AT_encoding @DW_ATE_signed}
{DW_AT_name "int"}
@@ -64,7 +69,7 @@ foreach_with_prefix is_64 {false true} {
DW_TAG_variable {
{DW_AT_name "foo"}
{DW_AT_location $foo_location_list DW_FORM_sec_offset}
- {DW_AT_type :$int_type}
+ {DW_AT_type :$int_type1}
}
DW_TAG_subprogram {
@@ -81,6 +86,43 @@ foreach_with_prefix is_64 {false true} {
}
}
+ # This CU uses the DW_FORM_sec_offset form to refer to the
+ # .debug_loclists section, but also has the DW_AT_loclists_base
+ # attribute present. The DW_AT_loclists_base is not used to interpret
+ # the DW_AT_location value, but it should also do no harm.
+ cu {
+ version 5
+ is_64 $is_64
+ } {
+ DW_TAG_compile_unit {
+ {DW_AT_loclists_base cu2_table DW_FORM_sec_offset}
+ } {
+ int_type2: DW_TAG_base_type {
+ {DW_AT_byte_size 4 DW_FORM_data1}
+ {DW_AT_encoding @DW_ATE_signed}
+ {DW_AT_name "int"}
+ }
+
+ DW_TAG_variable {
+ {DW_AT_name "bar"}
+ {DW_AT_location $bar_location_list DW_FORM_sec_offset}
+ {DW_AT_type :$int_type2}
+ }
+
+ DW_TAG_subprogram {
+ {DW_AT_name "func3"}
+ {DW_AT_low_pc $func3_addr}
+ {DW_AT_high_pc $func3_len DW_FORM_udata}
+ }
+
+ DW_TAG_subprogram {
+ {DW_AT_name "func4"}
+ {DW_AT_low_pc $func4_addr}
+ {DW_AT_high_pc $func4_len DW_FORM_udata}
+ }
+ }
+ }
+
loclists -is-64 $is_64 {
# The lists in this table are accessed by direct offset
# (DW_FORM_sec_offset).
@@ -97,6 +139,20 @@ foreach_with_prefix is_64 {false true} {
}
}
}
+
+ table -post-header-label cu2_table {
+ bar_location_list: list_ {
+ start_length $func3_addr $func3_len {
+ DW_OP_constu 0x345678
+ DW_OP_stack_value
+ }
+
+ start_length $func4_addr $func4_len {
+ DW_OP_constu 0x456789
+ DW_OP_stack_value
+ }
+ }
+ }
}
}
@@ -112,6 +168,8 @@ foreach_with_prefix is_64 {false true} {
gdb_breakpoint "func1"
gdb_breakpoint "func2"
+ gdb_breakpoint "func3"
+ gdb_breakpoint "func4"
gdb_continue_to_breakpoint "func1"
with_test_prefix "at func1" {
@@ -122,4 +180,14 @@ foreach_with_prefix is_64 {false true} {
with_test_prefix "at func2" {
gdb_test "print /x foo" " = 0x234567"
}
+
+ gdb_continue_to_breakpoint "func3"
+ with_test_prefix "at func3" {
+ gdb_test "print /x bar" " = 0x345678"
+ }
+
+ gdb_continue_to_breakpoint "func4"
+ with_test_prefix "at func4" {
+ gdb_test "print /x bar" " = 0x456789"
+ }
}
diff --git a/gdb/testsuite/gdb.dwarf2/rnglists-sec-offset.exp b/gdb/testsuite/gdb.dwarf2/rnglists-sec-offset.exp
index d898d11c0dc9..2bcbe0aeea8a 100644
--- a/gdb/testsuite/gdb.dwarf2/rnglists-sec-offset.exp
+++ b/gdb/testsuite/gdb.dwarf2/rnglists-sec-offset.exp
@@ -35,7 +35,8 @@ foreach_with_prefix is_64 {false true} {
Dwarf::assemble $asm_file {
global is_64
- declare_labels cu_range_list foo_range_list
+ declare_labels cu1_range_list cu2_range_list
+ declare_labels foo_range_list bar_range_list
# This CU uses the DW_FORM_sec_offset form to refer to the .debug_rnglists
# section.
@@ -44,7 +45,7 @@ foreach_with_prefix is_64 {false true} {
is_64 $is_64
} {
DW_TAG_compile_unit {
- {DW_AT_ranges $cu_range_list DW_FORM_sec_offset}
+ {DW_AT_ranges $cu1_range_list DW_FORM_sec_offset}
} {
DW_TAG_subprogram {
{DW_AT_name "foo"}
@@ -53,12 +54,31 @@ foreach_with_prefix is_64 {false true} {
}
}
+ # This CU uses the DW_FORM_sec_offset form to refer to the
+ # .debug_rnglists section, but also has the DW_AT_rnglists_base
+ # attribute present. The DW_AT_rnglists_base attribute is not used to
+ # interpret the DW_AT_ranges value, but it should also do no harm.
+ cu {
+ version 5
+ is_64 $is_64
+ } {
+ DW_TAG_compile_unit {
+ {DW_AT_ranges $cu2_range_list DW_FORM_sec_offset}
+ {DW_AT_rnglists_base cu2_table DW_FORM_sec_offset}
+ } {
+ DW_TAG_subprogram {
+ {DW_AT_name "bar"}
+ {DW_AT_ranges $bar_range_list DW_FORM_sec_offset}
+ }
+ }
+ }
+
rnglists -is-64 $is_64 {
# The lists in this table are accessed by direct offset
# (DW_FORM_sec_offset).
table {
- # For the CU.
- cu_range_list: list_ {
+ # For the first CU.
+ cu1_range_list: list_ {
start_end 0x4000 0x5000
}
@@ -67,6 +87,18 @@ foreach_with_prefix is_64 {false true} {
start_end 0x4000 0x4010
}
}
+
+ table -post-header-label cu2_table {
+ # For the second CU.
+ cu2_range_list: list_ {
+ start_end 0x5000 0x6000
+ }
+
+ # For the bar function.
+ bar_range_list: list_ {
+ start_end 0x5000 0x5010
+ }
+ }
}
}
@@ -77,4 +109,5 @@ foreach_with_prefix is_64 {false true} {
# Sanity checks to make sure GDB slurped the symbols correctly.
gdb_test "p/x &foo" " = 0x4000"
+ gdb_test "p/x &bar" " = 0x5000"
}
--
2.30.0
^ permalink raw reply [flat|nested] 34+ messages in thread* [PATCH 12/13] gdb/dwarf: make read_{loc, rng}list_index return sect_offset
2021-01-20 5:39 [PATCH 00/13] DWARF 5 rnglists & loclists fixes (PR 26813) Simon Marchi
` (10 preceding siblings ...)
2021-01-20 5:39 ` [PATCH 11/13] gdb/dwarf: split dwarf2_cu::ranges_base in two Simon Marchi
@ 2021-01-20 5:39 ` Simon Marchi
2021-02-25 19:26 ` Tom Tromey
2021-01-20 5:39 ` [PATCH 13/13] gdb/testsuite: add test for .debug_{rng, loc}lists section without offset array Simon Marchi
2021-02-02 15:43 ` [PATCH 00/13] DWARF 5 rnglists & loclists fixes (PR 26813) Simon Marchi
13 siblings, 1 reply; 34+ messages in thread
From: Simon Marchi @ 2021-01-20 5:39 UTC (permalink / raw)
To: gdb-patches; +Cc: Zoran Zaric, Simon Marchi
From: Simon Marchi <simon.marchi@efficios.com>
I think it's wrong that read_loclist_index and read_rnglist_index return
a CORE_ADDR. A CORE_ADDR is an address in the program. These functions
return offset in sections (.debug_loclists and .debug_rnglists). I
think sect_offset is more appropriate.
I'm wondering if struct attribute should have a "set_sect_offset"
method, that takes a sect_offset parameter, or if it's better to be
left as a simple "unsigned".
gdb/ChangeLog:
* dwarf2/read.c (read_loclist_index, read_rnglist_index): Return
a sect_offset.
(read_attribute_reprocess): Adjust.
Change-Id: I0e22e0864130fb490072b41ae099762918b8ad4d
---
gdb/dwarf2/read.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 86a69efe8ecc..b4ac290ca319 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -20195,7 +20195,8 @@ lookup_loclist_base (struct dwarf2_cu *cu)
/* Given a DW_FORM_loclistx value LOCLIST_INDEX, fetch the offset from the
array of offsets in the .debug_loclists section. */
-static CORE_ADDR
+
+static sect_offset
read_loclist_index (struct dwarf2_cu *cu, ULONGEST loclist_index)
{
dwarf2_per_objfile *per_objfile = cu->per_objfile;
@@ -20245,14 +20246,15 @@ read_loclist_index (struct dwarf2_cu *cu, ULONGEST loclist_index)
const gdb_byte *info_ptr = section->buffer + start_offset;
if (cu->header.offset_size == 4)
- return bfd_get_32 (abfd, info_ptr) + loclist_base;
+ return (sect_offset) (bfd_get_32 (abfd, info_ptr) + loclist_base);
else
- return bfd_get_64 (abfd, info_ptr) + loclist_base;
+ return (sect_offset) (bfd_get_64 (abfd, info_ptr) + loclist_base);
}
/* Given a DW_FORM_rnglistx value RNGLIST_INDEX, fetch the offset from the
array of offsets in the .debug_rnglists section. */
-static CORE_ADDR
+
+static sect_offset
read_rnglist_index (struct dwarf2_cu *cu, ULONGEST rnglist_index,
dwarf_tag tag)
{
@@ -20309,9 +20311,9 @@ read_rnglist_index (struct dwarf2_cu *cu, ULONGEST rnglist_index,
const gdb_byte *info_ptr = section->buffer + start_offset;
if (cu->header.offset_size == 4)
- return read_4_bytes (abfd, info_ptr) + rnglist_base;
+ return (sect_offset) (read_4_bytes (abfd, info_ptr) + rnglist_base);
else
- return read_8_bytes (abfd, info_ptr) + rnglist_base;
+ return (sect_offset) (read_8_bytes (abfd, info_ptr) + rnglist_base);
}
/* Process the attributes that had to be skipped in the first round. These
@@ -20332,18 +20334,18 @@ read_attribute_reprocess (const struct die_reader_specs *reader,
break;
case DW_FORM_loclistx:
{
- CORE_ADDR loclists_sect_off
+ sect_offset loclists_sect_off
= read_loclist_index (cu, attr->as_unsigned_reprocess ());
- attr->set_unsigned (loclists_sect_off);
+ attr->set_unsigned (to_underlying (loclists_sect_off));
}
break;
case DW_FORM_rnglistx:
{
- CORE_ADDR rnglists_sect_off
+ sect_offset rnglists_sect_off
= read_rnglist_index (cu, attr->as_unsigned_reprocess (), tag);
- attr->set_unsigned (rnglists_sect_off);
+ attr->set_unsigned (to_underlying (rnglists_sect_off));
}
break;
case DW_FORM_strx:
--
2.30.0
^ permalink raw reply [flat|nested] 34+ messages in thread* [PATCH 13/13] gdb/testsuite: add test for .debug_{rng, loc}lists section without offset array
2021-01-20 5:39 [PATCH 00/13] DWARF 5 rnglists & loclists fixes (PR 26813) Simon Marchi
` (11 preceding siblings ...)
2021-01-20 5:39 ` [PATCH 12/13] gdb/dwarf: make read_{loc, rng}list_index return sect_offset Simon Marchi
@ 2021-01-20 5:39 ` Simon Marchi
2021-02-02 15:43 ` [PATCH 00/13] DWARF 5 rnglists & loclists fixes (PR 26813) Simon Marchi
13 siblings, 0 replies; 34+ messages in thread
From: Simon Marchi @ 2021-01-20 5:39 UTC (permalink / raw)
To: gdb-patches; +Cc: Zoran Zaric, Simon Marchi
From: Simon Marchi <simon.marchi@efficios.com>
It is possible for the tables in the .debug_{rng,loc}lists sections to
not have an array of offsets. In that case, the offset_entry_count
field of the header is 0. The forms DW_FORM_{rng,loc}listx (reference
by index) can't be used with that table. Instead, the
DW_FORM_sec_offset form, which references a {rng,loc}list by direct
offset in the section, must be used. From what I saw, this is what GCC
currently produces.
Add tests for this case. I didn't see any bug related to this, I just
think that it would be nice to have coverage for this. A new
`-with-offset-array` option is added to the `table` procs, used when
generating {rng,loc}lists, to decide whether to generate the offset
array.
gdb/testsuite/ChangeLog:
* lib/dwarf.exp (rnglists): Add -no-offset-array option to
table proc.
* gdb.dwarf2/rnglists-sec-offset.exp: Add test for
.debug_rnglists table without offset array.
* gdb.dwarf2/loclists-sec-offset.exp: Add test for
.debug_loclists table without offset array.
Change-Id: I8e34a7bf68c9682215ffbbf66600da5b7db91ef7
---
.../gdb.dwarf2/loclists-sec-offset.c | 16 +++++
.../gdb.dwarf2/loclists-sec-offset.exp | 70 ++++++++++++++++++-
.../gdb.dwarf2/rnglists-sec-offset.exp | 34 ++++++++-
gdb/testsuite/lib/dwarf.exp | 48 ++++++++++---
4 files changed, 155 insertions(+), 13 deletions(-)
diff --git a/gdb/testsuite/gdb.dwarf2/loclists-sec-offset.c b/gdb/testsuite/gdb.dwarf2/loclists-sec-offset.c
index 924c6ccd4126..f8076406463f 100644
--- a/gdb/testsuite/gdb.dwarf2/loclists-sec-offset.c
+++ b/gdb/testsuite/gdb.dwarf2/loclists-sec-offset.c
@@ -43,6 +43,20 @@ func4 (void)
return 4;
}
+static int
+func5 (void)
+{
+ asm ("func5_label: .global func5_label\n");
+ return 5;
+}
+
+static int
+func6 (void)
+{
+ asm ("func6_label: .global func6_label\n");
+ return 6;
+}
+
int
main (void)
{
@@ -50,4 +64,6 @@ main (void)
func2 ();
func3 ();
func4 ();
+ func5 ();
+ func6 ();
}
diff --git a/gdb/testsuite/gdb.dwarf2/loclists-sec-offset.exp b/gdb/testsuite/gdb.dwarf2/loclists-sec-offset.exp
index ecd85ee116c1..573324af3d17 100644
--- a/gdb/testsuite/gdb.dwarf2/loclists-sec-offset.exp
+++ b/gdb/testsuite/gdb.dwarf2/loclists-sec-offset.exp
@@ -37,6 +37,8 @@ foreach_with_prefix is_64 {false true} {
lassign [function_range func2 $srcdir/$subdir/$srcfile] func2_addr func2_len
lassign [function_range func3 $srcdir/$subdir/$srcfile] func3_addr func3_len
lassign [function_range func4 $srcdir/$subdir/$srcfile] func4_addr func4_len
+ lassign [function_range func5 $srcdir/$subdir/$srcfile] func5_addr func5_len
+ lassign [function_range func6 $srcdir/$subdir/$srcfile] func6_addr func6_len
set asm_file [standard_output_file $srcfile2]
Dwarf::assemble $asm_file {
@@ -44,6 +46,8 @@ foreach_with_prefix is_64 {false true} {
global func2_addr func2_len
global func3_addr func3_len
global func4_addr func4_len
+ global func5_addr func5_len
+ global func6_addr func6_len
global is_64
declare_labels cu_range_list foo_range_list
@@ -56,7 +60,10 @@ foreach_with_prefix is_64 {false true} {
} {
declare_labels int_type1
declare_labels int_type2
- declare_labels foo_location_list bar_location_list
+ declare_labels int_type3
+ declare_labels foo_location_list
+ declare_labels bar_location_list
+ declare_labels baz_location_list
DW_TAG_compile_unit {
} {
@@ -123,6 +130,41 @@ foreach_with_prefix is_64 {false true} {
}
}
+ # This CU uses the DW_FORM_sec_offset form to refer to the
+ # .debug_loclists section. The corresponding contribution in the
+ # .debug_loclists section has no offset array.
+ cu {
+ version 5
+ is_64 $is_64
+ } {
+ DW_TAG_compile_unit {
+ } {
+ int_type3: DW_TAG_base_type {
+ {DW_AT_byte_size 4 DW_FORM_data1}
+ {DW_AT_encoding @DW_ATE_signed}
+ {DW_AT_name "int"}
+ }
+
+ DW_TAG_variable {
+ {DW_AT_name "baz"}
+ {DW_AT_location $baz_location_list DW_FORM_sec_offset}
+ {DW_AT_type :$int_type3}
+ }
+
+ DW_TAG_subprogram {
+ {DW_AT_name "func5"}
+ {DW_AT_low_pc $func5_addr}
+ {DW_AT_high_pc $func5_len DW_FORM_udata}
+ }
+
+ DW_TAG_subprogram {
+ {DW_AT_name "func6"}
+ {DW_AT_low_pc $func6_addr}
+ {DW_AT_high_pc $func6_len DW_FORM_udata}
+ }
+ }
+ }
+
loclists -is-64 $is_64 {
# The lists in this table are accessed by direct offset
# (DW_FORM_sec_offset).
@@ -153,6 +195,20 @@ foreach_with_prefix is_64 {false true} {
}
}
}
+
+ table -with-offset-array false {
+ baz_location_list: list_ {
+ start_length $func5_addr $func5_len {
+ DW_OP_constu 0x567890
+ DW_OP_stack_value
+ }
+
+ start_length $func6_addr $func6_len {
+ DW_OP_constu 0x678901
+ DW_OP_stack_value
+ }
+ }
+ }
}
}
@@ -170,6 +226,8 @@ foreach_with_prefix is_64 {false true} {
gdb_breakpoint "func2"
gdb_breakpoint "func3"
gdb_breakpoint "func4"
+ gdb_breakpoint "func5"
+ gdb_breakpoint "func6"
gdb_continue_to_breakpoint "func1"
with_test_prefix "at func1" {
@@ -190,4 +248,14 @@ foreach_with_prefix is_64 {false true} {
with_test_prefix "at func4" {
gdb_test "print /x bar" " = 0x456789"
}
+
+ gdb_continue_to_breakpoint "func5"
+ with_test_prefix "at func5" {
+ gdb_test "print /x baz" " = 0x567890"
+ }
+
+ gdb_continue_to_breakpoint "func6"
+ with_test_prefix "at func6" {
+ gdb_test "print /x baz" " = 0x678901"
+ }
}
diff --git a/gdb/testsuite/gdb.dwarf2/rnglists-sec-offset.exp b/gdb/testsuite/gdb.dwarf2/rnglists-sec-offset.exp
index 2bcbe0aeea8a..0733e90abc74 100644
--- a/gdb/testsuite/gdb.dwarf2/rnglists-sec-offset.exp
+++ b/gdb/testsuite/gdb.dwarf2/rnglists-sec-offset.exp
@@ -35,8 +35,8 @@ foreach_with_prefix is_64 {false true} {
Dwarf::assemble $asm_file {
global is_64
- declare_labels cu1_range_list cu2_range_list
- declare_labels foo_range_list bar_range_list
+ declare_labels cu1_range_list cu2_range_list cu3_range_list
+ declare_labels foo_range_list bar_range_list baz_range_list
# This CU uses the DW_FORM_sec_offset form to refer to the .debug_rnglists
# section.
@@ -73,6 +73,23 @@ foreach_with_prefix is_64 {false true} {
}
}
+ # This CU uses the DW_FORM_sec_offset form to refer to the .debug_rnglists
+ # section. The corresponding contribution in the .debug_rnglists has no
+ # offset array.
+ cu {
+ version 5
+ is_64 $is_64
+ } {
+ DW_TAG_compile_unit {
+ {DW_AT_ranges $cu3_range_list DW_FORM_sec_offset}
+ } {
+ DW_TAG_subprogram {
+ {DW_AT_name "baz"}
+ {DW_AT_ranges $baz_range_list DW_FORM_sec_offset}
+ }
+ }
+ }
+
rnglists -is-64 $is_64 {
# The lists in this table are accessed by direct offset
# (DW_FORM_sec_offset).
@@ -99,6 +116,18 @@ foreach_with_prefix is_64 {false true} {
start_end 0x5000 0x5010
}
}
+
+ table -with-offset-array false {
+ # For the third CU.
+ cu3_range_list: list_ {
+ start_end 0x6000 0x7000
+ }
+
+ # For the baz function.
+ baz_range_list: list_ {
+ start_end 0x6000 0x6010
+ }
+ }
}
}
@@ -110,4 +139,5 @@ foreach_with_prefix is_64 {false true} {
# Sanity checks to make sure GDB slurped the symbols correctly.
gdb_test "p/x &foo" " = 0x4000"
gdb_test "p/x &bar" " = 0x5000"
+ gdb_test "p/x &baz" " = 0x6000"
}
diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
index b444ef36778a..10fd88f6e0fa 100644
--- a/gdb/testsuite/lib/dwarf.exp
+++ b/gdb/testsuite/lib/dwarf.exp
@@ -1392,6 +1392,10 @@ namespace eval Dwarf {
# The -post-header-label option can be used to define a label just after
# the header of the table. This is the label that a DW_AT_rnglists_base
# attribute will usually refer to.
+ #
+ # The `-with-offset-array true|false` option can be used to control
+ # whether the headers of the location list tables have an array of
+ # offset. The default is true.
proc table { args } {
variable _debug_rnglists_table_count
@@ -1399,7 +1403,10 @@ namespace eval Dwarf {
variable _debug_rnglists_offset_size
variable _debug_rnglists_is_64_dwarf
- parse_args {{post-header-label ""}}
+ parse_args {
+ {post-header-label ""}
+ {with-offset-array true}
+ }
if { [llength $args] != 1 } {
error "table proc expects one positional argument (body)"
@@ -1477,7 +1484,12 @@ namespace eval Dwarf {
_op .2byte 5 "dwarf version"
_op .byte $_debug_rnglists_addr_size "address size"
_op .byte 0 "segment selector size"
- _op .4byte "$_debug_rnglists_list_count" "offset entry count"
+
+ if { ${with-offset-array} } {
+ _op .4byte "$_debug_rnglists_list_count" "offset entry count"
+ } else {
+ _op .4byte 0 "offset entry count"
+ }
define_label $post_header_label
@@ -1487,9 +1499,11 @@ namespace eval Dwarf {
}
# Emit the offset array.
- for {set list_idx 0} {$list_idx < $_debug_rnglists_list_count} {incr list_idx} {
- set list_label [_compute_list_label $list_idx]
- _op .${_debug_rnglists_offset_size}byte "$list_label - $post_header_label" "offset of list $list_idx"
+ if { ${with-offset-array} } {
+ for {set list_idx 0} {$list_idx < $_debug_rnglists_list_count} {incr list_idx} {
+ set list_label [_compute_list_label $list_idx]
+ _op .${_debug_rnglists_offset_size}byte "$list_label - $post_header_label" "offset of list $list_idx"
+ }
}
# Emit the actual list data.
@@ -1565,6 +1579,10 @@ namespace eval Dwarf {
# The -post-header-label option can be used to define a label just after the
# header of the table. This is the label that a DW_AT_loclists_base
# attribute will usually refer to.
+ #
+ # The `-with-offset-array true|false` option can be used to control
+ # whether the headers of the location list tables have an array of
+ # offset. The default is true.
proc table { args } {
variable _debug_loclists_table_count
@@ -1572,7 +1590,10 @@ namespace eval Dwarf {
variable _debug_loclists_offset_size
variable _debug_loclists_is_64_dwarf
- parse_args {{post-header-label ""}}
+ parse_args {
+ {post-header-label ""}
+ {with-offset-array true}
+ }
if { [llength $args] != 1 } {
error "table proc expects one positional argument (body)"
@@ -1672,7 +1693,12 @@ namespace eval Dwarf {
_op .2byte 5 "DWARF version"
_op .byte $_debug_loclists_addr_size "address size"
_op .byte 0 "segment selector size"
- _op .4byte $_debug_loclists_list_count "offset entry count"
+
+ if { ${with-offset-array} } {
+ _op .4byte "$_debug_loclists_list_count" "offset entry count"
+ } else {
+ _op .4byte 0 "offset entry count"
+ }
define_label $post_header_label
@@ -1682,9 +1708,11 @@ namespace eval Dwarf {
}
# Emit the offset array.
- for {set list_idx 0} {$list_idx < $_debug_loclists_list_count} {incr list_idx} {
- set list_label [_compute_list_label $list_idx]
- _op .${_debug_loclists_offset_size}byte "$list_label - $post_header_label" "offset of list $list_idx"
+ if { ${with-offset-array} } {
+ for {set list_idx 0} {$list_idx < $_debug_loclists_list_count} {incr list_idx} {
+ set list_label [_compute_list_label $list_idx]
+ _op .${_debug_loclists_offset_size}byte "$list_label - $post_header_label" "offset of list $list_idx"
+ }
}
# Emit the actual list data.
--
2.30.0
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 00/13] DWARF 5 rnglists & loclists fixes (PR 26813)
2021-01-20 5:39 [PATCH 00/13] DWARF 5 rnglists & loclists fixes (PR 26813) Simon Marchi
` (12 preceding siblings ...)
2021-01-20 5:39 ` [PATCH 13/13] gdb/testsuite: add test for .debug_{rng, loc}lists section without offset array Simon Marchi
@ 2021-02-02 15:43 ` Simon Marchi
13 siblings, 0 replies; 34+ messages in thread
From: Simon Marchi @ 2021-02-02 15:43 UTC (permalink / raw)
To: gdb-patches
I pushed this series.
Simon
On 2021-01-20 12:39 a.m., Simon Marchi wrote:
> This series fixes a few issues related to rnglists and loclists,
> prompted by PR 26813:
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=26813
>
> Simon Marchi (13):
> gdb/dwarf: change read_loclist_index complaints into errors
> gdb/dwarf: fix bound check in read_rnglist_index
> gdb/dwarf: add missing bound check to read_loclist_index
> gdb/dwarf: remove unnecessary check in read_{rng,loc}list_index
> gdb/dwarf: few fixes for handling DW_FORM_{rng,loc}listx
> gdb/dwarf: read correct rnglist/loclist header in
> read_{rng,loc}list_index
> gdb/dwarf: read DW_AT_ranges value as unsigned in
> partial_die_info::read
> gdb/testsuite: add .debug_rnglists tests
> gdb/testsuite: DWARF assembler: add context parameters to _location
> gdb/testsuite: add .debug_loclists tests
> gdb/dwarf: split dwarf2_cu::ranges_base in two
> gdb/dwarf: make read_{loc,rng}list_index return sect_offset
> gdb/testsuite: add test for .debug_{rng,loc}lists section without
> offset array
>
> gdb/dwarf2/attribute.c | 5 +-
> gdb/dwarf2/attribute.h | 1 +
> gdb/dwarf2/die.h | 36 +-
> gdb/dwarf2/read.c | 257 ++++++----
> .../gdb.dwarf2/loclists-multiple-cus.c | 37 ++
> .../gdb.dwarf2/loclists-multiple-cus.exp | 146 ++++++
> .../gdb.dwarf2/loclists-sec-offset.c | 69 +++
> .../gdb.dwarf2/loclists-sec-offset.exp | 261 ++++++++++
> .../gdb.dwarf2/rnglists-multiple-cus.exp | 102 ++++
> .../gdb.dwarf2/rnglists-sec-offset.exp | 143 ++++++
> gdb/testsuite/lib/dwarf.exp | 451 +++++++++++++++++-
> 11 files changed, 1381 insertions(+), 127 deletions(-)
> create mode 100644 gdb/testsuite/gdb.dwarf2/loclists-multiple-cus.c
> create mode 100644 gdb/testsuite/gdb.dwarf2/loclists-multiple-cus.exp
> create mode 100644 gdb/testsuite/gdb.dwarf2/loclists-sec-offset.c
> create mode 100644 gdb/testsuite/gdb.dwarf2/loclists-sec-offset.exp
> create mode 100644 gdb/testsuite/gdb.dwarf2/rnglists-multiple-cus.exp
> create mode 100644 gdb/testsuite/gdb.dwarf2/rnglists-sec-offset.exp
>
^ permalink raw reply [flat|nested] 34+ messages in thread