Closed Bug 1437482 Opened 2 years ago Closed 2 years ago

gcc-8: MathAlgorithms.h: 'unsigned-integer-overflow' attribute directive ignored [-Werror=attributes]

Categories

(Core :: MFBT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: Sylvestre, Assigned: Sylvestre)

References

Details

Attachments

(1 file)

Elapsed: 0.24s; From ../../dist/idl: Kept 936 existing; Added/updated 0; Removed 0 files and 0 directories.
 In file included from /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/mozilla/FloatingPoint.h:15,
                  from /root/firefox-gcc-last/mfbt/decimal/moz-decimal-utils.h:16,
                  from /root/firefox-gcc-last/mfbt/decimal/Decimal.cpp:32:
 /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/mozilla/MathAlgorithms.h:568:58: error: 'unsigned-integer-overflow' attribute directive ignored [-Werror=attributes]
    static constexpr SignedType compute(UnsignedType aValue)
                                                           ^


gcc 8 has no_sanitize but not the unsigned-integer-overflow/signed-integer-overflow
Comment on attachment 8950168 [details]
Bug 1437482 - gcc 8 has no-sanitize but not the {un,}signed-integer-overflow option

https://reviewboard.mozilla.org/r/219436/#review225346

::: mfbt/Attributes.h:280
(Diff revision 1)
> -#  define MOZ_NO_SANITIZE_UNSIGNED_OVERFLOW __attribute__((no_sanitize("unsigned-integer-overflow")))
> +#  if __has_attribute(__no_sanitize_unsigned_integer_overflow__)
> +#    define MOZ_NO_SANITIZE_UNSIGNED_OVERFLOW __attribute__((no_sanitize("unsigned-integer-overflow")))
> +#  else
> +/**
> + * gcc 8 has no-sanitize but not the unsigned-integer-overflow option
> + */

If MOZ_HAVE_NO_SANITIZE_ATTR doesn't match what these macros need, we shouldn't bother with it, and we should just use the exact necessary logic at the two definition-sites.  In other words the #define MOZ_HAVE_NO_SANITIZE_ATTR goo above should be eliminated and the relevant testing inlined into these two sites.

That said, this change doesn't work for me.

    [jwalden@find-waldo-now tmp]$ cat t.cpp 
    #if __has_attribute(__no_sanitize_unsigned_integer_overflow__)
    #error "have it"
    #endif
    [jwalden@find-waldo-now tmp]$ clang++-tip -fsanitize=unsigned-integer-overflow t.cpp 2>&1 | grep 'have it'
    [jwalden@find-waldo-now tmp]$ clang++-tip -fsanitize=integer-overflow t.cpp 2>&1 | grep 'have it'
    [jwalden@find-waldo-now tmp]$ clang++-tip t.cpp 2>&1 | grep 'have it'
    [jwalden@find-waldo-now tmp]$ clang++-tip --version
    clang version 6.0.0 (trunk 317840)
    Target: x86_64-unknown-linux-gnu
    Thread model: posix
    InstalledDir: /home/jwalden/.programs/new-compilers

Internet searches don't find the attribute you've tested with, and I can't find anything in clang docs that can be used to feature-test specifically for this.  https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#disabling-instrumentation-with-attribute-no-sanitize-undefined recommends a very high-level `#ifdef __clang__` check, so I would probably write it, regrettably, using some flavor of that.
Attachment #8950168 - Flags: review?(jwalden+bmo) → review-
Comment on attachment 8950168 [details]
Bug 1437482 - gcc 8 has no-sanitize but not the {un,}signed-integer-overflow option

https://reviewboard.mozilla.org/r/219436/#review226710
Attachment #8950168 - Flags: review?(jwalden+bmo) → review+
Component: Build Config → MFBT
Pushed by sledru@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/24117c13a404
gcc 8 has no-sanitize but not the {un,}signed-integer-overflow option r=Waldo
https://hg.mozilla.org/mozilla-central/rev/24117c13a404
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in before you can comment on or make changes to this bug.