Re: [PATCH] Merge handle_inferior_event and handle_inferior_event_1

[Pedro Alves <palves at redhat dot com>]

On 03/25/2019 03:35 PM, Tom Tromey wrote:
>>>>>> ">" == Sergio Durigan Junior <sergiodj@redhat.com> writes:
> 
>>> A few days ago (March 18th) a user reported a bug against Fedora GDB:
>>>
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1690120
>>>
>>> And, after hours and hours investigating this, I found that your commit
>>> actually fixes it.  Not sure if it was intended or not, but what a
>>> coincidence...
> 
> I am curious to know how this change could fix this bug.
> That seems mysterious to me.

There's one behavior change the patch makes, which is that before the patch,
if handle_inferior_event_1 threw an error, value_free_to_mark would not
be called.  After the patch, it's always called, from scoped_value_mark's dtor.

>From the backtrace in the bug report, we see that the crash happens late
during teardown, destroying an xmethod value in the global "all_values" vector:

 ...
 #13 std::vector<gdb::ref_ptr<value, value_ref_policy>, std::allocator<gdb::ref_ptr<value, value_ref_policy> > >::~vector (this=0x559d15b24a10 <all_values>, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/stl_vector.h:677
 No locals.
 #14 0x00007f92a8fe0700 in __run_exit_handlers () from /lib64/libc.so.6
 No symbol table info available.
 #15 0x00007f92a8fe0840 in exit () from /lib64/libc.so.6
 No symbol table info available.
 ...

So it looks like your patch made it so that the value in question
avoids the crash because the value in question is no longer in the
all_values vector by the time gdb is tearing down?

Open questions then would be:

 - what was the exception in question that was thrown from
   within handle_inferior_event_1?  (And if there was
   no exception, then the theory above is incorrect.)

 - why don't non-temporary xmethod values put in the value history
   cause a similar crash at tear down time?

>>> Of course, something I'd also like to ask is: do you think this change
>>> is trivial enough to be included in the 8.3 release?  I'd like to see it
>>> happen.  I can create a bug if needed.
> 
> I would have thought so but the fact that it causes this behavior makes
> me worry a bit.  Maybe that's backward!

Thanks,
Pedro Alves