[PATCH][m68k] Support generation of multiple GOTs

[Maxim Kuvyrkov]

Andreas Schwab wrote:
> Maxim Kuvyrkov <maxim@codesourcery.com> writes:
> 
>> +  /* List of GOT entries for this symbol.  This list is build during
>> +     offset finalization and is used within elf_m68k_finish_dynamic_symbol ()
>> +     to traverse all GOT entries for a particular symbol.
> 
> Please don't add parens when referring to functions in comments.  There
> are several occurrences throughout the patch.

Fixed.

> 
>> +  if ((got->rel_8o_n_entries
>> +       > ELF_M68K_REL_8O_MAX_N_ENTRIES_IN_GOT (info))
>> +      || (got->rel_8o_16o_n_entries
>> +	  > ELF_M68K_REL_8O_16O_MAX_N_ENTRIES_IN_GOT (info)))
>> +    /* This BFD has too many relocation.  */
>> +    {
>> +      char *rel;
>> +      int num;
>> +
>> +      if (got->rel_8o_n_entries
>> +	  > ELF_M68K_REL_8O_MAX_N_ENTRIES_IN_GOT (info))
>> +	{
>> +	  rel = "R_68K_GOT8O";
>> +	  num = ELF_M68K_REL_8O_MAX_N_ENTRIES_IN_GOT (info);
>> +	}
>> +      else
>> +	{
>> +	  rel = "R_68K_GOT8O and R_68K_GOT16O";
>> +	  num = ELF_M68K_REL_8O_16O_MAX_N_ENTRIES_IN_GOT (info);
>> +	}
>> +
>> +      (*_bfd_error_handler) (_("%B: GOT overflow: "
>> +			       "Number of %s relocations > %d"),
>> +			     abfd, rel, num);
> 
> This may be difficult to translate to some languages due to the
> substituted sentence fragment.  It's probably better to move the error
> handler call into the condition above to avoid the fragment.

Fixed.

> 
>> +AC_ARG_ENABLE([got],
>> +AS_HELP_STRING([--enable-got=<type>],
>> +               [GOT handling scheme (target, single, negative, multigot)]),
>> +[case "${enableval}" in
>> +  target | single | negative | multigot)  got_handling=$enableval ;;
>> +  *)  AC_ERROR(bad value ${enableval} for --enable-got option) ;;
>> +esac],
>> +[got_handling=target])
> 
> I don't think we need that configure option.  Passing the right option
> by default via gcc should be enough.

As I said, the choice of the default setting is dependent on the glibc 
version installed on a system.  This is due to glibc's dynamic linker 
making assumptions about GOT pointer.  I don't have strong feelings 
about this option, so I'm ok with removing it if you or Ben think it's 
the right thing to do.

> 
>> +@ifset M68K
>> +@subsection Options specific to Motorola 68K target
>> +
>> +@c man begin OPTIONS
>> +
>> +The following options are supported to control handling of GOT generation
>> +when linking for 68K targets.
>> +
>> +@table @gcctabopt
>> +
>> +@kindex --got
>> +@item --got=@var{type}
>> +This option tells the linker which GOT generation scheme to use.
>> +@var{type} should be one of @samp{single}, @samp{negative},
>> +@samp{multigot} or @samp{target}.  For more information refer to the
>> +Info entry for @file{ld}.
> 
> The last sentence should use @xref, doesn't it?

This is a man page referring to info document.  I don't think one can 
put an xref here.


Ben, does the patch look to you?


Thanks,

Maxim
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: fsf-multigot.ChangeLog
URL: <https://sourceware.org/pipermail/binutils/attachments/20080417/b0faade2/attachment.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: fsf-multigot.patch
URL: <https://sourceware.org/pipermail/binutils/attachments/20080417/b0faade2/attachment-0001.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: got-tests.tar.bz2
URL: <https://sourceware.org/pipermail/binutils/attachments/20080417/b0faade2/attachment-0002.ksh>