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: [PATCH] MIPS/BFD: Correct/clean up ELF FP attribute warning messages


"Maciej W. Rozycki" <macro@codesourcery.com> writes:
> On Mon, 19 Nov 2012, Richard Sandiford wrote:
>
>> >  While working on MIPS BFD ELF attribute code I have noticed that the 
>> > double- and single-float cases are swapped in warning messages -- the 
>> > former is denoted with "1" and the latter is denoted with "2" and the 
>> > messages interpret them in the opposite manner.  I guess that is an 
>> > unfortunate consequence of using magic numbers everywhere rather than 
>> > meaningful names and then a human error.  I have a follow-up change that 
>> > will correct this approach, however here is an independent essential 
>> > update to put the messages straight.
>> >
>> >  While working on this I have also noticed the messages are inconsistent 
>> > in that some report the name and the attribute of the output file first, 
>> > followed by the offending input file and its attribute, while the others 
>> > use the opposite order.  This only adds to the overall confusion, so I 
>> > have decided to take the opportunity and make the structure of all the 
>> > messages consistent, with the output file first -- this way the left-hand 
>> > side is going to stay the same across all the messages, if multiple are 
>> > produced.
>> 
>> Sorry to be a pain, but I don't think this part is a good change.
>> The current code is consistent in that it always uses the same error
>> message for the same incompatibility, whereas the changed code is
>> consistent in the order of the bfds.  From that point of view the
>> patch is trading one consistency for another.  But really, the fact
>> that we're using the output bfd at all is poor error reporting.
>> The real incompability is between two input objects, so ideally the
>> error message would be too.  (I've hit cases where the input bfd
>> had the ABI I wanted and it's taken me a while to figure out where
>> the "wrong" one came from.)  If that was fixed, I think we'd want
>> the current messages instead of the new ones.
>
>  BFD infers the output ABI from the first input file mapped to output, 
> there's no option to the linker to choose one AFAICT, at least not for the 
> MIPS target and this attribute (but then it'll be up to the individual 
> backend to choose which file name to use in reporting).  I suppose it 
> should be moderately easy to remember the name and use it.  I'll see if I 
> can spend half an hour to implement that.
>
>> As things stand, this patch doubles the number of error messages
>> that need to be translated.
>
>  I don't think we should ever trade user friendliness for easier 
> maintenance unless the effort required turns out prohibitive.  Which is 
> clearly not the case here, so I'm disregarding this argument, sorry.
>
>  And I stand by my view: whether it's the output or the first input file 
> that's reported, it's any later inputs that may be incompatible to it, not 
> the other way round.  I fail to see how e.g.:
>
> Warning: bar.o uses -msingle-float, foo.o uses -mdouble-float
> Warning: foo.o uses -mdouble-float, baz.o uses -mips32r2 -mfp64
>
> is any clearer than:
>
> Warning: foo.o uses -mdouble-float, bar.o uses -msingle-float
> Warning: foo.o uses -mdouble-float, baz.o uses -mips32r2 -mfp64
>
> -- in fact I think the opposite is the case.  With the latter (and the 
> knowledge the ABI chosen for output is reported first) you immediately see 
> what ABI the linker decided to use.  With the former you have to study the 
> messages in detail and investigate actual file names reported to see which 
> one is repeated; and also IMO the wording slightly suggests that the ABI 
> is actually switched as the changes are seen: first it's -msingle-float, 
> then -mdouble-float, and finally -mips32r2 -mfp64.  And last but not least 
> if there is only one message, then you never know which ABI resulted and 
> have to reach for `readelf', etc.
>
>  Of course you can say this may not necessarily matter in practice, but I 
> think in the face of such an ABI incompatibility problem to know whether 
> the ABI chosen by the linker meets one's expectations has a value too.
>
>> A patch to fix the -msingle-float/-mdouble-float mixup without changing
>> the set of error strings is preapproved though.  Thanks for catching it.
>
>  I'll defer any further work here until you have convinced me warning 
> reporting following your point of view provides value beyond one following 
> mine (there's no bonus for keeping status quo).
>
>  Meanwhile I'll look into keeping that first input BFD file name for 
> reporting.  And actually, after a bit of thinking, I believe it may even 
> make sense to say e.g.:
>
> Warning: foobar uses -mdouble-float (set by foo.o), baz.o uses -mips32r2 -mfp64
>
> to emphasize both the ABI chosen for output and where it came from.  What 
> do you think?

OK, go ahead with your patch.

Richard


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