Re: Define __start/__stop symbols when there is only a dynamic def

[Michael Matz <matz at suse dot de>]

Hi,

On Tue, 30 Jan 2018, Alan Modra wrote:

> On Mon, Jan 29, 2018 at 03:36:05PM -0800, H.J. Lu wrote:
> > There is a reason why I have been keeping asking for a testcase.
> > I will update my patch with the above testcase.
> 
> The last few emails in this thread illustrate just why changes based
> on testcases can be suspect.

Only when it's the wrong testcase ;-)

> I don't believe that ld 2.28 or earlier defined dynamic __start and
> __stop symbols without a dynamic reference.

If you include dynamic defs (from linked shared libs) as dynamic refs, 
then you're right.  Old binutils did define dynamic __start in app when 
there was a dynamic def in some linked shared lib.  
I.e. the situation of the test in 
  https://sourceware.org/ml/binutils/2018-01/msg00432.html
That worked in ld 2.26 and is the situation pacemaker wants.

> In fact, you need to use
> -E or some other way of making symbols dynamic in an executable to
> support dlsym on *normal* symbols.  I'm going to commit the extra test
> I posted earlier without -E, plus this patch.
> 
> 	* elflink.c (bfd_elf_define_start_stop): Make __start and __stop
> 	symbols dynamic.
> 
> diff --git a/bfd/elflink.c b/bfd/elflink.c
> index e81f6c6..f1ec880 100644
> --- a/bfd/elflink.c
> +++ b/bfd/elflink.c
> @@ -14354,8 +14354,13 @@ bfd_elf_define_start_stop (struct bfd_link_info *info,
>  	  bed = get_elf_backend_data (info->output_bfd);
>  	  (*bed->elf_backend_hide_symbol) (info, h, TRUE);
>  	}
> -      else if (ELF_ST_VISIBILITY (h->other) == STV_DEFAULT)
> -	h->other = (h->other & ~ELF_ST_VISIBILITY (-1)) | STV_PROTECTED;
> +      else
> +	{
> +	  if (ELF_ST_VISIBILITY (h->other) == STV_DEFAULT)
> +	    h->other = (h->other & ~ELF_ST_VISIBILITY (-1)) | STV_PROTECTED;
> +	  if (h->ref_dynamic || h->def_dynamic)
> +	    bfd_elf_link_record_dynamic_symbol (info, h);
> +	}

This doesn't work as you seemingly intended (testcase above fails).
See the larger context:

  if (h != NULL
      && (h->root.type == bfd_link_hash_undefined
    ...
      h->def_dynamic = 0;
    ...
         if (h->ref_dynamic || h->def_dynamic)
            bfd_elf_link_record_dynamic_symbol (info, h);

It does work after moving the overriding of def_dynamic downwards or 
remembering the old status:

diff --git a/bfd/elflink.c b/bfd/elflink.c
index f1ec880..3fe4555 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -14340,6 +14340,7 @@ bfd_elf_define_start_stop (struct bfd_link_info 
*info,
          || h->root.type == bfd_link_hash_undefweak
          || ((h->ref_regular || h->def_dynamic) && !h->def_regular)))
     {
+      bfd_boolean was_dynamic = h->ref_dynamic || h->def_dynamic;
       h->root.type = bfd_link_hash_defined;
       h->root.u.def.section = sec;
       h->root.u.def.value = 0;
@@ -14358,7 +14359,7 @@ bfd_elf_define_start_stop (struct bfd_link_info 
*info,
        {
          if (ELF_ST_VISIBILITY (h->other) == STV_DEFAULT)
            h->other = (h->other & ~ELF_ST_VISIBILITY (-1)) | STV_PROTECTED;
-         if (h->ref_dynamic || h->def_dynamic)
+         if (was_dynamic)
            bfd_elf_link_record_dynamic_symbol (info, h);
        }
       return &h->root;


Ciao,
Michael.