Omit arguments of __builtin_object_size from the CFG.

This builtin does not actually evaluate its arguments for side effects,
so we shouldn't include them in the CFG. In the analyzer, rely on the
constant expression evaluator to get the proper semantics, at least for
now. (In the future, we could get ambitious and try to provide path-
sensitive size values.)

In theory, this does pose a problem for liveness analysis: a variable can
be used within the __builtin_object_size argument expression but not show
up as live. However, it is very unlikely that such a value would be used
to compute the object size and not used to access the object in some way.

<rdar://problem/14760817>

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@188679 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Analysis/CFG.cpp b/lib/Analysis/CFG.cpp
index 695440c9cbe7ae37f0d223a5f71a33e2616b6be9..52706b012630263edd152370a2e084952a5f58fe 100644 (file)
--- a/lib/Analysis/CFG.cpp
+++ b/lib/Analysis/CFG.cpp
@@ -19,6 +19,7 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/StmtVisitor.h"
+#include "clang/Basic/Builtins.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/OwningPtr.h"
 #include "llvm/ADT/SmallPtrSet.h"
@@ -1462,18 +1463,33 @@ CFGBlock *CFGBuilder::VisitCallExpr(CallExpr *C, AddStmtChoice asc) {
       AddEHEdge = true;
   }
 
+  // If this is a call to a builtin function, it might not actually evaluate
+  // its arguments. Don't add them to the CFG if this is the case.
+  bool OmitArguments = false;
+
   if (FunctionDecl *FD = C->getDirectCallee()) {
     if (FD->isNoReturn())
       NoReturn = true;
     if (FD->hasAttr<NoThrowAttr>())
       AddEHEdge = false;
+    if (FD->getBuiltinID() == Builtin::BI__builtin_object_size)
+      OmitArguments = true;
   }
 
   if (!CanThrow(C->getCallee(), *Context))
     AddEHEdge = false;
 
-  if (!NoReturn && !AddEHEdge)
+  if (OmitArguments) {
+    assert(!NoReturn && "noreturn calls with unevaluated args not implemented");
+    assert(!AddEHEdge && "EH calls with unevaluated args not implemented");
+    autoCreateBlock();
+    appendStmt(Block, C);
+    return Visit(C->getCallee());
+  }
+
+  if (!NoReturn && !AddEHEdge) {
     return VisitStmt(C, asc.withAlwaysAdd(true));
+  }
 
   if (Block) {
     Succ = Block;

diff --git a/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp b/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
index 5300fae7756dd8e28e2c0303536f8ab0ba309c10..a871049848835915cafc549f055adce99d44ce1b 100644 (file)
--- a/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -37,12 +37,10 @@ bool BuiltinFunctionChecker::evalCall(const CallExpr *CE,
   if (!FD)
     return false;
 
-  unsigned id = FD->getBuiltinID();
-
-  if (!id)
+  switch (FD->getBuiltinID()) {
+  default:
     return false;
 
-  switch (id) {
   case Builtin::BI__builtin_expect:
   case Builtin::BI__builtin_addressof: {
     // For __builtin_expect, just return the value of the subexpression.
@@ -76,9 +74,24 @@ bool BuiltinFunctionChecker::evalCall(const CallExpr *CE,
     C.addTransition(state->BindExpr(CE, LCtx, loc::MemRegionVal(R)));
     return true;
   }
-  }
 
-  return false;
+  case Builtin::BI__builtin_object_size: {
+    // This must be resolvable at compile time, so we defer to the constant
+    // evaluator for a value.
+    SVal V = UnknownVal();
+    llvm::APSInt Result;
+    if (CE->EvaluateAsInt(Result, C.getASTContext(), Expr::SE_NoSideEffects)) {
+      // Make sure the result has the correct type.
+      SValBuilder &SVB = C.getSValBuilder();
+      BasicValueFactory &BVF = SVB.getBasicValueFactory();
+      BVF.getAPSIntType(CE->getType()).apply(Result);
+      V = SVB.makeIntVal(Result);
+    }
+
+    C.addTransition(state->BindExpr(CE, LCtx, V));
+    return true;
+  }
+  }
 }
 
 void ento::registerBuiltinFunctionChecker(CheckerManager &mgr) {

diff --git a/test/Analysis/builtin-functions.cpp b/test/Analysis/builtin-functions.cpp
index 5c18665d0fdb5c6c7a5c5ada30f069dc6b97099c..72d5ad23b16e82bc430939eb73454068db3c4900 100644 (file)
--- a/test/Analysis/builtin-functions.cpp
+++ b/test/Analysis/builtin-functions.cpp
@@ -2,6 +2,23 @@
 
 void clang_analyzer_eval(bool);
 
-void test(int x) {
+void testAddressof(int x) {
   clang_analyzer_eval(&x == __builtin_addressof(x)); // expected-warning{{TRUE}}
 }
+
+void testSize() {
+  struct {
+    int x;
+    int y;
+    char z;
+  } object;
+  clang_analyzer_eval(__builtin_object_size(&object.y, 0) == sizeof(object) - sizeof(int)); // expected-warning{{TRUE}}
+
+  // Clang can't actually evaluate these builtin "calls", but importantly they don't actually evaluate the argument expression either.
+  int i = 0;
+  char buf[10];
+  clang_analyzer_eval(__builtin_object_size(&buf[i++], 0) == sizeof(buf)); // expected-warning{{FALSE}}
+  clang_analyzer_eval(__builtin_object_size(&buf[++i], 0) == sizeof(buf) - 1); // expected-warning{{FALSE}}
+
+  clang_analyzer_eval(i == 0); // expected-warning{{TRUE}}
+}

diff --git a/test/Analysis/cfg.cpp b/test/Analysis/cfg.cpp
index c2ec2e9bec2cd34dc27e1d314ddf8dc0551e6852..26e12dcde9880dfcc4e99c9102059a7ff054d769 100644 (file)
--- a/test/Analysis/cfg.cpp
+++ b/test/Analysis/cfg.cpp
@@ -82,3 +82,19 @@ enum EmptyE {};
 void F(EmptyE e) {
   switch (e) {}
 }
+
+// CHECK: ENTRY
+// CHECK-NEXT: Succs (1): B1
+// CHECK: [B1]
+// CHECK-NEXT:   1: __builtin_object_size
+// CHECK-NEXT:   2: [B1.1] (ImplicitCastExpr, BuiltinFnToFnPtr, unsigned long (*)(const void *, int))
+// CHECK-NEXT:   3: [B1.2](dummy(), 0)
+// CHECK-NEXT:   4: (void)[B1.3] (CStyleCastExpr, ToVoid, void)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK: [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+void testBuiltinSize() {
+  extern int *dummy();
+  (void)__builtin_object_size(dummy(), 0);
+}