diff --git a/docs/analyzer/IPA.txt b/docs/analyzer/IPA.txt index 5c722c7a0e..071727f055 100644 --- a/docs/analyzer/IPA.txt +++ b/docs/analyzer/IPA.txt @@ -103,7 +103,8 @@ some cases, however, where the analyzer chooses not to inline: See "C++ Caveats" below. - In C++, ExprEngine does not inline custom implementations of operator 'new' - or operator 'delete'. See "C++ Caveats" below. + or operator 'delete', nor does it inline the constructors and destructors + associated with these. See "C++ Caveats" below. - Calls resulting in "dynamic dispatch" are specially handled. See more below. diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index 4ce434d198..4e4f103ea3 100644 --- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -18,6 +18,7 @@ #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/AST/DeclCXX.h" +#include "clang/AST/ParentMap.h" #include "llvm/ADT/SmallSet.h" #include "llvm/ADT/Statistic.h" #include "llvm/Support/SaveAndRestore.h" @@ -350,6 +351,15 @@ bool ExprEngine::inlineCall(const CallEvent &Call, const Decl *D, if (Target && isa(Target)) return false; + // FIXME: This is a hack. We don't use the correct region for a new + // expression, so if we inline the constructor its result will just be + // thrown away. This short-term hack is tracked in + // and the longer-term possible fix is discussed in PR12014. + const CXXConstructExpr *CtorExpr = Ctor.getOriginExpr(); + if (const Stmt *Parent = CurLC->getParentMap().getParent(CtorExpr)) + if (isa(Parent)) + return false; + // If the destructor is trivial, it's always safe to inline the constructor. if (Ctor.getDecl()->getParent()->hasTrivialDestructor()) break; @@ -363,7 +373,6 @@ bool ExprEngine::inlineCall(const CallEvent &Call, const Decl *D, // FIXME: This is a hack. We don't handle temporary destructors // right now, so we shouldn't inline their constructors. - const CXXConstructExpr *CtorExpr = Ctor.getOriginExpr(); if (CtorExpr->getConstructionKind() == CXXConstructExpr::CK_Complete) if (!Target || !isa(Target)) return false; diff --git a/test/Analysis/inline.cpp b/test/Analysis/inline.cpp index 6590776266..573b1647a3 100644 --- a/test/Analysis/inline.cpp +++ b/test/Analysis/inline.cpp @@ -1,8 +1,18 @@ -// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-ipa=inlining -verify %s +// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-ipa=inlining -verify %s void clang_analyzer_eval(bool); void clang_analyzer_checkInlined(bool); +typedef __typeof__(sizeof(int)) size_t; +extern "C" void *malloc(size_t); + +// This is the standard placement new. +inline void* operator new(size_t, void* __p) throw() +{ + return __p; +} + + class A { public: int getZero() { return 0; } @@ -227,3 +237,33 @@ namespace DefaultArgs { clang_analyzer_eval(obj.get() == 42); // expected-warning{{UNKNOWN}} } } + +namespace OperatorNew { + class IntWrapper { + public: + int value; + + IntWrapper(int input) : value(input) { + // We don't want this constructor to be inlined unless we can actually + // use the proper region for operator new. + // See PR12014 and . + clang_analyzer_checkInlined(false); // no-warning + } + }; + + void test() { + IntWrapper *obj = new IntWrapper(42); + // should be TRUE + clang_analyzer_eval(obj->value == 42); // expected-warning{{UNKNOWN}} + } + + void testPlacement() { + IntWrapper *obj = static_cast(malloc(sizeof(IntWrapper))); + IntWrapper *alias = new (obj) IntWrapper(42); + + clang_analyzer_eval(alias == obj); // expected-warning{{TRUE}} + + // should be TRUE + clang_analyzer_eval(obj->value == 42); // expected-warning{{UNKNOWN}} + } +}