[draft patch 0/6] Split FYI and some review notes

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

Hello Aleksandar,

to send something here is some draft, there will be still some changes.

I had first difficulties to apply it to FSF GDB HEAD as it has some conflicts
so I had to rebase it first.

It is not a normal review as your patch has merged code moves + code
modifications which is unreviewable.  Moves and modifications need to be in
separate patches so that one can see what has changed in the diff.

fileio_read_stralloc you have reimplemented for gdbserver; I have rather
chosen to generalize the gdb/ variant and call the same common/ code from both
gdb and gdbserver.  One may ask whether it is not more complicated than its
reimplementation, not sure.

By moving linux_find_memory_regions_full you have dropped make_cleanup there.
Its use from GDB may now leak memory as its called FUNC may throw an
exception.  I have put there 'void **memory_to_free_ptr' to make it reusable
from both gdb and gdbserver.

I tried to make the patches similar to your code so that one can diff the
result against your patch.

There were many code formatting little changes and also needless casts
(probably from C++).

For the last [draft patch 6/6] - things not done there yet:

get_dynamic already parses/scans PHDRs, you introduce new PHDR parser, they
should be merged.

get_hex_build_id must not be based on soname, moreover playing with realname
on it, it is too fragile.  It should be based on addresses, you know that l_ld
is absolute address inside the library.  So find maps/smaps entry containing
l_ld, subtract its file offset from vaddr and you have the ELF header address.

Do not run get_hex_build_id quadratically - currently for each library you
open and scan maps/smaps again.  Probably the best approach is to scan all
l_ld addresses from r_debug first, then qsort them and then linearly match
them to the maps/smaps entries.  I am not sure but I guess they still should
be returned to GDB in their original order from r_debug for some backward and
solib-svr4.c compatibility.  Also Gary Benson is working on incremental shlib
transfers from gdbserver so that it does not clash too much (it will anyway).
While just quadratic computation would be in real world acceptable all the
open/read/close syscalls I find really needlessly expensive.

PT_NOTE segment contains arbitrary number of notes, up to its size.  You check
only the first entry there.

On Fri, 22 Feb 2013 16:06:44 +0100, Aleksandar Ristovski wrote:
> As per Jan's request, this patch adds 'build-id' to the gdbserver
> response. The value is hex encoded string representing
> .note.gnu.build-id section of the corresponding shared library.

gdbserver reports PT_NOTE segment, not any section.  gdbserver is dependent on
runtime information (segments), it cannot rely on debug/link information
(sections).

BTW ping me (possibly off-list) whether you plan to work on it now yourself or
whether I should be doing more of the changes described above; I sure have
also enough other work...


Thanks,
Jan