[llvm-bugs] [Bug 46025] New: The implicit-integer-sign-change is pointless for symbolic constants

[via llvm-bugs]

https://bugs.llvm.org/show_bug.cgi?id=46025

            Bug ID: 46025
           Summary: The implicit-integer-sign-change is pointless for
                    symbolic constants
           Product: clang
           Version: 10.0
          Hardware: PC
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P
         Component: C
          Assignee: unassignedclangbugs at nondot.org
          Reporter: bruno at clisp.org
                CC: blitzrakete at gmail.com, dgregor at apple.com,
                    erik.pilkington at gmail.com, llvm-bugs at lists.llvm.org,
                    richard-llvm at metafoo.co.uk

Created attachment 23512
  --> https://bugs.llvm.org/attachment.cgi?id=23512&action=edit
Test case

The -fsanitize=implicit-integer-sign-change checks can be useful in general.
But when involving symbolic constants defined by a library's header file, as I
user I don't control whether they evaluate to 'int' or 'unsigned int'.

Here's a test case:

============================ foo.c ============================
#include <signal.h>

int
main ()
{
  struct sigaction act;
  act.sa_flags = /* SA_NODEFER | SA_ONSTACK | */ SA_RESETHAND;
  return 0;
}
===============================================================

$ clang -Wall foo.c -E | grep sa_flags
    int sa_flags;
  act.sa_flags = 0x80000000;

$ clang -Wall foo.c -fsanitize=implicit-integer-sign-change
$ ./a.out 
foo.c:7:50: runtime error: implicit conversion from type 'unsigned int' of
value 2147483648 (32-bit, unsigned) to type 'int' changed the value to
-2147483648 (32-bit, signed)

So, glibc defines the 'sa_flags' field as being of type 'int' (like POSIX
mandates, see
https://pubs.opengroup.org/onlinepubs/9699919799/functions/sigaction.html).
glibc also defines SA_RESETHAND as 0x80000000. Per ISO C, this constant is of
type 'unsigned int'. The sanitizer then complains about an implicit conversion
from 'unsigned int' to 'int'.

This is not useful, as the programmer did not make a mistake here.

As a programmer, I do not want to add an explicit cast:
  act.sa_flags = (int)(/* SA_NODEFER | SA_ONSTACK | */ SA_RESETHAND);
because generally, such explicit casts introduce bugs when the standards change
or some platform is not 100% standards compliant.

The library authors also surely don't want to write
#define SA_RESETHAND (~0x7fffffff)
because 1) the value is meant to be a single bit, and writing it this way would
obfuscate this bit (mask) aspect, 2) there are hundreds of such bit masks
defined in the libc headers (think of all possible ioctl values).

The library authors also surely don't want to write
#define SA_RESETHAND (int)0x80000000
either, because then SA_RESETHAND would not be usable in a preprocessor
expression (e.g., squid/src/tools.cc has "#if SA_RESETHAND == 0 &&
!_SQUID_WINDOWS_").

So, if clang does not change the implicit-integer-sign-change sanitizer, I can
only recommend to everyone to not use this particular sanitizer.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20200521/74499e28/attachment-0001.html>