diff --git a/clang/lib/Analysis/BodyFarm.cpp b/clang/lib/Analysis/BodyFarm.cpp index 603da67156254..b2110f6b5450c 100644 --- a/clang/lib/Analysis/BodyFarm.cpp +++ b/clang/lib/Analysis/BodyFarm.cpp @@ -742,8 +742,9 @@ static const ObjCIvarDecl *findBackingIvar(const ObjCPropertyDecl *Prop) { static Stmt *createObjCPropertyGetter(ASTContext &Ctx, const ObjCMethodDecl *MD) { - // First, find the backing ivar. + // First, find the backing ivar. const ObjCIvarDecl *IVar = nullptr; + const ObjCPropertyDecl *Prop = nullptr; // Property accessor stubs sometimes do not correspond to any property decl // in the current interface (but in a superclass). They still have a @@ -751,54 +752,57 @@ static Stmt *createObjCPropertyGetter(ASTContext &Ctx, if (MD->isSynthesizedAccessorStub()) { const ObjCInterfaceDecl *IntD = MD->getClassInterface(); const ObjCImplementationDecl *ImpD = IntD->getImplementation(); - for (const auto *PI: ImpD->property_impls()) { - if (const ObjCPropertyDecl *P = PI->getPropertyDecl()) { - if (P->getGetterName() == MD->getSelector()) - IVar = P->getPropertyIvarDecl(); + for (const auto *PI : ImpD->property_impls()) { + if (const ObjCPropertyDecl *Candidate = PI->getPropertyDecl()) { + if (Candidate->getGetterName() == MD->getSelector()) { + Prop = Candidate; + IVar = Prop->getPropertyIvarDecl(); + } } } } if (!IVar) { - const ObjCPropertyDecl *Prop = MD->findPropertyDecl(); + Prop = MD->findPropertyDecl(); IVar = findBackingIvar(Prop); - if (!IVar) - return nullptr; + } - // Ignore weak variables, which have special behavior. - if (Prop->getPropertyAttributes() & ObjCPropertyAttribute::kind_weak) - return nullptr; + if (!IVar || !Prop) + return nullptr; + + // Ignore weak variables, which have special behavior. + if (Prop->getPropertyAttributes() & ObjCPropertyAttribute::kind_weak) + return nullptr; - // Look to see if Sema has synthesized a body for us. This happens in - // Objective-C++ because the return value may be a C++ class type with a - // non-trivial copy constructor. We can only do this if we can find the - // @synthesize for this property, though (or if we know it's been auto- - // synthesized). - const ObjCImplementationDecl *ImplDecl = + // Look to see if Sema has synthesized a body for us. This happens in + // Objective-C++ because the return value may be a C++ class type with a + // non-trivial copy constructor. We can only do this if we can find the + // @synthesize for this property, though (or if we know it's been auto- + // synthesized). + const ObjCImplementationDecl *ImplDecl = IVar->getContainingInterface()->getImplementation(); - if (ImplDecl) { - for (const auto *I : ImplDecl->property_impls()) { - if (I->getPropertyDecl() != Prop) - continue; - - if (I->getGetterCXXConstructor()) { - ASTMaker M(Ctx); - return M.makeReturn(I->getGetterCXXConstructor()); - } + if (ImplDecl) { + for (const auto *I : ImplDecl->property_impls()) { + if (I->getPropertyDecl() != Prop) + continue; + + if (I->getGetterCXXConstructor()) { + ASTMaker M(Ctx); + return M.makeReturn(I->getGetterCXXConstructor()); } } - - // Sanity check that the property is the same type as the ivar, or a - // reference to it, and that it is either an object pointer or trivially - // copyable. - if (!Ctx.hasSameUnqualifiedType(IVar->getType(), - Prop->getType().getNonReferenceType())) - return nullptr; - if (!IVar->getType()->isObjCLifetimeType() && - !IVar->getType().isTriviallyCopyableType(Ctx)) - return nullptr; } + // Sanity check that the property is the same type as the ivar, or a + // reference to it, and that it is either an object pointer or trivially + // copyable. + if (!Ctx.hasSameUnqualifiedType(IVar->getType(), + Prop->getType().getNonReferenceType())) + return nullptr; + if (!IVar->getType()->isObjCLifetimeType() && + !IVar->getType().isTriviallyCopyableType(Ctx)) + return nullptr; + // Generate our body: // return self->_ivar; ASTMaker M(Ctx); @@ -807,11 +811,8 @@ static Stmt *createObjCPropertyGetter(ASTContext &Ctx, if (!selfVar) return nullptr; - Expr *loadedIVar = - M.makeObjCIvarRef( - M.makeLvalueToRvalue( - M.makeDeclRefExpr(selfVar), - selfVar->getType()), + Expr *loadedIVar = M.makeObjCIvarRef( + M.makeLvalueToRvalue(M.makeDeclRefExpr(selfVar), selfVar->getType()), IVar); if (!MD->getReturnType()->isReferenceType()) diff --git a/clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp index 8c86e83608b1e..8070d869f6785 100644 --- a/clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp @@ -11,17 +11,18 @@ // //===----------------------------------------------------------------------===// -#include "clang/Lex/Lexer.h" -#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/AST/ASTContext.h" #include "clang/AST/Attr.h" #include "clang/AST/ParentMap.h" #include "clang/AST/RecursiveASTVisitor.h" #include "clang/Analysis/Analyses/LiveVariables.h" +#include "clang/Lex/Lexer.h" +#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" #include "llvm/ADT/BitVector.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallString.h" #include "llvm/Support/SaveAndRestore.h" @@ -408,15 +409,17 @@ class DeadStoreObs : public LiveVariables::Observer { // Special case: check for initializations with constants. // // e.g. : int x = 0; + // struct A = {0, 1}; + // struct B = {{0}, {1, 2}}; // // If x is EVER assigned a new value later, don't issue // a warning. This is because such initialization can be // due to defensive programming. - if (E->isEvaluatable(Ctx)) + if (isConstant(E)) return; if (const DeclRefExpr *DRE = - dyn_cast(E->IgnoreParenCasts())) + dyn_cast(E->IgnoreParenCasts())) if (const VarDecl *VD = dyn_cast(DRE->getDecl())) { // Special case: check for initialization from constant // variables. @@ -444,6 +447,29 @@ class DeadStoreObs : public LiveVariables::Observer { } } } + +private: + /// Return true if the given init list can be interpreted as constant + bool isConstant(const InitListExpr *Candidate) const { + // We consider init list to be constant if each member of the list can be + // interpreted as constant. + return llvm::all_of(Candidate->inits(), + [this](const Expr *Init) { return isConstant(Init); }); + } + + /// Return true if the given expression can be interpreted as constant + bool isConstant(const Expr *E) const { + // It looks like E itself is a constant + if (E->isEvaluatable(Ctx)) + return true; + + // We should also allow defensive initialization of structs, i.e. { 0 } + if (const auto *ILE = dyn_cast(E->IgnoreParenCasts())) { + return isConstant(ILE); + } + + return false; + } }; } // end anonymous namespace diff --git a/clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp index 65e52e139ee41..bcae73378028a 100644 --- a/clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp @@ -34,9 +34,9 @@ namespace { class InnerPointerChecker : public Checker { - CallDescription AppendFn, AssignFn, ClearFn, CStrFn, DataFn, EraseFn, - InsertFn, PopBackFn, PushBackFn, ReplaceFn, ReserveFn, ResizeFn, - ShrinkToFitFn, SwapFn; + CallDescription AppendFn, AssignFn, AddressofFn, ClearFn, CStrFn, DataFn, + DataMemberFn, EraseFn, InsertFn, PopBackFn, PushBackFn, ReplaceFn, + ReserveFn, ResizeFn, ShrinkToFitFn, SwapFn; public: class InnerPointerBRVisitor : public BugReporterVisitor { @@ -73,9 +73,10 @@ class InnerPointerChecker InnerPointerChecker() : AppendFn({"std", "basic_string", "append"}), AssignFn({"std", "basic_string", "assign"}), + AddressofFn({"std", "addressof"}), ClearFn({"std", "basic_string", "clear"}), - CStrFn({"std", "basic_string", "c_str"}), - DataFn({"std", "basic_string", "data"}), + CStrFn({"std", "basic_string", "c_str"}), DataFn({"std", "data"}, 1), + DataMemberFn({"std", "basic_string", "data"}), EraseFn({"std", "basic_string", "erase"}), InsertFn({"std", "basic_string", "insert"}), PopBackFn({"std", "basic_string", "pop_back"}), @@ -90,6 +91,9 @@ class InnerPointerChecker /// pointers referring to the container object's inner buffer. bool isInvalidatingMemberFunction(const CallEvent &Call) const; + /// Check whether the called function returns a raw inner pointer. + bool isInnerPointerAccessFunction(const CallEvent &Call) const; + /// Mark pointer symbols associated with the given memory region released /// in the program state. void markPtrSymbolsReleased(const CallEvent &Call, ProgramStateRef State, @@ -130,6 +134,12 @@ bool InnerPointerChecker::isInvalidatingMemberFunction( Call.isCalled(SwapFn)); } +bool InnerPointerChecker::isInnerPointerAccessFunction( + const CallEvent &Call) const { + return (Call.isCalled(CStrFn) || Call.isCalled(DataFn) || + Call.isCalled(DataMemberFn)); +} + void InnerPointerChecker::markPtrSymbolsReleased(const CallEvent &Call, ProgramStateRef State, const MemRegion *MR, @@ -172,6 +182,11 @@ void InnerPointerChecker::checkFunctionArguments(const CallEvent &Call, if (!ArgRegion) continue; + // std::addressof function accepts a non-const reference as an argument, + // but doesn't modify it. + if (Call.isCalled(AddressofFn)) + continue; + markPtrSymbolsReleased(Call, State, ArgRegion, C); } } @@ -195,36 +210,49 @@ void InnerPointerChecker::checkPostCall(const CallEvent &Call, CheckerContext &C) const { ProgramStateRef State = C.getState(); + // TODO: Do we need these to be typed? + const TypedValueRegion *ObjRegion = nullptr; + if (const auto *ICall = dyn_cast(&Call)) { - // TODO: Do we need these to be typed? - const auto *ObjRegion = dyn_cast_or_null( + ObjRegion = dyn_cast_or_null( ICall->getCXXThisVal().getAsRegion()); - if (!ObjRegion) - return; - if (Call.isCalled(CStrFn) || Call.isCalled(DataFn)) { - SVal RawPtr = Call.getReturnValue(); - if (SymbolRef Sym = RawPtr.getAsSymbol(/*IncludeBaseRegions=*/true)) { - // Start tracking this raw pointer by adding it to the set of symbols - // associated with this container object in the program state map. + // Check [string.require] / second point. + if (isInvalidatingMemberFunction(Call)) { + markPtrSymbolsReleased(Call, State, ObjRegion, C); + return; + } + } - PtrSet::Factory &F = State->getStateManager().get_context(); - const PtrSet *SetPtr = State->get(ObjRegion); - PtrSet Set = SetPtr ? *SetPtr : F.getEmptySet(); - assert(C.wasInlined || !Set.contains(Sym)); - Set = F.add(Set, Sym); + if (isInnerPointerAccessFunction(Call)) { - State = State->set(ObjRegion, Set); - C.addTransition(State); - } - return; + if (isa(Call)) { + // NOTE: As of now, we only have one free access function: std::data. + // If we add more functions like this in the list, hardcoded + // argument index should be changed. + ObjRegion = + dyn_cast_or_null(Call.getArgSVal(0).getAsRegion()); } - // Check [string.require] / second point. - if (isInvalidatingMemberFunction(Call)) { - markPtrSymbolsReleased(Call, State, ObjRegion, C); + if (!ObjRegion) return; + + SVal RawPtr = Call.getReturnValue(); + if (SymbolRef Sym = RawPtr.getAsSymbol(/*IncludeBaseRegions=*/true)) { + // Start tracking this raw pointer by adding it to the set of symbols + // associated with this container object in the program state map. + + PtrSet::Factory &F = State->getStateManager().get_context(); + const PtrSet *SetPtr = State->get(ObjRegion); + PtrSet Set = SetPtr ? *SetPtr : F.getEmptySet(); + assert(C.wasInlined || !Set.contains(Sym)); + Set = F.add(Set, Sym); + + State = State->set(ObjRegion, Set); + C.addTransition(State); } + + return; } // Check [string.require] / first point. diff --git a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp index 72b8ada1dfab9..620b7b3c3eea7 100644 --- a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp +++ b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp @@ -423,6 +423,14 @@ SVal SValBuilder::evalBinOp(ProgramStateRef state, BinaryOperator::Opcode op, return UnknownVal(); } + if (op == BinaryOperatorKind::BO_Cmp) { + // We can't reason about C++20 spaceship operator yet. + // + // FIXME: Support C++20 spaceship operator. + // The main problem here is that the result is not integer. + return UnknownVal(); + } + if (Optional LV = lhs.getAs()) { if (Optional RV = rhs.getAs()) return evalBinOpLL(state, op, *LV, *RV, type); diff --git a/clang/test/Analysis/PR47511.cpp b/clang/test/Analysis/PR47511.cpp new file mode 100644 index 0000000000000..d42799f4fbde5 --- /dev/null +++ b/clang/test/Analysis/PR47511.cpp @@ -0,0 +1,19 @@ +// RUN: %clang_analyze_cc1 -std=c++20 -w -analyzer-checker=core -verify %s + +// expected-no-diagnostics + +namespace std { +struct strong_ordering { + int n; + constexpr operator int() const { return n; } + static const strong_ordering equal, greater, less; +}; +constexpr strong_ordering strong_ordering::equal = {0}; +constexpr strong_ordering strong_ordering::greater = {1}; +constexpr strong_ordering strong_ordering::less = {-1}; +} // namespace std + +void test() { + // no crash + (void)(0 <=> 0); +} diff --git a/clang/test/Analysis/dead-stores.c b/clang/test/Analysis/dead-stores.c index a17e1692496da..145b81bd03327 100644 --- a/clang/test/Analysis/dead-stores.c +++ b/clang/test/Analysis/dead-stores.c @@ -635,3 +635,44 @@ void testVolatile() { volatile int v; v = 0; // no warning } + +struct Foo { + int x; + int y; +}; + +struct Foo rdar34122265_getFoo(void); + +int rdar34122265_test(int input) { + // This is allowed for defensive programming. + struct Foo foo = {0, 0}; + if (input > 0) { + foo = rdar34122265_getFoo(); + } else { + return 0; + } + return foo.x + foo.y; +} + +void rdar34122265_test_cast() { + // This is allowed for defensive programming. + struct Foo foo = {0, 0}; + (void)foo; +} + +struct Bar { + struct Foo x, y; +}; + +struct Bar rdar34122265_getBar(void); + +int rdar34122265_test_nested(int input) { + // This is allowed for defensive programming. + struct Bar bar = {{0, 0}, {0, 0}}; + if (input > 0) { + bar = rdar34122265_getBar(); + } else { + return 0; + } + return bar.x.x + bar.y.y; +} diff --git a/clang/test/Analysis/inner-pointer.cpp b/clang/test/Analysis/inner-pointer.cpp index d8b011a7aa64e..920a1fe8dcc9d 100644 --- a/clang/test/Analysis/inner-pointer.cpp +++ b/clang/test/Analysis/inner-pointer.cpp @@ -17,6 +17,11 @@ void func_value(T a); string my_string = "default"; void default_arg(int a = 42, string &b = my_string); +template +T *addressof(T &arg); + +char *data(std::string &c); + } // end namespace std void consume(const char *) {} @@ -273,6 +278,15 @@ void deref_after_swap() { // expected-note@-1 {{Inner pointer of container used after re/deallocation}} } +void deref_after_std_data() { + const char *c; + std::string s; + c = std::data(s); // expected-note {{Pointer to inner buffer of 'std::string' obtained here}} + s.push_back('c'); // expected-note {{Inner buffer of 'std::string' reallocated by call to 'push_back'}} + consume(c); // expected-warning {{Inner pointer of container used after re/deallocation}} + // expected-note@-1 {{Inner pointer of container used after re/deallocation}} +} + struct S { std::string s; const char *name() { @@ -361,8 +375,24 @@ void func_default_arg() { // expected-note@-1 {{Inner pointer of container used after re/deallocation}} } +void func_addressof() { + const char *c; + std::string s; + c = s.c_str(); + addressof(s); + consume(c); // no-warning +} + +void func_std_data() { + const char *c; + std::string s; + c = std::data(s); + consume(c); // no-warning +} + struct T { std::string to_string() { return s; } + private: std::string s; }; diff --git a/clang/test/Analysis/properties.mm b/clang/test/Analysis/properties.mm index 2d93d6dc63856..fc3be967115e1 100644 --- a/clang/test/Analysis/properties.mm +++ b/clang/test/Analysis/properties.mm @@ -77,3 +77,23 @@ void testConsistencyCustomCopy(CustomCopyWrapper *w) { clang_analyzer_eval(w.inner.value == 42); // expected-warning{{TRUE}} } + +@protocol NoDirectPropertyDecl +@property IntWrapperStruct inner; +@end +@interface NoDirectPropertyDecl +@end +@implementation NoDirectPropertyDecl +@synthesize inner; +@end + +// rdar://67416721 +void testNoDirectPropertyDecl(NoDirectPropertyDecl *w) { + clang_analyzer_eval(w.inner.value == w.inner.value); // expected-warning{{TRUE}} + + int origValue = w.inner.value; + if (origValue != 42) + return; + + clang_analyzer_eval(w.inner.value == 42); // expected-warning{{TRUE}} +}