================
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:
> 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.
Vararg sanitization should now work.
================
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);
----------------
samsonov wrote:
> 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.
Fixed
================
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:
> 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.
Fixed
http://reviews.llvm.org/D5082
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
