Re: [PATCH] gdb/dwarf2: Check for missing abbrev

[Simon Marchi <simark@simark.ca>]

On 3/13/24 16:18, Aaron Merey wrote:
> A corrupt debuginfo file can result in a null abbrev_info pointer
> being passed to cooked_indexer::scan_attributes.  There is no check
> for whether the abbrev pointer is null and SIGSEGV occurs when
> attempting to dereference the pointer.
> 
> Fix this by calling throw_error at the beginning of scan_attributes
> when the abbrev is nullptr.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31478
> ---
>  gdb/dwarf2/read.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index 4afb026b8ce..a6e5a3c856e 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -16077,6 +16077,9 @@ cooked_indexer::scan_attributes (dwarf2_per_cu_data *scanning_per_cu,
>  				 bool *is_enum_class,
>  				 bool for_specification)
>  {
> +  if (abbrev == nullptr)
> +    throw_error (NOT_AVAILABLE_ERROR, _("Unable to find DWARF attributes."));

We generally don't make functions error out like this if they are passed
bad parameters (we can use gdb_assert for that if we really want to).
It's up to the callers to respect the contract and not pass nullptr
where nullptr is not expected.  So I would instead suggest to add the
nullptr check in the caller (in scan_attributes itself, after the
peek_die_abbrev call lower).

However, I'd like if we could analyze the problem a bit further to
understand more precisely what happens, just to be sure that there isn't
a more fundamental problem and we're not just papering over the problem.
I added instructions on the bug that should help you reproduce.

What I see is that read_unsigned_leb128 in peek_die_abbrev unexpectedly
returns 0.  Is it because the corrupted file contains zeros where it
shouldn't?  Or is the file truncated and we are reading past the end of
the buffer, and there happens to be a zero there?

Simon