Re: [PATCH 01/13] gdb/dwarf: change read_loclist_index complaints into errors

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

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