[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

1 files changed, 80 insertions, 11 deletions

diff --git a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
index c9dafb51..0945bbf 100644
--- a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
+++ b/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)