[RFC][PATCH] fix gdb segv when objfile can't be opened

Mike Gulick mike.gulick@mathworks.com
Thu Oct 19 19:39:00 GMT 2017 

On 10/19/2017 01:54 PM, Simon Marchi wrote:
> On 2017-10-19 11:59, Mike Gulick wrote:
>> I apologize for the improperly formatted patch -- I'm really struggling
>> to get thunderbird to behave as I want.
>>
>> Here is an updated patch.  I would have sent it with git send-email, but
>> I could not figure out the proper way to add this preface before the
>> patch (without it looking like part of the commit message).
> 
> Hi Mike,
> 
> Thanks, I was able to apply this version correctly.
> 
> If I have a short comment that's not meant to be in the commit message, I usually
> include it in brackets like this:
> 
>   [Re-sending this patch because the first try was not formatted correctly.]
> 
> If it's longer you can always end it with a line "Actual commit message:".  Either way, it's not really a big deal, as long as it's clear.  You can use the --annotate option of git-send-email to edit the message before sending it.
> 
Thanks.

>> ---
>> From 5dee04076518554e4baae864569d6f4faee9b685 Mon Sep 17 00:00:00 2001
>> From: Mike Gulick <mgulick@mathworks.com>
>> Date: Wed, 18 Oct 2017 16:04:27 -0400
>> Subject: [PATCH] fix gdb segv when objfile can't be opened
>>
>> This fixes PR 16577.
>>
>> This patch changes gdb_bfd_map_section to issue a warning rather than an
>> error if it is unable to read the object file, and sets the size of the
>> section/frame that it attempted to read to 0 on error.
>>
>> The description of gdb_bfd_map_section states that it will try to read
>> or map the contents of the section SECT, and if successful, the section
>> data is returned and *SIZE is set to the size of the section data.  This
>> function was throwing an error and leaving *size as-is.  Setting the
>> section size to 0 indicates to dwarf2_build_frame_info that there is no
>> data to read, otherwise it will try to read from an invalid frame
>> pointer.
>>
>> Changing the error to a warning allows this to be handled gracefully.
>> Additionally, the error was clobbering the breakpoint output indicating
>> the current frame (function name, arguments, source file, and line number).
>> E.g.
>>
>> Thread 3 "foo" hit Breakpoint 1, BFD: reopening
>> /tmp/jna-1013829440/jna2973250704389291330.tmp: No such file or
>> directory
>>
>> BFD: reopening /tmp/jna-1013829440/jna2973250704389291330.tmp: No such
>> file or directory
> 
> For some reason, I am not able to reproduce the crash using the instructions in the bug report, and gdb master.
> 
> (gdb) up
> #1  0x00007ffff78d525a in sleep () from /lib/x86_64-linux-gnu/libc.so.6
> (gdb)
> BFD: reopening ./badlib.so: No such file or directory
> 
> BFD: reopening ./badlib.so: No such file or directory
> 
> Can't read data for section '.eh_frame' in file './badlib.so'
> (gdb)
> Initial frame selected; you cannot go up.
> (gdb)
> Initial frame selected; you cannot go up.
> (gdb)
> Initial frame selected; you cannot go up.
> (gdb) bt
> #0  0x00007ffff78d52f0 in nanosleep () from /lib/x86_64-linux-gnu/libc.so.6
> #1  0x00007ffff78d525a in sleep () from /lib/x86_64-linux-gnu/libc.so.6
> 
> 
> Would you be able to create a test case to reproduce it?  We would need one to go in with the fix in the end anyway, and it's easier for reviewers if they can just run a test file rather than try to reproduce by hand.  You can start by copying an existing solib test, like gdb.base/solib-display.exp.  See here for more details about tests:
> 
> http://sourceware.org/gdb/wiki/TestingGDB
> http://sourceware.org/gdb/wiki/GDBTestcaseCookbook
> 
> Don't hesitate to ask here or on IRC if you need assistance.
> 

I attached a new reproducer in the bug report that reproduces this
problem in the current git master (and more closely follows the problem
I was seeing in our large C++/Java application).  This causes the error
messages to be emitted when the breakpoint is hit, and the stepping
forward triggers the segfault.

I'll take a look at building a test case.

>> (gdb)
>>
>> While the "BFD: reopening ..." messages will still appear interspersed in the
>> breakpoint output, the current frame info is now displayed:
>>
>> Thread 3 "foo" hit Breakpoint 1, BFD: reopening
>> /tmp/jna-1013829440/jna1875755897659885075.tmp: No such file or
>> directory
>>
>> BFD: reopening /tmp/jna-1013829440/jna1875755897659885075.tmp: No such
>> file or directory
>>
>> warning: Can't read data for section '.eh_frame' in file
>> '/tmp/jna-1013829440/jna1875755897659885075.tmp'
>> do_something () at file.cpp:80
>> 80    {
>> (gdb)
>> ---
>>  gdb/gdb_bfd.c | 12 +++++++++---
>>  1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
>> index 29080b8..229f5ae 100644
>> --- a/gdb/gdb_bfd.c
>> +++ b/gdb/gdb_bfd.c
>> @@ -705,9 +705,15 @@ gdb_bfd_map_section (asection *sectp, bfd_size_type *size)
>>
>>    data = NULL;
>>    if (!bfd_get_full_section_contents (abfd, sectp, &data))
>> -    error (_("Can't read data for section '%s' in file '%s'"),
>> -       bfd_get_section_name (abfd, sectp),
>> -       bfd_get_filename (abfd));
>> +    {
>> +      warning (_("Can't read data for section '%s' in file '%s'"),
>> +           bfd_get_section_name (abfd, sectp),
>> +           bfd_get_filename (abfd));
>> +      /* Section is invalid -- set size to 0 and return NULL */
>> +      descriptor->size = 0;
>> +      *size = descriptor->size;
>> +      return (const gdb_byte *) NULL;
>> +    }
>>    descriptor->data = data;
>>
>>   done:
> 
> I don't know if it is really this function's responsibility to clear *size in case of error, or it would be the callers responsibility to properly check for errors.  But if the function doesn't throw anymore, the comment in gdb_bfd.h should be updated accordingly.

I had trouble figuring out what the 'error' function actually does (I
couldn't find where it was defined).  When I'm stepping through GDB in
the debugger, the lines past 'error' never seem to get called.  It's
like 'error' throws an exception that is caught elsewhere.  I was also
unable to figure out why the error message isn't displayed.  The new
reproducer shows this issue.  I wasn't sure if setting *size or even
descriptor->size was the right thing to do, but it seemed reasonable to
me since the comment in gdb_bfd.h states that this function updates
*size.  There's currently only one caller for 'gdb_bfd_map_section', so
I have no problem updating *size there if that is preferred.

Thanks,
Mike

More information about the Gdb-patches mailing list