Re: [PATCH][M68K] Binutils support for TLS

[Maxim Kuvyrkov <maxim at codesourcery dot com>]

Andreas Schwab wrote:
> Maxim Kuvyrkov <maxim@codesourcery.com> writes:
>
> > +  /* Check if we should switch to negative range of the offsets. */
> > +  if (arg->offset1[got_offset_size] + entry_size
> > +      > arg->offset2[got_offset_size])
> > +    {
> > +      /* Verify that this is the only switch to negative range for
> > +	 got_offset_size.  */
> > +      BFD_ASSERT (arg->use_neg_got_offsets_p
> > +		  && (arg->offset2[got_offset_size]
> > +		      != arg->offset2[- got_offset_size - 1]));
> > +
> > +      /* Switch.  */
> > +      arg->offset1[got_offset_size] = arg->offset1[-got_offset_size - 1];
> > +      arg->offset2[got_offset_size] = arg->offset2[-got_offset_size - 1];
>
> I'm getting an assertion failures when running the linker tests.  You
> need to initialize arg_.use_neg_got_offsets_p before calling
> hash_traverse in elf_m68k_finalize_got_offsets.

Andreas, thanks for looking at the TLS patches.

It is better to just remove this field as it is not very useful now.  To 
keep the assert and make it reliable, it is enough to set 
arg->offset2[-N] to the values of arg->offset2[N] when negative offsets 
are not used.

The attached patch does the above and should fix the problem (I say 
"should" because I couldn't reproduce the bug on my system ;) ).

> Furthermore, the
> underlying type of an enum may be unsigned, so you need to cast to int
> before negating it.

Dully fixed by the attached patch.

--
Maxim

--- bfd/elf32-m68k.c
+++ bfd/elf32-m68k.c
@@ -1968,9 +1968,6 @@ struct elf_m68k_finalize_got_offsets_arg
   bfd_vma *offset1;
   bfd_vma *offset2;
 
-  /* Should we use negative (relative to GP) offsets for GOT entries.  */
-  bfd_boolean use_neg_got_offsets_p;
-
   /* Mapping from global symndx to global symbols.
      This is used to build lists of got entries for global symbols.  */
   struct elf_m68k_link_hash_entry **symndx2h;
@@ -2007,14 +2004,15 @@ elf_m68k_finalize_got_offsets_1 (void **
       > arg->offset2[got_offset_size])
     {
       /* Verify that this is the only switch to negative range for
-	 got_offset_size.  */
-      BFD_ASSERT (arg->use_neg_got_offsets_p
-		  && (arg->offset2[got_offset_size]
-		      != arg->offset2[- got_offset_size - 1]));
+	 got_offset_size.  If this assertion fails, then we've miscalculated
+	 range for got_offset_size entries in
+	 elf_m68k_finalize_got_offsets.  */
+      BFD_ASSERT (arg->offset2[got_offset_size]
+		  != arg->offset2[-(int) got_offset_size - 1]);
 
       /* Switch.  */
-      arg->offset1[got_offset_size] = arg->offset1[-got_offset_size - 1];
-      arg->offset2[got_offset_size] = arg->offset2[-got_offset_size - 1];
+      arg->offset1[got_offset_size] = arg->offset1[-(int) got_offset_size - 1];
+      arg->offset2[got_offset_size] = arg->offset2[-(int) got_offset_size - 1];
 
       /* Verify that now we have enough room for the entry.  */
       BFD_ASSERT (arg->offset1[got_offset_size] + entry_size
@@ -2129,6 +2127,15 @@ elf_m68k_finalize_got_offsets (struct el
       start_offset = arg_.offset2[i];
     }
 
+  if (!use_neg_got_offsets_p)
+    /* Make sure that if we try to switch to negative offsets in
+       elf_m68k_finalize_got_offsets_1, the assert therein will catch
+       the bug.  */
+    {
+      for (i = (int) R_8; i < (int) R_32; ++i)
+	arg_.offset2[-i - 1] = arg_.offset2[i];
+    }
+
   /* Setup got->offset.  offset1[R_8] is either in the middle or at the
      beginning of GOT depending on use_neg_got_offsets_p.  */
   got->offset = arg_.offset1[R_8];