[PATCH] Implement nonnull-attribute sanitizer

[Alexey Samsonov]

================
Comment at: projects/compiler-rt/lib/ubsan/ubsan_handlers.cc:355
@@ +354,3 @@
+
+static void handleNonnullArg(NonNullArgData *Data, ValueHandle ArgIndex,
+                             ReportOptions Opts) {
----------------
rsmith wrote:
> Please consistently use either `NonNull` or `Nonnull`.
Done

================
Comment at: tools/clang/lib/CodeGen/CGCall.cpp:2414-2415
@@ +2413,4 @@
+                                unsigned ParmNum) {
+  if (!CGF.SanOpts->NonnullAttribute || !FD || ParmNum >= FD->getNumParams())
+    return;
+  const NonNullAttr *NNAtt = FD->getAttr<NonNullAttr>();
----------------
rsmith wrote:
> What should happen here:
> 
>   __attribute__((nonnull)) void f(const char *, ...);
>   int main() { void *p = 0; f("%s", p); }
> 
> (I have no idea if the attribute applies in this case.)
Neither am I :( GCC docs say that "If no argument index list is given to the nonnull attribute, all pointer arguments are marked as non-null", but it's not clear if it refers to call arguments, or formal parameters. What is worse, GCC and Clang seem to disagree on this:
  $ cat tmp/ubsan/bad.cpp
  __attribute__((nonnull)) void *foo2(int *a, ...);
  
  void bar() { foo2(0, (void*)0); }
  $ g++ -Wnonnull -Wall tmp/ubsan/bad.cpp -c -o /dev/null
  tmp/ubsan/bad.cpp: In function ‘void bar()’:
  tmp/ubsan/bad.cpp:3:30: warning: null argument where non-null required (argument 1) [-Wnonnull]
  void bar() { foo2(0, (void*)0); }
                              ^
  tmp/ubsan/bad.cpp:3:30: warning: null argument where non-null required (argument 2) [-Wnonnull]
  $ ./bin/clang++ -Wnonnull -Wall tmp/ubsan/bad.cpp -c -o /dev/null
  tmp/ubsan/bad.cpp:3:30: warning: null passed to a callee which requires a non-null argument [-Wnonnull]
  void bar() { foo2(0, (void*)0); }
                    ~          ^
  1 warning generated.

Sigh. Should Clang strive for GCC-compatible behavior, or you have the idea of which behavior is "more predictable" in this case?

================
Comment at: tools/clang/lib/CodeGen/CGCall.cpp:2416-2420
@@ +2415,7 @@
+    return;
+  const NonNullAttr *NNAtt = FD->getAttr<NonNullAttr>();
+  auto PVD = FD->getParamDecl(ParmNum);
+  if (!(NNAtt && NNAtt->isNonNull(PVD->getFunctionScopeIndex())) &&
+      !PVD->hasAttr<NonNullAttr>())
+    return;
+  CodeGenFunction::SanitizerScope SanScope(&CGF);
----------------
rsmith wrote:
> Can a function have multiple `__attribute__((nonnull(N)))`s on it?
Nice catch! Both Clang and GCC accept this code and seem to calculate union of all arguments passed into nonnull().

  $ cat tmp/ubsan/bad.cpp
  __attribute__((nonnull(1)))
  __attribute__((nonnull(2)))
  void foo(int *a, int *a2);
  void bar() { foo(0, 0); }
  $ g++ -Wnonnull -Wall tmp/ubsan/bad.cpp -c -o /dev/null
  tmp/ubsan/bad.cpp: In function ‘void bar()’:
  tmp/ubsan/bad.cpp:9:11: warning: null argument where non-null required (argument 1) [-Wnonnull]
     foo(0, 0);
             ^
  tmp/ubsan/bad.cpp:9:11: warning: null argument where non-null required (argument 2) [-Wnonnull]
  $ ./bin/clang++ -Wnonnull -Wall tmp/ubsan/bad.cpp -c -o /dev/null
  tmp/ubsan/bad.cpp:9:11: warning: null passed to a callee which requires a non-null argument [-Wnonnull]
    foo(0, 0);
           ~^
  tmp/ubsan/bad.cpp:9:11: warning: null passed to a callee which requires a non-null argument [-Wnonnull]
    foo(0, 0);
        ~   ^
  2 warnings generated.

The code as written won't handle this case properly (the last nonnull() attribute in function declaration will win). Sadly, there is the same problem already present in EmitFunctionProlog() function. I will work on a fix in a separate change.

================
Comment at: tools/clang/lib/CodeGen/CGCall.cpp:2422
@@ +2421,3 @@
+  CodeGenFunction::SanitizerScope SanScope(&CGF);
+  assert(RV.isScalar());
+  llvm::Value *V = RV.getScalarVal();
----------------
rsmith wrote:
> What guarantees this? I don't see where you're checking that the parameter is of a pointer type.
This seems to be handled by Sema:

  tmp/ubsan/bad.cpp:1:33: warning: 'nonnull' attribute only applies to pointer arguments [-Wignored-attributes]
  void foo(int *a, __attribute__((nonnull)) int b);
                   ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~

http://reviews.llvm.org/D5082