Page MenuHomeFreeBSD

Fix new -Werror compiler warnings in binutils

Authored by arichardson on Nov 3 2017, 12:36 PM.



Latest clang git has a warning -Wnull-pointer-arithmetic which will trigger
a -Werror failure. Addition and subtraction from a null pointer is
undefined behaviour and could be optimized into anything. Furthermore,
using the difference between two pointers and casting the result back to a
pointer is not portable since the size of ptrdiff_t does not necessary
have to be the same as size of void* (this happens e.g. on CHERI). Using
intptr_t instead fixes this portability issue and the compiler warning.

Obtained from: CheriBSD

Test Plan

I can now compile binutils with latest clang HEAD.

Diff Detail

rS FreeBSD src repository
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

arichardson created this revision.Nov 3 2017, 12:36 PM

I don't have strong opinions about this - hopefully this code can be fixed by some judicious application of svn rm in the near future...

116 ↗(On Diff #34712)

C doesn't require that int* and void* have the same representation. On some (very) old machines, loads and stores worked only at the granularity of a machine word (typically 16 bits, though sometimes larger). An int* on such a machine was represented as a (word) address, but a char* might be a pair of an address and a byte offset. Accessing a char required a load an a shift or mask.

I think binutils still supports at least one such architecture (though FreeBSD doesn't).

Generally, I think we lower WARNS on contrib code, especially poor-quality GNU code, though when the fix is this simple it might be worth retaining - especially as binutils is due a de-orbit burn soon and is unlikely to ever receive an update..

jhb added a comment.Nov 13 2017, 6:45 PM

I would probably be inclined to just add -Wno-foo to WARNFLAGS.clang with a suitable compiler version check instead if this generates equivalent code for non-CHERI. (For CHERI I think we can just always build ld.bfd as a plain MIPS binary while we still need ld.bfd).

arichardson retitled this revision from Fix new compiler warnings in binutils to Fix new -Werror compiler warnings in binutils.Nov 29 2017, 9:23 PM
arichardson added a reviewer: dim.
arichardson updated this revision to Diff 35980.

removed unnecessary comment

No objection from me.

dim accepted this revision.Dec 24 2017, 4:48 PM

I'm going to commit this now, since we need it in the clang600-import branch.

This revision is now accepted and ready to land.Dec 24 2017, 4:48 PM
This revision was automatically updated to reflect the committed changes.