================
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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
