Patchwork always export the symbols for the proc_service interface

login
register
mail settings
Submitter Norbert Lange
Date Jan. 4, 2020, 8:20 p.m.
Message ID <20200104202004.83439-2-nolange79@gmail.com>
Download mbox | patch
Permalink /patch/37198/
State New
Headers show

Comments

Norbert Lange - Jan. 4, 2020, 8:20 p.m.
Compiling GDB with '-fvisibility=hidden' will remove the
symbols that should be exported.
This patch explicitly marks them as visible.

gdb/ChangeLog

	PR build/24805
        * gdbsupport/gdb_proc_service.h: push/pop visibility
        attribute for exported functions.
---
 gdb/gdbsupport/gdb_proc_service.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
Tom Tromey - Jan. 6, 2020, 4:07 p.m.
>>>>> "Norbert" == Norbert Lange <nolange79@gmail.com> writes:

Norbert> Compiling GDB with '-fvisibility=hidden' will remove the
Norbert> symbols that should be exported.
Norbert> This patch explicitly marks them as visible.

Norbert> gdb/ChangeLog

Norbert> 	PR build/24805
Norbert>         * gdbsupport/gdb_proc_service.h: push/pop visibility
Norbert>         attribute for exported functions.

Thanks for the patch.

I have one question and one comment.

Norbert> +++ b/gdb/gdbsupport/gdb_proc_service.h
Norbert> @@ -23,6 +23,11 @@
 
Norbert>  #ifdef HAVE_PROC_SERVICE_H
 
Norbert> +/* ensure the symbols are public, even if the default is hidden */

In the gdb style, comments should be complete sentences, which end in a
period followed by 2 spaces.  So this should read:

/* Ensure the symbols are public, even if the default is hidden.  */

Norbert> +#if __GNUC__ >= 4
Norbert> +#pragma GCC visibility pop
Norbert> +#endif

Would it be better to just have this done a single time at the top and
bottom of the file?

thanks,
Tom
Norbert Lange - Jan. 6, 2020, 11:22 p.m.
Am Mo., 6. Jan. 2020 um 17:07 Uhr schrieb Tom Tromey <tom@tromey.com>:
>
> >>>>> "Norbert" == Norbert Lange <nolange79@gmail.com> writes:
>
> Norbert> Compiling GDB with '-fvisibility=hidden' will remove the
> Norbert> symbols that should be exported.
> Norbert> This patch explicitly marks them as visible.
>
> Norbert> gdb/ChangeLog
>
> Norbert>        PR build/24805
> Norbert>         * gdbsupport/gdb_proc_service.h: push/pop visibility
> Norbert>         attribute for exported functions.
>
> Thanks for the patch.
>
> I have one question and one comment.
>
> Norbert> +++ b/gdb/gdbsupport/gdb_proc_service.h
> Norbert> @@ -23,6 +23,11 @@
>
> Norbert>  #ifdef HAVE_PROC_SERVICE_H
>
> Norbert> +/* ensure the symbols are public, even if the default is hidden */
>
> In the gdb style, comments should be complete sentences, which end in a
> period followed by 2 spaces.  So this should read:
>
> /* Ensure the symbols are public, even if the default is hidden.  */

Noted, am I required to redo the patch if that's the only change?

>
> Norbert> +#if __GNUC__ >= 4
> Norbert> +#pragma GCC visibility pop
> Norbert> +#endif
>
> Would it be better to just have this done a single time at the top and
> bottom of the file?

there are other headers included in between, so that's a bad idea
(and you could do attach function attributes instead for the second case,
see patch in bug report.)

I still dont get why the external header is supported at all, if the
implementation
needs to match 100% anyway (unlike where you just call functions from a header).

Norbert

Patch

diff --git a/gdb/gdbsupport/gdb_proc_service.h b/gdb/gdbsupport/gdb_proc_service.h
index 8565da3286..72abb5c1c0 100644
--- a/gdb/gdbsupport/gdb_proc_service.h
+++ b/gdb/gdbsupport/gdb_proc_service.h
@@ -23,6 +23,11 @@ 
 
 #ifdef HAVE_PROC_SERVICE_H
 
+/* ensure the symbols are public, even if the default is hidden */
+#if __GNUC__ >= 4
+#pragma GCC visibility push(default)
+#endif
+
 /* glibc's proc_service.h doesn't wrap itself with extern "C".  Need
    to do it ourselves.  */
 EXTERN_C_PUSH
@@ -31,6 +36,10 @@  EXTERN_C_PUSH
 
 EXTERN_C_POP
 
+#if __GNUC__ >= 4
+#pragma GCC visibility pop
+#endif
+
 #else /* HAVE_PROC_SERVICE_H */
 
 /* The following fallback definitions have been imported and adjusted
@@ -69,6 +78,10 @@  EXTERN_C_POP
 # endif
 #endif
 
+#if __GNUC__ >= 4
+#pragma GCC visibility push(default)
+#endif
+
 EXTERN_C_PUSH
 
 /* Functions in this interface return one of these status codes.  */
@@ -168,6 +181,10 @@  extern void ps_plog (const char *fmt, ...);
 
 EXTERN_C_POP
 
+#if __GNUC__ >= 4
+#pragma GCC visibility pop
+#endif
+
 #endif /* HAVE_PROC_SERVICE_H */
 
 #endif /* COMMON_GDB_PROC_SERVICE_H */