Page MenuHomeFreeBSD

Avoid using abs() on unsigned quantity in powerpc's reloc_jmpslot()
ClosedPublic

Authored by dim on Dec 28 2014, 4:13 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Apr 22, 11:42 PM
Unknown Object (File)
Mar 5 2024, 1:59 PM
Unknown Object (File)
Feb 6 2024, 11:37 PM
Unknown Object (File)
Jan 9 2024, 4:20 PM
Unknown Object (File)
Nov 20 2023, 10:23 PM
Unknown Object (File)
Nov 7 2023, 8:54 AM
Unknown Object (File)
Nov 5 2023, 7:28 AM
Unknown Object (File)
Oct 31 2023, 4:50 AM
Subscribers
None

Details

Summary

During powerpc tests in the clang350-import branch, I noticed this -Werror warning:

libexec/rtld-elf/powerpc/reloc.c:486:6: error: taking the absolute value of unsigned type 'Elf_Addr' (aka 'unsigned int') has no effect [-Werror,-Wabsolute-value]
        if (abs(offset) < 32*1024*1024) {     /* inside 32MB? */
            ^
libexec/rtld-elf/powerpc/reloc.c:486:6: note: remove the call to 'abs' since unsigned values cannot be negative
        if (abs(offset) < 32*1024*1024) {     /* inside 32MB? */
            ^~~
1 error generated.

Just before this line, offset is calculated as the difference between two other Elf_Addr quantities, so the result may well be "negative" in the sense that it has wrapped around UINT_MAX.

So I propose to avoid using abs() altogether, and just test explicitly for the correct ranges. I replaced the decimal constants by hexadecimal ones, since that seemed a little less magical to me. Otherwise the 0xfe000000 constant can be changed to 4064*1024*1024.

Test Plan

Compile, and check if it does not blow up on actual powerpc32 hardware. (Justin, maybe you can try this?)

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

dim retitled this revision from to Avoid using abs() on unsigned quantity in powerpc's reloc_jmpslot().
dim updated this object.
dim edited the test plan for this revision. (Show Details)
dim added reviewers: jhibbits, kib.

I think the patch is obfuscation.

FWIW, why clang issues the warning ? Rules for C types cast definitely require uint to be converted to int there, with preservation of the bit-value. This, combining with the fact that ppc32 uses two-compliment representation of the integers, makes the code well-defined and behaving as the author intended. The 'arguments' about portability do not make any sense in this context, since the code implements C runtime for ppc.

Better approach to satisfy clang non-suitability for system programming, is to explicitely cast abs() argument to int.
Does it fix the warning ?

dim edited edge metadata.

To avoid the warning about abs(), explicitly cast the input argument to int instead.

If casting the input argument explicitly to int makes you happier, then it is fine with me. It will obviously also fix the warning.

kib edited edge metadata.

I am unhappy that perfectly valid code has to be mangled to satisfy the compiler.

This revision is now accepted and ready to land.Dec 28 2014, 6:31 PM
dim updated this revision to Diff 2904.

Closed by commit rS276342 (authored by @dim).