[PATCH] Avoid .symver on common symbols [BZ #21666]

[PATCH] Avoid .symver on common symbols [BZ #21666]

[H.J. Lu]

The .symver directive on common symbol just creates a new common symbol,
not an alias and the newer assembler with the bug fix for

https://sourceware.org/bugzilla/show_bug.cgi?id=21661

will issue an error.  Before the fix, we got

$ readelf -sW libc.so | grep "loc[12s]"
  5109: 00000000003a0608     8 OBJECT  LOCAL  DEFAULT   36 loc1
  5188: 00000000003a0610     8 OBJECT  LOCAL  DEFAULT   36 loc2
  5455: 00000000003a0618     8 OBJECT  LOCAL  DEFAULT   36 locs
  6575: 00000000003a05f0     8 OBJECT  GLOBAL DEFAULT   36 locs@GLIBC_2.2.5
  7156: 00000000003a05f8     8 OBJECT  GLOBAL DEFAULT   36 loc1@GLIBC_2.2.5
  7312: 00000000003a0600     8 OBJECT  GLOBAL DEFAULT   36 loc2@GLIBC_2.2.5

in libc.so.  The versioned loc1, loc2 and locs have the wrong addresses.
After the fix, we got

$ readelf -sW libc.so | grep "loc[12s]"
  6570: 000000000039e3b8     8 OBJECT  GLOBAL DEFAULT   34 locs@GLIBC_2.2.5
  7151: 000000000039e3c8     8 OBJECT  GLOBAL DEFAULT   34 loc1@GLIBC_2.2.5
  7307: 000000000039e3c0     8 OBJECT  GLOBAL DEFAULT   34 loc2@GLIBC_2.2.5

OK for master?

Thanks.

H.J.
---
	[BZ #21666]
	* misc/regexp.c: Include <stdlib.h>.
	(loc1): Initialized to NULL.
	(loc2): Likewise.
	(locs): Likewise.
---
 misc/regexp.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/misc/regexp.c b/misc/regexp.c
index 19d76c0..9017bc1 100644
--- a/misc/regexp.c
+++ b/misc/regexp.c
@@ -29,14 +29,17 @@
 
 #if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_23)
 
-/* Define the variables used for the interface.  */
-char *loc1;
-char *loc2;
+#include <stdlib.h>	/* Get NULL.  */
+
+/* Define the variables used for the interface.  Avoid .symver on common
+   symbol, which just creates a new common symbol, not an alias.  */
+char *loc1 = NULL;
+char *loc2 = NULL;
 compat_symbol (libc, loc1, loc1, GLIBC_2_0);
 compat_symbol (libc, loc2, loc2, GLIBC_2_0);
 
 /* Although we do not support the use we define this variable as well.  */
-char *locs;
+char *locs = NULL;
 compat_symbol (libc, locs, locs, GLIBC_2_0);
 
 
-- 
2.9.4

Re: [PATCH] Avoid .symver on common symbols [BZ #21666]

[Florian Weimer]

On 06/23/2017 06:11 PM, H.J. Lu wrote:
> +/* Define the variables used for the interface.  Avoid .symver on common
> +   symbol, which just creates a new common symbol, not an alias.  */
> +char *loc1 = NULL;
> +char *loc2 = NULL;

I think __attribute__ ((nocommon)) without the initializer would be more
explicit.  We already use that for _res in resolv/res_libc.c.

Does this result in a visible difference for applications?  If yes,
please file a bug for this and reference it in the ChangeLog and commit
message.

Why didn't our test suite catch it?

Thanks,
Florian

Re: [PATCH] Avoid .symver on common symbols [BZ #21666]

[H.J. Lu]

[-- Attachment #1: Type: text/plain, Size: 974 bytes --]

On Fri, Jun 23, 2017 at 9:17 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 06/23/2017 06:11 PM, H.J. Lu wrote:
>> +/* Define the variables used for the interface.  Avoid .symver on common
>> +   symbol, which just creates a new common symbol, not an alias.  */
>> +char *loc1 = NULL;
>> +char *loc2 = NULL;
>
> I think __attribute__ ((nocommon)) without the initializer would be more
> explicit.  We already use that for _res in resolv/res_libc.c.

Done.

> Does this result in a visible difference for applications?  If yes,
> please file a bug for this and reference it in the ChangeLog and commit
> message.

It will be very hard to tell since these symbols were exported from libc.so
by accident and we only keep them in libc.so for backward binary compatibility.
Application can no longer reference them from libc.so.

> Why didn't our test suite catch it?

We never tried to catch errors like this.

Here is the updated patch.  OK for master?

Thanks.

-- 
H.J.

[-- Attachment #2: 0001-Avoid-.symver-on-common-symbols-BZ-21666.patch --]
[-- Type: text/x-patch, Size: 2231 bytes --]

From 67145d84d9719d270f63f4399e337a4bdcb778f5 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Fri, 23 Jun 2017 09:03:19 -0700
Subject: [PATCH] Avoid .symver on common symbols [BZ #21666]

The .symver directive on common symbol just creates a new common symbol,
not an alias and the newer assembler with the bug fix for

https://sourceware.org/bugzilla/show_bug.cgi?id=21661

will issue an error.  Before the fix, we got

$ readelf -sW libc.so | grep "loc[12s]"
  5109: 00000000003a0608     8 OBJECT  LOCAL  DEFAULT   36 loc1
  5188: 00000000003a0610     8 OBJECT  LOCAL  DEFAULT   36 loc2
  5455: 00000000003a0618     8 OBJECT  LOCAL  DEFAULT   36 locs
  6575: 00000000003a05f0     8 OBJECT  GLOBAL DEFAULT   36 locs@GLIBC_2.2.5
  7156: 00000000003a05f8     8 OBJECT  GLOBAL DEFAULT   36 loc1@GLIBC_2.2.5
  7312: 00000000003a0600     8 OBJECT  GLOBAL DEFAULT   36 loc2@GLIBC_2.2.5

in libc.so.  The versioned loc1, loc2 and locs have the wrong addresses.
After the fix, we got

$ readelf -sW libc.so | grep "loc[12s]"
  6570: 000000000039e3b8     8 OBJECT  GLOBAL DEFAULT   34 locs@GLIBC_2.2.5
  7151: 000000000039e3c8     8 OBJECT  GLOBAL DEFAULT   34 loc1@GLIBC_2.2.5
  7307: 000000000039e3c0     8 OBJECT  GLOBAL DEFAULT   34 loc2@GLIBC_2.2.5

	[BZ #21666]
	* misc/regexp.c (loc1): Add __attribute__ ((nocommon));
	(loc2): Likewise.
	(locs): Likewise.
---
 misc/regexp.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/misc/regexp.c b/misc/regexp.c
index 19d76c0..eaea7c3 100644
--- a/misc/regexp.c
+++ b/misc/regexp.c
@@ -29,14 +29,15 @@
 
 #if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_23)
 
-/* Define the variables used for the interface.  */
-char *loc1;
-char *loc2;
+/* Define the variables used for the interface.  Avoid .symver on common
+   symbol, which just creates a new common symbol, not an alias.  */
+char *loc1 __attribute__ ((nocommon));
+char *loc2 __attribute__ ((nocommon));
 compat_symbol (libc, loc1, loc1, GLIBC_2_0);
 compat_symbol (libc, loc2, loc2, GLIBC_2_0);
 
 /* Although we do not support the use we define this variable as well.  */
-char *locs;
+char *locs __attribute__ ((nocommon));
 compat_symbol (libc, locs, locs, GLIBC_2_0);
 
 
-- 
2.9.4

Re: [PATCH] Avoid .symver on common symbols [BZ #21666]

[Florian Weimer]

On 06/23/2017 06:26 PM, H.J. Lu wrote:

>> Does this result in a visible difference for applications?  If yes,
>> please file a bug for this and reference it in the ChangeLog and commit
>> message.
> 
> It will be very hard to tell since these symbols were exported from libc.so
> by accident and we only keep them in libc.so for backward binary compatibility.
> Application can no longer reference them from libc.so.

Ah, it's the old <regexp.h> interface, and the symbols were exported
deliberately.  We apparently do not have any tests for it.

> Here is the updated patch.  OK for master?

Yes, please.  Thanks.

Florian

Re: [PATCH] Avoid .symver on common symbols [BZ #21666]

[Szabolcs Nagy]

On 23/06/17 22:13, Florian Weimer wrote:
> On 06/23/2017 06:26 PM, H.J. Lu wrote:
> 
>>> Does this result in a visible difference for applications?  If yes,
>>> please file a bug for this and reference it in the ChangeLog and commit
>>> message.
>>
>> It will be very hard to tell since these symbols were exported from libc.so
>> by accident and we only keep them in libc.so for backward binary compatibility.
>> Application can no longer reference them from libc.so.
> 
> Ah, it's the old <regexp.h> interface, and the symbols were exported
> deliberately.  We apparently do not have any tests for it.
> 
>> Here is the updated patch.  OK for master?
> 
> Yes, please.  Thanks.
> 

i think this should be ok for backporting too
since without it old releases cannot be built
with new binutils.

Re: [PATCH] Avoid .symver on common symbols [BZ #21666]

[H.J. Lu]

On Wed, Jul 12, 2017 at 9:56 AM, Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> On 23/06/17 22:13, Florian Weimer wrote:
>> On 06/23/2017 06:26 PM, H.J. Lu wrote:
>>
>>>> Does this result in a visible difference for applications?  If yes,
>>>> please file a bug for this and reference it in the ChangeLog and commit
>>>> message.
>>>
>>> It will be very hard to tell since these symbols were exported from libc.so
>>> by accident and we only keep them in libc.so for backward binary compatibility.
>>> Application can no longer reference them from libc.so.
>>
>> Ah, it's the old <regexp.h> interface, and the symbols were exported
>> deliberately.  We apparently do not have any tests for it.
>>
>>> Here is the updated patch.  OK for master?
>>
>> Yes, please.  Thanks.
>>
>
> i think this should be ok for backporting too
> since without it old releases cannot be built
> with new binutils.
>

I have ported it to hjl/pr21120/2.25 branch.  I also backported 2 patches for
GCC 7. See:

https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/hjl/pr21120/2.25



-- 
H.J.

Re: [PATCH] Avoid .symver on common symbols [BZ #21666]

[Zack Weinberg]

On Fri, Jun 23, 2017 at 5:13 PM, Florian Weimer <fweimer@redhat.com> wrote:
> On 06/23/2017 06:26 PM, H.J. Lu wrote:
>
>>> Does this result in a visible difference for applications?  If yes,
>>> please file a bug for this and reference it in the ChangeLog and commit
>>> message.
>>
>> It will be very hard to tell since these symbols were exported from libc.so
>> by accident and we only keep them in libc.so for backward binary compatibility.
>> Application can no longer reference them from libc.so.
>
> Ah, it's the old <regexp.h> interface, and the symbols were exported
> deliberately.  We apparently do not have any tests for it.

FYI, there's some reason to believe that this interface never actually
worked correctly -- see the discussion from when it was deprecated.

zw