Re: [PATCH] PR ld/19572: -Ttext-segment accepts out of range value

[Alan Modra <amodra at gmail dot com>]

On Mon, Feb 08, 2016 at 10:08:00AM -0800, H.J. Lu wrote:
> On Sat, Feb 6, 2016 at 7:52 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> > On Sat, Feb 6, 2016 at 5:59 AM, Alan Modra <amodra@gmail.com> wrote:
> >> On Fri, Feb 05, 2016 at 01:16:58PM -0800, H.J. Lu wrote:
> >>> The address for -Tbss, -Tdata, -Ttext, -Ttext-segment, -Trodata-segment
> >>> and -Tldata-segment shouldn't be bigger than the address space.
> >>
> >> Does it really matter if someone specifies an address that wraps?
> >>
> >> If it does, then it opens up other questions like:  Is the 32-bit
> >> address range 0 to 4G-1 or -2G to 2G-1?  We have ELF targets (see
> >> bed->sign_extend_vma) where the latter might be more natural.
> >
> > When address passed to -Txxxx overflows, bfd_scan_vma returns:
> >
> >  if (overflow)
> >     value = ~ (bfd_vma) 0;
> >
> > My patch just checks this condition and issues an error.  Is there any
> > anything wrong with it?

Your patch does not just do that.

> Unless I am shown "~ (bfd_vma) 0" is a valid address for -Txxx before
> this Friday, I will check it in on Friday.

The discussion in this thread has not been about the change to
lexsup.c but rather the change to ldlang.c, which is controversial for
x86_64 and wrong for some other targets.  You have not answered these
concerns.

Also, even just the lexsup.c change is ugly.  Why put the check after
a comment and statement related to the for-loop, instead of
immediately after the existing check for a valid hex number?  What's
more it would be nicer to check errno and modify the fallback code in
bfd_scan_vma, used when strtoull in unavailable, to set errno.  That
way your error message about overflow would be true in all cases.

-- 
Alan Modra
Australia Development Lab, IBM