Re: JIT debugging (Attach and speed)

On 03/22/2016 04:47 PM, Yichao Yu wrote:
> On Tue, Mar 22, 2016 at 12:41 PM, Pedro Alves <[email protected]> wrote:
>> On 03/22/2016 04:22 PM, Yichao Yu wrote:
>>> On Tue, Mar 22, 2016 at 12:15 PM, Pedro Alves <[email protected]> wrote:
>>>> On 03/22/2016 03:46 PM, Pedro Alves wrote:
>>>
>>>> I re-read the 2011 discussion, and it seems like we had an idea for a fix:
>>>
>>> IIUC the proposed fix might cause regression in some cases?
>>
>> Yeah, there's no full fix available, only some ideas thrown out.
>> The last discussed one wouldn't cause a regression -- the
>> "longjmp"-caching idea.  We may still need to defer breakpoint re-set
>> to at most once per jit load event, something like Paul's original
>> patch, but with a breakpoint_re_set call somewhere.
>>
>> It'd even be better to somehow restrict breakpoint re-setting
>> to the jit modules that were added/removed/changed, but
>> that's harder.
>>
>>>
>>>>>
>>>>> Do you know whether this happens with 7.11 and master, and if so,
>>>>> would it be possible for you to git bisect the culprit?
>>>
>>> This is 7.11 package from ArchLinux. I could try bi-secting although
>>> apparently you are faster at pin-point the issue.
>>>
>>>>
>>>> Currently, jit_inferior_created_hook -> jit_inferior_init is only
>>>> called when the inferior execs...
>>>>
>>>> Grepping around, I think that might have been
>>>> the fix for PR gdb/13431 (03bef283c2d3):
>>>>    https://sourceware.org/ml/gdb-patches/2012-02/msg00023.html
>>>> which removed the inferior_created (jit_inferior_created_observer).
>>>>
>>>> Adding an inferior_created observer back likely fixes the issue.
>>>
>>> I'm happy to test patches.
>>
>> I'm happy to provide guidance, but a fix would likely happen faster
>> if someone else stepped up to write it.
> 
> Are these lines (or at least the first one) the ones you think should
> be added back?
> 
> -  observer_attach_inferior_created (jit_inferior_created_observer);
>     observer_attach_inferior_exit (jit_inferior_exit_hook);
> -  observer_attach_executable_changed (jit_executable_changed_observer);
> 

Something like that.  At least the first one.  Not sure the second is
needed, since with Tromey's change the data is associated with the objfile.

> I can try that although I'm not particularly sure what was the reason
> they are removed 

Not sure either.  I assume studying Tromey's description of the original
change helps bring that to light.

> and how to check for regressions.

GDB has a regression test suite under src/gdb/testsuite/.  The
gdb/testsuite/README file has instructions.  

Basically, run "make check -j8" before the patch, "make check -j8"
after the patch, and diff the resulting testsuite/gdb.sum files.

Note that there are some tests that may be racy on your machine, so you
may get unrelated some noise.  Running a particular test a
couple times, with:

 make check TESTS="gdb.base/foo.exp"

should help you determine whether that's the case.

It'd be very nice if we had a _new_ test that covers your use case,
to avoid regressing again.  That likely makes the patch bigger than
what we could accept without a copyright assignment though.  If you'd
like to pursue that, let me know and I'll send you the forms.

Thanks,
Pedro Alves