This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Ping: [PATCH] objcopy: fix 32-bit build


>>> On 29.01.18 at 00:04, <amodra@gmail.com> wrote:
> On Wed, Jan 24, 2018 at 12:53:17AM -0700, Jan Beulich wrote:
>> >>> On 10.01.18 at 10:32, <JBeulich@suse.com> wrote:
>> > --- a/binutils/objcopy.c
>> > +++ b/binutils/objcopy.c
>> > @@ -2064,7 +2064,7 @@ merge_gnu_build_notes (bfd * abfd, asect
>> >  	     For now though, since v1 and v2 was not intended to
>> >  	     handle gaps, we chose an artificially large end
>> >  	     address.  */
>> > -	  end = (bfd_vma) 0x7ffffffffffffffUL;
>> > +	  end = ~((bfd_vma)1 << 63);
>> >  	  break;
>> >  	  
>> >  	case 8:
>> > @@ -2083,7 +2083,7 @@ merge_gnu_build_notes (bfd * abfd, asect
>> >  		 For now though, since v1 and v2 was not intended to
>> >  		 handle gaps, we chose an artificially large end
>> >  		 address.  */
>> > -	      end = (bfd_vma) 0x7ffffffffffffffUL;
>> > +	      end = ~((bfd_vma)1 << 63);
>> >  	    }
>> >  	  break;
> 
> Why not use "(bfd_vma) -1"?

Well - why was the original value not 0xffffffffffffffff? (I notice I've
actually submitted a stale patch - the proper value above ought to
be ~((bfd_vma)0x1f << 59) if we want to retain the current value,
as I had noticed only later, and then I apparently forgot to refresh
the patch. I did raise this point in the description though.)

>  Won't this complain about shift count
> exceeding width of type when bfd_vma is 32-bit?

Yes, I expect that. Hence me saying "I do assume this isn't sufficient
to address the non-64-bit-BFD case as well" in the description.

>  (And if you don't get
> a warning, there is at least a possibility of an all-ones bit pattern
> when 32-bit while 64-bit will have the MSB zero.  A difference I don't
> like.)

Ideally the original author would fix this, as I can second-guess what
the intention was with the value chosen. The change here is what I
minimally (continue to) need to be able to build and test on a 32-bit
host. Once I build 2.30 for a wider range of environments, I'm sure
I'll have to extend the fix/workaround. (I would really have hoped
for 2.30 to ship without such a build issue, or at the very least to
have a clear understanding of the original intentions here by now,
so I know how to "properly" work around this.)

Jan


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]