[review v2] jit: remove bp locations when unregistering jit code

["Simon Marchi (Code Review)" <gerrit at gnutoolchain-gerrit dot osci dot io>]

Simon Marchi has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/704
......................................................................


Patch Set 2:

> Sorry for the delayed response, had switch to other issues.
> 
> > You said that the test suite doesn't execute jit-ed code, but it seems like we do execute some jit-ed code in the jitreader.exp test (the code is jit-ed and executed in jithost.c).  It might not be too hard to augment it to reproduce the bug (even if it's just hackish at first)?
> > 
> > Otherwise, jit-main.c (for jit.exp) takes the approach of copying an ELF file into memory (and adjusting some addresses).  It might be possible to lookup a function symbol in this mapped ELF file and execute it?
> 
> I was able to tweak jit-main.c to adjust address for jit_function_XXXX and execute it but to reproduce the bug I'd also need to adjust addresses in debug sections too (so that `break jit_function_XXXX` resolves to the correct address) and that seems much more effort.

Hmm indeed I see the problem, relocating the DWARF info would be tricky.

> Same applies to jithost.c - its jit reader does not populate debug symbols right now and it is not possible to put a breakpoint in jit function (which is why it uses hard-coded trap).

Ok, that's odd.  I would expect that when defining a block with a name (like jitreader.c does with "jit_function_00"), it would define a symbol on which you can put a breakpoint.  But it apparently does not.

I'm a bit lost in how all this interacts with opencl though.  Can you explain how the jit comes in play with opencl?  Is there a jit reader for opencl?  In the tests, how can we put a breakpoint on "testkernel", where does that symbol come from?
 
> It is still something I'd really like to make work because currently there is no easy testing facility for more complex jit tests in gdb. Maybe reusing dwarf processing bits from gdb itself can make it feasible, will have to give it a try.

I agree, the more advanced use cases need to be tested.

> Note though that you can run my current opencl test case on a regular x86_64, it is intended to work with CPU opencl runtime (for example https://software.intel.com/en-us/articles/opencl-drivers).

I was able to build and run a simple "hello world" opencl example, but unfortunately I'm unable to run any of the opencl tests in GDB.  I get:

 128 (gdb) tbreak testkernel^M
 129 Function "testkernel" not defined.^M
 130 Make breakpoint pending on future shared library load? (y or [n]) y^M
 131 Temporary breakpoint 1 (testkernel) pending.^M

and the breakpoint is never resolved and hit, the inferior just exits.

I'm not against adding opencl tests, but I think if we can reproduce and test this issue with a plain x86-64 test, it's much more valuable, as nearly everybody will run this test out of the box, so we have more changes of catching any regression.


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Id9133540d67fa0c4619ac88324b0349b89e4b2b1
Gerrit-Change-Number: 704
Gerrit-PatchSet: 2
Gerrit-Owner: Mihails Strasuns <mihails.strasuns@intel.com>
Gerrit-Reviewer: Mihails Strasuns <mihails.strasuns@intel.com>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Luis Machado <luis.machado@linaro.org>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Wed, 11 Dec 2019 05:51:30 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment