Re: [PATCH][PR build/24805] Explicitly export symbols from gdb_proc_service

[Norbert Lange <nolange79@gmail.com>]

Am Di., 14. Jan. 2020 um 16:36 Uhr schrieb Pedro Alves <palves@redhat.com>:
>
> We can't drop support for the glibc header, since that would mean
> to also drop support for the Solaris version of the header, and
> also for whatever other libcs that people build gdb with (e.g.,
> does musl have its own version of the header with different types?).
> Up until not so long ago, glibc didn't use to install the header,
> that's why we keep a local copy, IIRC.

No, you don't get what I am trying to say.
Commonly you only use external headers for function calls,
and the exact definition can vary a bit.

What you are doing, for ex. in proc-service.c is implementing the function.

like:
> ps_err_e
> ps_pdread (struct ps_prochandle *ph, psaddr_t addr, void *buf, size_t size)

you don't get much, if any, information from the header that's not replicated
in the source. The only upside would be to be able to detect mismatches
(but that could be done in a separate test_external_proc_header.c,
potentially even during configuration).

Whatever other libc's (musl doesnt provide/use this AFAIK) or OS's do,
including their header does not help at all?

You get no mismatch: Don't need it.
You get a mismatch: you need to fix that in your c file aswell.

>
> How about something like this?  It's similar to your #2 at
> <https://sourceware.org/bugzilla/show_bug.cgi?id=24805#c3>, but
> I'm using typeof to avoid issues with different systems using
> different prototypes.

Yeah, that's better, unless typeof is not supported by the compiler
(or its C++ mode).

Am Di., 14. Jan. 2020 um 16:47 Uhr schrieb Pedro Alves <palves@redhat.com>:
>
> On 1/14/20 3:36 PM, Pedro Alves wrote:
> > On 1/6/20 11:39 PM, Norbert Lange wrote:
> >> Am Mo., 6. Jan. 2020 um 20:21 Uhr schrieb Pedro Alves <palves@redhat.com>:
> >>>
> >>> On 1/4/20 8:20 PM, Norbert Lange wrote:
> >>>> Compiling GDB with '-fvisibility=hidden' will remove the
> >>>> symbols that should be exported.
> >>>> This patch explicitly marks them as visible.
> >>>
> >>> Curious.  We have gdb/proc-service.list supposedly for this,
> >>> doesn't -Wl,--dynamic-list work with -fvisibility=hidden then?
> >>>
> >>
> >> Obviously it doesn't, else I would not have spent time figuring out
> >> why libthread_db wont load.
> >
> > OK.  Haven't looked at visibility issues in years.  It also wasn't clear
> > to me whether the issue could be that -Wl,--dynamic-list wasn't used
> > in your build for some reason, maybe related to how you're configuring GDB.
> >
> >> -Wl,--dynamic-list merely filters the visible symbols, it does not see
> >> "hidden" ones.
> >
> > BTW, you didn't post an actual patch to the list:
> >  https://sourceware.org/ml/gdb-patches/2020-01/msg00083.html
>
> I knew I must have been confused.  I found your patch on the list,
> as a reply to that...

NP

>
> > How about something like this?  It's similar to your #2 at
> > <https://sourceware.org/bugzilla/show_bug.cgi?id=24805#c3>, but
> > I'm using typeof to avoid issues with different systems using
> > different prototypes.
>
> BTW, I forgot to mention why I suggested this as alternative
> to the push/pop approach.  It was that the push/pop approach
> makes everything indirectly included by <proc_service.h>
> have default visibility too.  I don't know whether that ends
> up being any function in practice, though.

Yes I am aware of this issue, and its a latent problem I guess.
Normally you don't implement header definitions, so you might be ok.

Adding tests whether the functions are exported would be nice.

Norbert.