[RFA:] Fix ld/13990, ARM BFD_ASSERT with --shared and --gc-sections

Richard Sandiford rdsandiford@googlemail.com
Thu Apr 19 21:49:00 GMT 2012


I'll leave the proper review to an ARM maintainer, but:

The refcount change itself looks good -- hope it goes in.  However...

Hans-Peter Nilsson <hans-peter.nilsson@axis.com> writes:
> diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
> index f5b5c4d..5907974 100644
> --- a/bfd/elf32-arm.c
> +++ b/bfd/elf32-arm.c
> @@ -12256,18 +12256,34 @@ elf32_arm_gc_sweep_hook (bfd *                     abfd,
>        if (may_need_local_target_p
>  	  && elf32_arm_get_plt_info (abfd, eh, r_symndx, &root_plt, &arm_plt))
>  	{
> -	  BFD_ASSERT (root_plt->refcount > 0);
> -	  root_plt->refcount -= 1;
> +	  /* If PLT refcount book-keeping is wrong and too low, we'll
> +	     see a zero value (going to -1) for the root PLT reference
> +	     count.  */
> +	  if (root_plt->refcount >= 0)
> +	    {
> +	      BFD_ASSERT (root_plt->refcount != 0);
> +	      root_plt->refcount -= 1;
> +
> +	      /* No use incrementing these members as all their uses are
> +		 dominated by tests on the PLT refcount being
> +		 non-negative.  */
> +	      if (r_type == R_ARM_THM_CALL)
> +		arm_plt->maybe_thumb_refcount--;
> +
> +	      if (r_type == R_ARM_THM_JUMP24
> +		  || r_type == R_ARM_THM_JUMP19)
> +		arm_plt->thumb_refcount--;

...while I take your point, I think it would be better to keep
these last two as they are.  The fact that...

> +	    }
> +	  else
> +	    /* A value of -1 means the symbol has become local, forced
> +	       or seeing a hidden definition.  Any other negative value
> +	       is an error.  */
> +	    BFD_ASSERT (root_plt->refcount == -1);
>  
> +	  /* Use of this member (controlling the IPLT type), is not
> +	     fully dominated by PLT refcount tests so always update it.  */
>  	  if (!call_reloc_p)
>  	    arm_plt->noncall_refcount--;
> -
> -	  if (r_type == R_ARM_THM_CALL)
> -	    arm_plt->maybe_thumb_refcount--;
> -
> -	  if (r_type == R_ARM_THM_JUMP24
> -	      || r_type == R_ARM_THM_JUMP19)
> -	    arm_plt->thumb_refcount--;

...noncall_refcount is useful for IPLTs shows that the others may
become useful in future too.  Whoever uses them in future may not
know that this GC code exists.

This block is really supposed to be undoing the counting in
check_relocs.  Information for removed relocs shouldn't be left lying
around to trip up later code.  Your refcount fix sticks to that
principle because for refcount < 0 the precise count has already,
deliberately, been dropped.  The other counts are still meaningful
though, whether or not they are used for refcount < 0 at present.

Thanks,
Richard



More information about the Binutils mailing list