[analyzer] Extend UnixAPIChecker open() checks to handle openat().

The openat() API has similar constraints to the open() API -- it just takes
an extra parameter.

rdar://problem/29526458

llvm-svn: 290005

clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp

@@ -21,6 +21,7 @@
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/raw_ostream.h"
 #include <fcntl.h>
@@ -28,6 +29,16 @@
 using namespace clang;
 using namespace ento;
 
+enum class OpenVariant {
+  /// The standard open() call:
+  ///    int open(const char *path, int oflag, ...);
+  Open,
+
+  /// The variant taking a directory file descriptor and a relative path:
+  ///    int openat(int fd, const char *path, int oflag, ...);
+  OpenAt
+};
+
 namespace {
 class UnixAPIChecker : public Checker< check::PreStmt<CallExpr> > {
   mutable std::unique_ptr<BugType> BT_open, BT_pthreadOnce, BT_mallocZero;
@@ -37,6 +48,8 @@ public:
   void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
 
   void CheckOpen(CheckerContext &C, const CallExpr *CE) const;
+  void CheckOpenAt(CheckerContext &C, const CallExpr *CE) const;
+
   void CheckPthreadOnce(CheckerContext &C, const CallExpr *CE) const;
   void CheckCallocZero(CheckerContext &C, const CallExpr *CE) const;
   void CheckMallocZero(CheckerContext &C, const CallExpr *CE) const;
@@ -49,6 +62,10 @@ public:
   typedef void (UnixAPIChecker::*SubChecker)(CheckerContext &,
                                              const CallExpr *) const;
 private:
+
+  void CheckOpenVariant(CheckerContext &C,
+                        const CallExpr *CE, OpenVariant Variant) const;
+
   bool ReportZeroByteAllocation(CheckerContext &C,
                                 ProgramStateRef falseState,
                                 const Expr *arg,
@@ -90,25 +107,71 @@ void UnixAPIChecker::ReportOpenBug(CheckerContext &C,
 }
 
 void UnixAPIChecker::CheckOpen(CheckerContext &C, const CallExpr *CE) const {
+  CheckOpenVariant(C, CE, OpenVariant::Open);
+}
+
+void UnixAPIChecker::CheckOpenAt(CheckerContext &C, const CallExpr *CE) const {
+  CheckOpenVariant(C, CE, OpenVariant::OpenAt);
+}
+
+void UnixAPIChecker::CheckOpenVariant(CheckerContext &C,
+                                      const CallExpr *CE,
+                                      OpenVariant Variant) const {
+  // The index of the argument taking the flags open flags (O_RDONLY,
+  // O_WRONLY, O_CREAT, etc.),
+  unsigned int FlagsArgIndex;
+  const char *VariantName;
+  switch (Variant) {
+  case OpenVariant::Open:
+    FlagsArgIndex = 1;
+    VariantName = "open";
+    break;
+  case OpenVariant::OpenAt:
+    FlagsArgIndex = 2;
+    VariantName = "openat";
+    break;
+  };
+
+  // All calls should at least provide arguments up to the 'flags' parameter.
+  unsigned int MinArgCount = FlagsArgIndex + 1;
+
+  // If the flags has O_CREAT set then open/openat() require an additional
+  // argument specifying the file mode (permission bits) for the created file.
+  unsigned int CreateModeArgIndex = FlagsArgIndex + 1;
+
+  // The create mode argument should be the last argument.
+  unsigned int MaxArgCount = CreateModeArgIndex + 1;
+
   ProgramStateRef state = C.getState();
 
-  if (CE->getNumArgs() < 2) {
+  if (CE->getNumArgs() < MinArgCount) {
     // The frontend should issue a warning for this case, so this is a sanity
     // check.
     return;
-  } else if (CE->getNumArgs() == 3) {
-    const Expr *Arg = CE->getArg(2);
+  } else if (CE->getNumArgs() == MaxArgCount) {
+    const Expr *Arg = CE->getArg(CreateModeArgIndex);
     QualType QT = Arg->getType();
     if (!QT->isIntegerType()) {
+      SmallString<256> SBuf;
+      llvm::raw_svector_ostream OS(SBuf);
+      OS << "The " << CreateModeArgIndex + 1
+         << llvm::getOrdinalSuffix(CreateModeArgIndex + 1)
+         << " argument to '" << VariantName << "' is not an integer";
+
       ReportOpenBug(C, state,
-                    "Third argument to 'open' is not an integer",
+                    SBuf.c_str(),
                     Arg->getSourceRange());
       return;
     }
-  } else if (CE->getNumArgs() > 3) {
+  } else if (CE->getNumArgs() > MaxArgCount) {
+    SmallString<256> SBuf;
+    llvm::raw_svector_ostream OS(SBuf);
+    OS << "Call to '" << VariantName << "' with more than " << MaxArgCount
+       << " arguments";
+
     ReportOpenBug(C, state,
-                  "Call to 'open' with more than three arguments",
-                  CE->getArg(3)->getSourceRange());
+                  SBuf.c_str(),
+                  CE->getArg(MaxArgCount)->getSourceRange());
     return;
   }
 
@@ -128,7 +191,7 @@ void UnixAPIChecker::CheckOpen(CheckerContext &C, const CallExpr *CE) const {
   }
 
   // Now check if oflags has O_CREAT set.
-  const Expr *oflagsEx = CE->getArg(1);
+  const Expr *oflagsEx = CE->getArg(FlagsArgIndex);
   const SVal V = state->getSVal(oflagsEx, C.getLocationContext());
   if (!V.getAs<NonLoc>()) {
     // The case where 'V' can be a location can only be due to a bad header,
@@ -154,10 +217,15 @@ void UnixAPIChecker::CheckOpen(CheckerContext &C, const CallExpr *CE) const {
   if (!(trueState && !falseState))
     return;
 
-  if (CE->getNumArgs() < 3) {
+  if (CE->getNumArgs() < MaxArgCount) {
+    SmallString<256> SBuf;
+    llvm::raw_svector_ostream OS(SBuf);
+    OS << "Call to '" << VariantName << "' requires a "
+       << CreateModeArgIndex + 1
+       << llvm::getOrdinalSuffix(CreateModeArgIndex + 1)
+       << " argument when the 'O_CREAT' flag is set";
     ReportOpenBug(C, trueState,
-                  "Call to 'open' requires a third argument when "
-                  "the 'O_CREAT' flag is set",
+                  SBuf.c_str(),
                   oflagsEx->getSourceRange());
   }
 }
@@ -366,6 +434,7 @@ void UnixAPIChecker::checkPreStmt(const CallExpr *CE,
   SubChecker SC =
     llvm::StringSwitch<SubChecker>(FName)
       .Case("open", &UnixAPIChecker::CheckOpen)
+      .Case("openat", &UnixAPIChecker::CheckOpenAt)
       .Case("pthread_once", &UnixAPIChecker::CheckPthreadOnce)
       .Case("calloc", &UnixAPIChecker::CheckCallocZero)
       .Case("malloc", &UnixAPIChecker::CheckMallocZero)

clang/test/Analysis/unix-api.c

@@ -9,6 +9,7 @@
 #endif
 
 int open(const char *, int, ...);
+int openat(int, const char *, int, ...);
 int close(int fildes);
 
 void open_1(const char *path) {
@@ -21,21 +22,37 @@ void open_1(const char *path) {
 void open_2(const char *path) {
   int fd;
   int mode = 0x0;
-  fd = open(path, O_RDONLY, mode, NULL); // expected-warning{{Call to 'open' with more than three arguments}}
+  fd = open(path, O_RDONLY, mode, NULL); // expected-warning{{Call to 'open' with more than 3 arguments}}
+  if (fd > -1)
+    close(fd);
+}
+
+void openat_2(int base_fd, const char *path) {
+  int fd;
+  int mode = 0x0;
+  fd = openat(base_fd, path, O_RDONLY, mode, NULL); // expected-warning{{Call to 'openat' with more than 4 arguments}}
   if (fd > -1)
     close(fd);
 }
 
 void open_3(const char *path) {
   int fd;
-  fd = open(path, O_RDONLY, NULL); // expected-warning{{Third argument to 'open' is not an integer}}
+  fd = open(path, O_RDONLY, NULL); // expected-warning{{The 3rd argument to 'open' is not an integer}}
   if (fd > -1)
     close(fd);
 }
 
+void openat_3(int base_fd, const char *path) {
+  int fd;
+  fd = openat(base_fd, path, O_RDONLY, NULL); // expected-warning{{The 4th argument to 'openat' is not an integer}}
+  if (fd > -1)
+    close(fd);
+}
+
+
 void open_4(const char *path) {
   int fd;
-  fd = open(path, O_RDONLY, ""); // expected-warning{{Third argument to 'open' is not an integer}}
+  fd = open(path, O_RDONLY, ""); // expected-warning{{The 3rd argument to 'open' is not an integer}}
   if (fd > -1)
     close(fd);
 }
@@ -45,7 +62,7 @@ void open_5(const char *path) {
   struct {
     int val;
   } st = {0};
-  fd = open(path, O_RDONLY, st); // expected-warning{{Third argument to 'open' is not an integer}}
+  fd = open(path, O_RDONLY, st); // expected-warning{{The 3rd argument to 'open' is not an integer}}
   if (fd > -1)
     close(fd);
 }
@@ -62,14 +79,14 @@ void open_6(const char *path) {
 
 void open_7(const char *path) {
   int fd;
-  fd = open(path, O_RDONLY, &open); // expected-warning{{Third argument to 'open' is not an integer}}
+  fd = open(path, O_RDONLY, &open); // expected-warning{{The 3rd argument to 'open' is not an integer}}
   if (fd > -1)
     close(fd);
 }
 
 void open_8(const char *path) {
   int fd;
-  fd = open(path, O_RDONLY, 0.0f); // expected-warning{{Third argument to 'open' is not an integer}}
+  fd = open(path, O_RDONLY, 0.0f); // expected-warning{{The 3rd argument to 'open' is not an integer}}
   if (fd > -1)
     close(fd);
 }