Re: [PATCH] PR ld/21903: Treat common symbol as undefined for --no-define-common

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Mon, Aug 7, 2017 at 4:17 AM, Alan Modra <amodra@gmail.com> wrote:
> On Sun, Aug 06, 2017 at 07:53:33AM -0700, H.J. Lu wrote:
>> On Fri, Aug 4, 2017 at 5:22 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> > When --no-define-common is used to build shared library, treat common
>> > symbol as undefined so that common symbols that are referenced from a
>> > shared library to be assigned addresses only in the main program.  This
>> > eliminates the unused duplicate space in the shared library, and also
>> > prevents any possible confusion over resolving to the wrong duplicate
>> > when there are many dynamic modules with specialized search paths for
>> > runtime symbol resolution.
>> >
>> > --no-define-common is only allowed when building a shared library.
>>
>> > OK for master?
>>
>> I am going to check it in.
>
> Why the hurry?
>
>> > --- a/bfd/elflink.c
>> > +++ b/bfd/elflink.c
>> > @@ -4250,6 +4250,11 @@ error_free_dyn:
>> >
>> >        override = FALSE;
>> >
>> > +      /* Treat common symbol as undefined for --no-define-common.  */
>> > +      if (isym->st_shndx == SHN_COMMON
>> > +         && info->inhibit_common_definition)
>> > +       isym->st_shndx = SHN_UNDEF;
>> > +
>> >        flags = BSF_NO_FLAGS;
>> >        sec = NULL;
>> >        value = isym->st_value;
>
> Surely this should be put a little later.
>
>       common = bed->common_definition (isym);
>       if (common && info->inhibit_common_definition)
>         {
>           isym->st_shndx = SHN_UNDEF;
>           common = FALSE;
>         }
>
> The testcase wasn't checked very well either.  It fails on arc, cris,
> and frv.  Why do you need _start and main, and wouldn't it be better
> to put the ".dc.a foo" in .data?
>

I am checking in this patch,

-- 
H.J.

From 70700c92be133d5b144a34c3a3bc11401bc746f4 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Mon, 7 Aug 2017 06:03:12 -0700
Subject: [PATCH] Move common symbol check after bed->common_definition

bfd/

	* elflink.c (elf_link_add_object_symbols): Move common symbol
	check after bed->common_definition.

ld/

	* testsuite/ld-elf/pr21903.s (start): Removed.
	(_start): Likewise.
	(__start): Likewise.
	(main): Likewise.
	(bar): New.
---
 bfd/elflink.c                 | 11 ++++++-----
 ld/testsuite/ld-elf/pr21903.s | 12 ++++--------
 2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/bfd/elflink.c b/bfd/elflink.c
index 0cc5f871db..58f1a4779d 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -4250,15 +4250,16 @@ error_free_dyn:
 
       override = FALSE;
 
-      /* Treat common symbol as undefined for --no-define-common.  */
-      if (isym->st_shndx == SHN_COMMON
-	  && info->inhibit_common_definition)
-	isym->st_shndx = SHN_UNDEF;
-
       flags = BSF_NO_FLAGS;
       sec = NULL;
       value = isym->st_value;
       common = bed->common_definition (isym);
+      if (common && info->inhibit_common_definition)
+	{
+	  /* Treat common symbol as undefined for --no-define-common.  */
+	  isym->st_shndx = SHN_UNDEF;
+	  common = FALSE;
+	}
       discarded = FALSE;
 
       bind = ELF_ST_BIND (isym->st_info);
diff --git a/ld/testsuite/ld-elf/pr21903.s b/ld/testsuite/ld-elf/pr21903.s
index 9dbf96c62e..ce26b33f02 100644
--- a/ld/testsuite/ld-elf/pr21903.s
+++ b/ld/testsuite/ld-elf/pr21903.s
@@ -1,12 +1,8 @@
 	.text
-	.global start	/* Used by SH targets.  */
-start:
-	.global _start
-_start:
-	.global __start
-__start:
-	.global main	/* Used by HPPA targets.  */
-main:
+	.global bar
+bar:
+	.byte 0
+	.data
 	.dc.a foo
 	.ifdef	HPUX
 foo	.comm	4
-- 
2.13.3