[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