[ was: Re: [PATCH 7/9] [libbacktrace] Handle DW_FORM_GNU_ref_alt ]
On 18-01-19 15:25, Ian Lance Taylor wrote:
> On Thu, Jan 17, 2019 at 6:14 AM Tom de Vries <tdevr...@suse.de> wrote:
>>
>> On 17-01-19 01:35, Ian Lance Taylor wrote:
>>> On Wed, Jan 16, 2019 at 4:17 PM Tom de Vries <tdevr...@suse.de> wrote:
>>>>
>>>> this handles DW_FORM_GNU_ref_alt which references the .debug_info
>>>> section in the .gnu_debugaltlink file.
>>>>
>>>> OK for trunk?
>>>>
>>>> Thanks,
>>>> - Tom
>>>>
>>>> On 11-12-18 11:14, Tom de Vries wrote:
>>>>> 2018-12-10 Tom de Vries <tdevr...@suse.de>
>>>>>
>>>>> * dwarf.c (enum attr_val_encoding): Add ATTR_VAL_REF_ALT_INFO.
>>>>> (struct unit): Add low and high fields.
>>>>> (struct unit_vector): New type.
>>>>> (struct dwarf_data): Add units and units_counts fields.
>>>>> (read_attribute): Handle DW_FORM_GNU_ref_alt using
>>>>> ATTR_VAL_REF_ALT_INFO.
>>>>> (find_unit): New function.
>>>>> (find_address_ranges): Add and handle unit_tag parameter.
>>>>> (build_address_map): Add and handle units_vec parameter.
>>>>> (read_referenced_name_1): Handle DW_FORM_GNU_ref_alt.
>>>>> (build_dwarf_data): Pass units_vec to build_address_map. Store
>>>>> resulting
>>>>> units vector.
>>>
>>>
>>>>> @@ -281,6 +283,10 @@ struct unit
>>>>> /* The offset of UNIT_DATA from the start of the information for
>>>>> this compilation unit. */
>>>>> size_t unit_data_offset;
>>>>> + /* Start of the compilation unit. */
>>>>> + size_t low;
>>>>> + /* End of the compilation unit. */
>>>>> + size_t high;
>>>
>>> The comments should say what low and high are measured in, which I
>>> guess is file offset. Or is it offset from the start of UNIT_DATA?
>>> Either way, If that is right, then the fields should be named
>>> low_offset and high_offset. Otherwise it seems easy to confuse with
>>> function_addrs, where low and high refer to PC values.
>>>
>>
>> Done.
>>
>>> Also if they are offsets from UNIT_DATA then size_t is OK, but if the
>>> are file offsets they should be off_t.
>>>
>>
>> AFAIU, in the case where off_t vs size_t would make a difference, we're
>> running into trouble much earlier. I've filed PR 88890 - "libbacktrace
>> on 32-bit system with _FILE_OFFSET_BITS == 64" (
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88890 ) about this.
>>
>> Anyway, I've made the conservative choice of using off_t for now (but I
>> could argue that it's a memory offset, given that the assumption is that
>> the entire debug section is read into memory).
>
> Since the entire debug section is read into memory, if they are
> offsets from UNIT_DATA, they should be size_t, not off_t. I wasn't
> trying to say that we should make a conservative choice here, I was
> trying to say that we should make a correct choice. An offset from
> UNIT_DATA should be size_t, a file offset should be off_t.
These are offsets from the start of the .debug_info section, so AFAIU
that means these could be size_t.
Tested with x86_64 build and host libbacktrace make check.
OK for trunk if bootstrap and reg-test on x86_64 succeeds?
Thanks,
- Tom
[libbacktrace] Use size_t for low_offset/high_offset fields of struct unit
2019-01-22 Tom de Vries <tdevr...@suse.de>
* dwarf.c (struct unit): Use size_t for low_offset/high_offset fields.
(units_search, find_unit): Use size_t for offset.
(build_address_map): Use size_t for unit_offset.
---
libbacktrace/dwarf.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/libbacktrace/dwarf.c b/libbacktrace/dwarf.c
index aacbd3a453d..e78d36b0b43 100644
--- a/libbacktrace/dwarf.c
+++ b/libbacktrace/dwarf.c
@@ -285,10 +285,10 @@ struct unit
size_t unit_data_offset;
/* Offset of the start of the compilation unit from the start of the
.debug_info section. */
- off_t low_offset;
+ size_t low_offset;
/* Offset of the end of the compilation unit from the start of the
.debug_info section. */
- off_t high_offset;
+ size_t high_offset;
/* DWARF version. */
int version;
/* Whether unit is DWARF64. */
@@ -891,9 +891,9 @@ read_attribute (enum dwarf_form form, struct dwarf_buf *buf,
static int
units_search (const void *vkey, const void *ventry)
{
- const off_t *key = (const off_t *) vkey;
+ const size_t *key = (const size_t *) vkey;
const struct unit *entry = *((const struct unit *const *) ventry);
- off_t offset;
+ size_t offset;
offset = *key;
if (offset < entry->low_offset)
@@ -907,7 +907,7 @@ units_search (const void *vkey, const void *ventry)
/* Find a unit in PU containing OFFSET. */
static struct unit *
-find_unit (struct unit **pu, size_t units_count, off_t offset)
+find_unit (struct unit **pu, size_t units_count, size_t offset)
{
struct unit **u;
u = bsearch (&offset, pu, units_count, sizeof (struct unit *), units_search);
@@ -1514,7 +1514,7 @@ build_address_map (struct backtrace_state *state, uintptr_t base_address,
size_t i;
struct unit **pu;
size_t prev_addrs_count;
- off_t unit_offset = 0;
+ size_t unit_offset = 0;
memset (&addrs->vec, 0, sizeof addrs->vec);
memset (&unit_vec->vec, 0, sizeof unit_vec->vec);
