[PATCH] Make analyzer check all argument for nonnull attribute

[Daniel Fahlgren]

Hi,

This patch fixes the fixme in NonNullParamChecker about multiple
attributes. It does that by iterating over all parameters for all
nonnull-attributes the function has.

For a function with a lot of parameters and many attributes that might
seem a bit wasteful. However, that is likely not a problem on real code.
I also ran some synthetic benchmarks on a worst case scenario (25
parameters and 25 nonnull attributes) and couldn't find any noticeable
performance issue.

Any comments or feedback?

Best regards,
Daniel Fahlgren

Index: test/Analysis/nonnull.m
===================================================================
--- test/Analysis/nonnull.m	(revision 218377)
+++ test/Analysis/nonnull.m	(working copy)
@@ -76,6 +76,104 @@
   rdar16153464_check(0); // expected-warning{{nonnull}}
 }
 
+// Multiple attributes, the basic case
+void multipleAttributes_1(char *p, char *q) __attribute((nonnull(1))) __attribute((nonnull(2)));
+
+void testMultiple_1(void) {
+  char c;
+  multipleAttributes_1(&c, &c); // no-warning
+}
+
+void testMultiple_2(void) {
+  char c;
+  multipleAttributes_1(0, &c); // expected-warning{{nonnull}}
+}
+
+void testMultiple_3(void) {
+  char c;
+  multipleAttributes_1(&c, 0); // expected-warning{{nonnull}}
+}
+
+void testMultiple_4(void) {
+  multipleAttributes_1(0, 0);// expected-warning{{nonnull}}
+}
+
+// Multiple attributes, multiple prototypes
+void multipleAttributes_2(char *p, char *q) __attribute((nonnull(1)));
+void multipleAttributes_2(char *p, char *q) __attribute((nonnull(2)));
+
+void testMultiple_5(void) {
+  char c;
+  multipleAttributes_2(0, &c);// expected-warning{{nonnull}}
+}
+
+void testMultiple_6(void) {
+  char c;
+  multipleAttributes_2(&c, 0);// expected-warning{{nonnull}}
+}
+
+void testMultiple_7(void) {
+  multipleAttributes_2(0, 0);// expected-warning{{nonnull}}
+}
+
+// Multiple attributes, same index
+void multipleAttributes_3(char *p, char *q) __attribute((nonnull(1))) __attribute((nonnull(1)));
+
+void testMultiple_8(void) {
+  char c;
+  multipleAttributes_3(0, &c); // expected-warning{{nonnull}}
+}
+
+void testMultiple_9(void) {
+  char c;
+  multipleAttributes_3(&c, 0); // no-warning
+}
+
+// Multiple attributes, the middle argument is missing an attribute
+void multipleAttributes_4(char *p, char *q, char *r) __attribute((nonnull(1))) __attribute((nonnull(3)));
+
+void testMultiple_10(void) {
+  char c;
+  multipleAttributes_4(0, &c, &c); // expected-warning{{nonnull}}
+}
+
+void testMultiple_11(void) {
+  char c;
+  multipleAttributes_4(&c, 0, &c); // no-warning
+}
+
+void testMultiple_12(void) {
+  char c;
+  multipleAttributes_4(&c, &c, 0); // expected-warning{{nonnull}}
+}
+
+
+// Multiple attributes, when the last is without index
+void multipleAttributes_all_1(char *p, char *q) __attribute((nonnull(1))) __attribute((nonnull));
+
+void testMultiple_13(void) {
+  char c;
+  multipleAttributes_all_1(0, &c); // expected-warning{{nonnull}}
+}
+
+void testMultiple_14(void) {
+  char c;
+  multipleAttributes_all_1(&c, 0); // expected-warning{{nonnull}}
+}
+
+// Multiple attributes, when the first is without index
+void multipleAttributes_all_2(char *p, char *q) __attribute((nonnull)) __attribute((nonnull(2)));
+
+void testMultiple_15(void) {
+  char c;
+  multipleAttributes_all_2(0, &c); // expected-warning{{nonnull}}
+}
+
+void testMultiple_16(void) {
+  char c;
+  multipleAttributes_all_2(&c, 0); // expected-warning{{nonnull}}
+}
+
 void testVararg(int k, void *p) {
   extern void testVararg_check(int, ...) __attribute__((nonnull));
   void *n = 0;
Index: lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp	(revision 218377)
+++ lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp	(working copy)
@@ -31,6 +31,9 @@
   : public Checker< check::PreCall > {
   mutable std::unique_ptr<BugType> BTAttrNonNull;
   mutable std::unique_ptr<BugType> BTNullRefArg;
+  void checkNonNullAttr(const CallEvent &Call,
+                        CheckerContext &C,
+                        const NonNullAttr *Att) const;
 
 public:
 
@@ -49,10 +52,20 @@
   if (!FD)
     return;
 
-  // FIXME: This is wrong; there can be multiple attributes with different sets
-  // of non-null parameter indices.
-  const NonNullAttr *Att = FD->getAttr<NonNullAttr>();
+  if (FD->hasAttr<NonNullAttr>()) {
+    // Check all attributes
+    for (const NonNullAttr *NonNull: FD->specific_attrs<NonNullAttr>()) {
+      checkNonNullAttr(Call, C, NonNull);
+    }
+  } else {
+    checkNonNullAttr(Call, C, nullptr);
+  }
+}
 
+void NonNullParamChecker::checkNonNullAttr(const CallEvent &Call,
+                                           CheckerContext &C,
+                                           const NonNullAttr *Att) const {
+
   ProgramStateRef state = C.getState();
 
   CallEvent::param_type_iterator TyI = Call.param_type_begin(),

_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits