Re: [PATCH v4 2/2] gdb: CTF support

[Wei-min Pan <weimin.pan@oracle.com>]

On 10/15/2019 1:18 PM, Tom Tromey wrote:
 >>>>>> Wei-min Pan <weimin.pan@oracle.com> writes:
 >
 >>> I didn't look, did the top-level changes go in to gcc?
 >>> The top-level configury code is canonically maintained there.
 >
 >> Given the following rules in Makefile.def:
 >
 >> dependencies = { module=configure-gcc; on=all-binutils; };
 >> dependencies = { module=all-binutils; on=all-libctf; };
 >> dependencies = { module=all-gdb; on=all-libctf; };
 >
 >> binutils gets built before gcc does. It's not clear why making changes
 >> into gcc is needed?
 >
 > Sorry, I was not clear enough.
 >
 > Most top-level files, like Makefile.def, are shared between gcc and
 > gdb+binutils.  However, gcc and gdb have different source repositories.
 > In order to reduce the possibility of long-term divergence, the gcc
 > repository was declared the canonical repository -- in general (there
 > are exceptions) -- changes are checked in first there, then brought over
 > to the gdb repository.
 >
 > My question was whether you did this.

Thanks for the explanation. It looks like that I will need to add
gcc/Makefile.def (dependencies): all-gdb depends on all-libctf.

 >
 >>> It's preferable to use the type-safe registry approach in new code.
 >
 >> This register key was not intended to manage the object with new/delete
 >> but to be used to close file descriptors that are associated with the
 >> ctf file/archive.
 >
 > It's still preferable to use the type-safe approach.  You can easily
 > introduce a new deleter object that works just as you like. In fact the
 > code will nearly be identical -- just type-safe.

OK, will make the change.

 >
 >>> gdb doesn't generally use typedefs like this, especially now that it's
 >>> in C++.
 >
 >> It seems there are several places, e.g. aarch64-tdep.c, event-loop.c,
 >> linespec.c, procfs.c, that do. We can drop "typedef" if you prefer.
 >
 > Yes, I'm afraid you can't always judge the current standard in gdb by
 > the existing code, because the transition from C to C++ did not also
 > involve updating every single thing -- just the important things.

OK.

 >
 >>> Instead of this function, it's more usual in gdb to use:
 >>>
 >>>        CORE_ADDR baseaddr
 >>>        = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
 >>>
 >>> Maybe this function is even wrong in some situation, I'm not sure.
 >
 >> You suspect that bfd_get_section_by_name might return a wrong *asection?
 >> Yes, we can use ANOFFSET, as you suggested, for the text base but still
 >> need to get the size of the text section.
 >
 > I'm not sure if it can or not.  Anyway it seems you can use
 > SECT_OFF_TEXT to also get the size... ?

You mean using SECT_OFF_TEXT to get the address?

Will send out the diffs as soon as I'm done with these changes.

Thanks again.