[PATCH] Implement nonnull-attribute sanitizer

[Richard Smith]

================
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>();
----------------
samsonov wrote:
> 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?
I think the GCC behavior makes more sense. We should fix the frontend to also understand this, and model it properly in the AST. Our current representation doesn't work.

================
Comment at: tools/clang/lib/CodeGen/CGCall.cpp:2422
@@ +2421,3 @@
+  CodeGenFunction::SanitizerScope SanScope(&CGF);
+  assert(RV.isScalar());
+  llvm::Value *V = RV.getScalarVal();
----------------
samsonov wrote:
> 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);
>                    ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~
> 
I was thinking about this case:

  void f(non_scalar_type x) __attribute__((nonnull));
  void g() { f(x); }

... but it looks like the attribute handling flattens the attribute into a list of parameters when it's applied. That's broken in several ways, alas. For instance, we fail to diagnose this properly:

  template<typename T> __attribute__((nonnull)) void f(T t);
  void g() { f((void*)0); }

... because we can't preserve that we had a parameter-less nonnull attribute until we get to template instantiation.

http://reviews.llvm.org/D5082