[PATCH v2 3/3] gdb/jit: enable tracking multiple jitter objfiles

[Simon Marchi]

On 2020-06-16 5:49 a.m., Tankut Baris Aktemur wrote:
>        objf_data = get_jit_objfile_data (reg_symbol.objfile);
>        objf_data->register_code = reg_symbol.minsym;
>        objf_data->descriptor = desc_symbol.minsym;

This made me realize that we now have per-JITer-objfile information in both
jit_objfile_data and the new jiter_and_bp structure.  I think it would be more
consistent to bring it all in one of them.

I see two way forward:

1. Move register_code and descriptor from jit_objfile_data to jiter_and_bp.
   jiter_and_bp becomes the source of truth for information about JITer
   objfiles.  jit_objfile_data would only be used to decribe JITed objfiles.
2. Move the content of jiter_and_bp to jit_objfile_data.
   If we still think it's useful to have a list of JITer objfiles,
   jit_program_space_data::jiter_and_bps would become a list of objfiles
   known to be JITers:

     /* Objfiles in this program space that define the JIT interface symbols.  */
     std::forward_list<objfile *> jiters;

I would prefer #2, because using registries is kind of our standard way to keep
per-stuff data (where stuff is objfile, program_space, inferior, etc).

And instead of having the "jiters" list, when we want to iterate on jiters, maybe
we can just iterate on progspace->objfiles () and check those objfiles whose
jit_objfile_data indicate they are JITers.  The tradeoff is: it's a bit less
efficient, but the code is simpler (one less list to maintain, that is possibly
out of sync).  If the operations that require iterating on the existing JITers are
not on some hot path, it should be fine.

While digging into this, there are two improvements I'd make (orthogonal to this
patch):

- Make jit.c use the newer / type-safe / c++-friendly objfile_key instead of objfile_data.
- Split jit_objfile_data in two: one type for the JITers and one type for the JITed.  Even
  today, it is used for both, and that's kind of confusing.

I ended up trying all of this to make sure it made sense, instead of just throwing ideas
out there, so I took the time to make a "clean" branch.  See here:

  https://github.com/simark/binutils-gdb/tree/multiple-jiters

Please tell me what you think about this.  If you agree with the direction, I could
officially post it to the list.

There's one more thing I would improve (and I think it would also apply to your version
of the patch): in jit_breakpoint_re_set_internal, we keep looking up symbols in the same
libraries over and over.  Maybe we could mark the objfiles to say that we have already
looked up the symbols, as there's no point in looking them up again in the future.  Either
an objfile doesn't have them, and therefore will never have them.  Or it has them and we
already have them.  We could then just skip to the step where we check if the relocated
address has changed.

Simon