Re: -fsanitize=undefined errors

[Jim Meyering]

On Wed, Dec 3, 2014 at 1:20 PM, Pádraig Brady <address@hidden> wrote:
> On 03/12/14 18:18, Eric Blake wrote:
>> [adding the public list]
>>
>> On 12/03/2014 10:49 AM, Dingbao Xie wrote:
>>> Dear coreutils maintainer,
>>> I'm a visiting phd student at UC davis and currently works
>>> on a project aiming to detect undefined behaviors.
>>> clang has a sanitizer called ubsan which can identify certain
>>> undefined behaviors at runtime. I tried to do experiment on coreutils
>>> and found a shift out of bounds in a c file.
>>> Below is the detail information:
>>>
>>> xdb@xie:$ ./arch _<<<\x00
>>> ../../lib/quotearg.c:554:45: runtime error: left shift of 1 by 31 places
>>> cannot be represented in type 'int'
>>> ./arch: extra operand `_'
>>> Try `./arch --help' for more information
>>>
>>>
>>> Could you please confirm that whether it is a serious problem or not?
>>> BTW, I built coreutil-6.11 with clang (-fsanitize=undefined).
>>
>> Thanks for the report.  This issue has already been fixed upstream;
>> http://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=831b84, and is
>> already in newer versions of coreutils.  You would be much better off
>> doing your testing on the latest coreutils.git instead of old versions.
>
> On a related note I just noticed a false positive -fsanitize=undefined
> failure with gcc 4.9.2 which the attached should avoid.
> An alternative to the attached approach might be to avoid unaligned
> accesses altogether by getting readisaac() to memcpy only unaligned slop?

Thanks for addressing that.
Regarding the patch,

+/* If we can make unaligned accesses then don't have
+   -fsanitize=undefined warn about it.  */
+#undef ATTRIBUTE_NO_WARN_SANITIZE_UNDEFINED
+#if !_STRING_ARCH_unaligned ||__GNUC__ < 4 \
+    ||(__GNUC__ == 4 && __GNUC_MINOR__ < 9)
+# define ATTRIBUTE_NO_WARN_SANITIZE_UNDEFINED /* empty */
+#else
+# define ATTRIBUTE_NO_WARN_SANITIZE_UNDEFINED __attribute__ \
+  ((__no_sanitize_undefined__))
+#endif

Please put a space after each "||" operator.
Also, it's more readable to put the __attribute__ keyword on the
same line as it's ((...)) arguments:

  # define ATTRIBUTE_NO_WARN_SANITIZE_UNDEFINED \
    __attribute__ ((__no_sanitize_undefined__))