This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: New linker option --dsbt_index for C6x; c6x-uclinux fixes
On Wed, 29 Sep 2010, Bernd Schmidt wrote:
> > There are enough differences between TI and GNU command-line arguments
> > that I really don't think compatibility is likely; we should just follow
> > normal GNU style here.
>
> Changed. However, note that there's also the wrapper that converts
> GNU-style command line options into something the TI compiler/linker
> understand. That one also accepts --dsbt_index.
Naturally the wrapper will need updating for the final option names chosen
for the GNU tools, but I don't think that's a problem.
> +
> + void
> + elf32_tic6x_setup (struct bfd_link_info *info,
> + struct elf32_tic6x_params *params)
Should have a comment above the function, since it's not just an
implementation of one of the standard BFD hooks (for which the generic
explanation of the hook suffices in my view without needing a comment for
each target's implementation).
> + @table @gcctabopt
> +
> + @kindex --dsbt_index
> + @item --dsbt_index @var{index}
> + This option sets the DSBT index of the current executable or shared library
> + to @var{index}.
The documentation needs updating for the changed option name.
As I read the patch, the default is 0 if no --dsbt-index option is passed.
I think this needs documenting. Given that the ABI permits the DSBT index
of a shared library to be dynamically assigned, I think the preferred
behavior would be:
* When linking an executable, if no --dsbt-index option is passed then
default to 0 (the executable always uses index 0).
* When linking a shared library, if no --dsbt-index option is passed then
convert static R_C6000_DSBT_INDEX relocations into dynamic relocations.
That can definitely be postponed. But I think the default does need to be
documented now.
> + /* Create our note section. */
> +
> + static void
> + tic6x_after_open (void)
Are you sure about that comment? (And, for that matter, about the
copyright dates on this file.)
> + {
> + char *end;
> + params.dsbt_index = strtol (optarg, &end, 0);
> + if (*end == 0)
> + break;
> + einfo (_("%P%F: invalid --dsbt-index\n"), optarg);
Missing range check here (and no testcases for such a check or for other
invalid arguments). The linker's default check isn't sufficient since
strtol returns long; the long value might be out of range but become in
range once converted to int (0x100000000, for example), so I think you do
need to check the "long" value returned by strtol is in the range 0 to
0x7fff here.
--
Joseph S. Myers
joseph@codesourcery.com