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

Status: RESOLVED FIXED

(Core :: MFBT, defect)

Description

[Sylvestre Ledru [:Sylvestre]]

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 1

[Sylvestre Ledru [:Sylvestre]]

Attachment: Bug 1437482 - gcc 8 has no-sanitize but not the {un,}signed-integer-overflow option

Review commit: https://reviewboard.mozilla.org/r/219436/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/219436/

Comment 2

[Jeff Walden [:Waldo]]

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.

Comment 3

[Sylvestre Ledru [:Sylvestre]]

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

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/219436/diff/1-2/

Comment 4

[Jeff Walden [:Waldo]]

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

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/24117c13a404
gcc 8 has no-sanitize but not the {un,}signed-integer-overflow option r=Waldo

Comment 6

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/24117c13a404