Home Original page

[Patch, microblaze, bfd/gas/include] Add MicroBlaze TLS Support

David Holsgrove david.holsgrove@xilinx.com
Thu Nov 29 07:26:00 GMT 2012 
Hi Michael,

> -----Original Message-----
> From: Michael Eager [mailto:eager@eagerm.com]
> Sent: Thursday, 29 November 2012 1:10 pm
> To: David Holsgrove
> Cc: binutils@sourceware.org; John Williams; Vinod Kathail; Tom Shui; Vidhumouli
> Hunsigida; Nagaraju Mekala; Edgar E. Iglesias
> Subject: Re: [Patch, microblaze, bfd/gas/include] Add MicroBlaze TLS Support
> 
> On 11/22/2012 04:06 AM, David Holsgrove wrote:
> >
> > Add support for handling TLS symbol suffixes and generating
> > TLS relocs for General Dynamic and Local Dynamic models.
> >
> > Check in generated files that have changed as a result of new
> > TLS Relocs
> 
> +check_unique_offset(bfd_vma offset, int tls_type, bfd_vma v)
> +{
> +  static bfd_vma offsets[1000];
> +  static bfd_vma values[1000];
> 
> Why 1000?
> 

This is inherited code from out of tree sources, and it would appear that this function
check_unique_offset is dead debug code, which I missed as it simply returns every
time it is called without doing anything;

static void
check_unique_offset(bfd_vma offset, int tls_type, bfd_vma v)
{
  static bfd_vma offsets[1000];
  static bfd_vma values[1000];

  static int noffsets = 0;
  int scan1;

  return;

> +  for (scan1 = 0; scan1 < noffsets; scan1++)
> +  {
> +	if (offsets[scan1] == offset)
> +	  {
> +		fprintf (stderr, "Duplicate Offset: %lx type: %x Old: %lx New: %lx
> \n",
> +                     offset, tls_type, values[scan1], v);
> +	  }
> +  }
> +  fprintf (stderr, "%d Registered Offset: %lx Value: %lx type: %x\n",
> +                   noffsets, offset, v, tls_type);
> 
> Please fix indent.  There are several places where the indent is incorrect.
> 
> Is this debugging code?  Are the messages errors which you expect to
> generate?  If yes, use bfd_perror().
> 

So I'll remove this function as unnecessary.

> +          if ( IS_TLS_LD (tls_type) )
> 
> No spaces after/before parens surrounding expression.  There are other places
> which have extra spaces.
> 
> +            abort();
> 
> Space before paren in function calll.  There are other places which need spaces
> before parens.

Thanks, will fix in the next version.

> 
> +                      check_unique_offset( off, tls_type, 0xDDDDDDDD);
> 
> What is 0xDDDDDDDD?
> 

All calls to check_unique_offset can be removed.

> +      size *= (sizeof (*local_got_refcounts)
> +               + sizeof (*local_got_tls_masks));
> 
> One line.
> 

Thanks again, will forward v2 of patch shortly.
David
> 
> --
> Michael Eager	 eager@eagercon.com
> 1960 Park Blvd., Palo Alto, CA 94306  650-325-8077
> 
>
More information about the Binutils mailing list