[1/3] Mark __libc_multiple_libcs with attribute_hidden [BZ #18822]
Commit Message
On Sun, Aug 20, 2017 at 09:54:22AM +0200, Florian Weimer wrote:
> * H. J. Lu:
>
> > [BZ #18822]
> > * csu/libc-start.c (__libc_multiple_libcs): Add attribute_hidden.
> > * elf/dl-open.c (__libc_multiple_libcs): Likewise.
>
> Please put a hidden declaration into a header file instead.
Done. Here is the updated patch. OK for master?
H.J.
---
Since __libc_multiple_libcs is defined as hidden symbol in init-first.c,
it should be always marked with attribute_hidden.
[BZ #18822]
* csu/libc-start.c (__libc_multiple_libcs): Removed.
* elf/dl-open.c: Include <libc-internal.h>.
(__libc_multiple_libcs): Removed.
* elf/dl-sysdep.c: Include <libc-internal.h> instead of
<hp-timing.h>.
* include/libc-internal.h (__libc_multiple_libcs): New.
* misc/sbrk.c: Include <libc-internal.h>.
(__libc_multiple_libcs): Removed.
---
csu/libc-start.c | 2 --
elf/dl-open.c | 3 +--
elf/dl-sysdep.c | 2 +-
include/libc-internal.h | 4 ++++
misc/sbrk.c | 4 +---
5 files changed, 7 insertions(+), 8 deletions(-)
Comments
On 08/20/2017 04:12 PM, H.J. Lu wrote:
> +/* Set nonzero if we have to be prepared for more than one libc being
> + used in the process. */
> +extern int __libc_multiple_libcs attribute_hidden;
I think the comment gives the wrong impression. The flag is not always
set if there are multiple libcs in the process, and it certainly is not
set just because we might end up having multiple libcs in the future
(which is why the “have to be prepared” part irks me).
Thanks,
Florian
On Mon, Aug 21, 2017 at 3:03 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 08/20/2017 04:12 PM, H.J. Lu wrote:
>> +/* Set nonzero if we have to be prepared for more than one libc being
>> + used in the process. */
>> +extern int __libc_multiple_libcs attribute_hidden;
>
> I think the comment gives the wrong impression. The flag is not always
> set if there are multiple libcs in the process, and it certainly is not
> set just because we might end up having multiple libcs in the future
> (which is why the “have to be prepared” part irks me).
I copied it from csu/init-first.c:
/* Set nonzero if we have to be prepared for more than one libc being
used in the process. Safe assumption if initializer never runs. */
int __libc_multiple_libcs attribute_hidden = 1;
Should I just leave out the comments?
On 08/21/2017 01:16 PM, H.J. Lu wrote:
> On Mon, Aug 21, 2017 at 3:03 AM, Florian Weimer <fweimer@redhat.com> wrote:
>> On 08/20/2017 04:12 PM, H.J. Lu wrote:
>>> +/* Set nonzero if we have to be prepared for more than one libc being
>>> + used in the process. */
>>> +extern int __libc_multiple_libcs attribute_hidden;
>>
>> I think the comment gives the wrong impression. The flag is not always
>> set if there are multiple libcs in the process, and it certainly is not
>> set just because we might end up having multiple libcs in the future
>> (which is why the “have to be prepared” part irks me).
>
> I copied it from csu/init-first.c:
>
> /* Set nonzero if we have to be prepared for more than one libc being
> used in the process. Safe assumption if initializer never runs. */
> int __libc_multiple_libcs attribute_hidden = 1;
Ahh, that's only appropriate for this particular definition.
> Should I just leave out the comments?
Yes please, unless someone can describe what the flag actually means (I
tried to use it in the past, and it did not work reliably with inner libcs).
Thanks,
Florian
@@ -27,8 +27,6 @@
extern void __libc_init_first (int argc, char **argv, char **envp);
-extern int __libc_multiple_libcs;
-
#include <tls.h>
#ifndef SHARED
# include <dl-osinfo.h>
@@ -33,12 +33,11 @@
#include <tls.h>
#include <stap-probe.h>
#include <atomic.h>
+#include <libc-internal.h>
#include <dl-dst.h>
-extern int __libc_multiple_libcs; /* Defined in init-first.c. */
-
/* We must be careful not to leave us in an inconsistent state. Thus we
catch any error and re-raise it after cleaning up. */
@@ -41,7 +41,7 @@
#include <dl-machine.h>
#include <dl-procinfo.h>
#include <dl-osinfo.h>
-#include <hp-timing.h>
+#include <libc-internal.h>
#include <tls.h>
#include <dl-tunables.h>
@@ -53,4 +53,8 @@ extern void __init_misc (int, char **, char **);
extern __typeof (__profile_frequency) __profile_frequency attribute_hidden;
# endif
+/* Set nonzero if we have to be prepared for more than one libc being
+ used in the process. */
+extern int __libc_multiple_libcs attribute_hidden;
+
#endif /* _LIBC_INTERNAL */
@@ -18,14 +18,12 @@
#include <errno.h>
#include <stdint.h>
#include <unistd.h>
+#include <libc-internal.h>
/* Defined in brk.c. */
extern void *__curbrk;
extern int __brk (void *addr);
-/* Defined in init-first.c. */
-extern int __libc_multiple_libcs attribute_hidden;
-
/* Extend the process's data space by INCREMENT.
If INCREMENT is negative, shrink data space by - INCREMENT.
Return start of new space allocated, or -1 for errors. */