Skip to content

Commit 9255c51

Browse files
SavchenkoValeriySavchenkoValeriy
authored
Merge pull request #2820 from SavchenkoValeriy/april_analyzer_cherry_picks_2
Static analyzer cherry picks #22
2 parents 5bcd6b7 + ffa7868 commit 9255c51

File tree

8 files changed

+244
-71
lines changed

8 files changed

+244
-71
lines changed

‎clang/lib/Analysis/BodyFarm.cpp

+42-41
Original file line numberDiff line numberDiff line change
@@ -742,63 +742,67 @@ static const ObjCIvarDecl *findBackingIvar(const ObjCPropertyDecl *Prop) {
742742

743743
static Stmt *createObjCPropertyGetter(ASTContext &Ctx,
744744
const ObjCMethodDecl *MD) {
745-
// First, find the backing ivar.
745+
// First, find the backing ivar.
746746
const ObjCIvarDecl *IVar = nullptr;
747+
const ObjCPropertyDecl *Prop = nullptr;
747748

748749
// Property accessor stubs sometimes do not correspond to any property decl
749750
// in the current interface (but in a superclass). They still have a
750751
// corresponding property impl decl in this case.
751752
if (MD->isSynthesizedAccessorStub()) {
752753
const ObjCInterfaceDecl *IntD = MD->getClassInterface();
753754
const ObjCImplementationDecl *ImpD = IntD->getImplementation();
754-
for (const auto *PI: ImpD->property_impls()) {
755-
if (const ObjCPropertyDecl *P = PI->getPropertyDecl()) {
756-
if (P->getGetterName() == MD->getSelector())
757-
IVar = P->getPropertyIvarDecl();
755+
for (const auto *PI : ImpD->property_impls()) {
756+
if (const ObjCPropertyDecl *Candidate = PI->getPropertyDecl()) {
757+
if (Candidate->getGetterName() == MD->getSelector()) {
758+
Prop = Candidate;
759+
IVar = Prop->getPropertyIvarDecl();
760+
}
758761
}
759762
}
760763
}
761764

762765
if (!IVar) {
763-
const ObjCPropertyDecl *Prop = MD->findPropertyDecl();
766+
Prop = MD->findPropertyDecl();
764767
IVar = findBackingIvar(Prop);
765-
if (!IVar)
766-
return nullptr;
768+
}
767769

768-
// Ignore weak variables, which have special behavior.
769-
if (Prop->getPropertyAttributes() & ObjCPropertyAttribute::kind_weak)
770-
return nullptr;
770+
if (!IVar || !Prop)
771+
return nullptr;
772+
773+
// Ignore weak variables, which have special behavior.
774+
if (Prop->getPropertyAttributes() & ObjCPropertyAttribute::kind_weak)
775+
return nullptr;
771776

772-
// Look to see if Sema has synthesized a body for us. This happens in
773-
// Objective-C++ because the return value may be a C++ class type with a
774-
// non-trivial copy constructor. We can only do this if we can find the
775-
// @synthesize for this property, though (or if we know it's been auto-
776-
// synthesized).
777-
const ObjCImplementationDecl *ImplDecl =
777+
// Look to see if Sema has synthesized a body for us. This happens in
778+
// Objective-C++ because the return value may be a C++ class type with a
779+
// non-trivial copy constructor. We can only do this if we can find the
780+
// @synthesize for this property, though (or if we know it's been auto-
781+
// synthesized).
782+
const ObjCImplementationDecl *ImplDecl =
778783
IVar->getContainingInterface()->getImplementation();
779-
if (ImplDecl) {
780-
for (const auto *I : ImplDecl->property_impls()) {
781-
if (I->getPropertyDecl() != Prop)
782-
continue;
783-
784-
if (I->getGetterCXXConstructor()) {
785-
ASTMaker M(Ctx);
786-
return M.makeReturn(I->getGetterCXXConstructor());
787-
}
784+
if (ImplDecl) {
785+
for (const auto *I : ImplDecl->property_impls()) {
786+
if (I->getPropertyDecl() != Prop)
787+
continue;
788+
789+
if (I->getGetterCXXConstructor()) {
790+
ASTMaker M(Ctx);
791+
return M.makeReturn(I->getGetterCXXConstructor());
788792
}
789793
}
790-
791-
// Sanity check that the property is the same type as the ivar, or a
792-
// reference to it, and that it is either an object pointer or trivially
793-
// copyable.
794-
if (!Ctx.hasSameUnqualifiedType(IVar->getType(),
795-
Prop->getType().getNonReferenceType()))
796-
return nullptr;
797-
if (!IVar->getType()->isObjCLifetimeType() &&
798-
!IVar->getType().isTriviallyCopyableType(Ctx))
799-
return nullptr;
800794
}
801795

796+
// Sanity check that the property is the same type as the ivar, or a
797+
// reference to it, and that it is either an object pointer or trivially
798+
// copyable.
799+
if (!Ctx.hasSameUnqualifiedType(IVar->getType(),
800+
Prop->getType().getNonReferenceType()))
801+
return nullptr;
802+
if (!IVar->getType()->isObjCLifetimeType() &&
803+
!IVar->getType().isTriviallyCopyableType(Ctx))
804+
return nullptr;
805+
802806
// Generate our body:
803807
// return self->_ivar;
804808
ASTMaker M(Ctx);
@@ -807,11 +811,8 @@ static Stmt *createObjCPropertyGetter(ASTContext &Ctx,
807811
if (!selfVar)
808812
return nullptr;
809813

810-
Expr *loadedIVar =
811-
M.makeObjCIvarRef(
812-
M.makeLvalueToRvalue(
813-
M.makeDeclRefExpr(selfVar),
814-
selfVar->getType()),
814+
Expr *loadedIVar = M.makeObjCIvarRef(
815+
M.makeLvalueToRvalue(M.makeDeclRefExpr(selfVar), selfVar->getType()),
815816
IVar);
816817

817818
if (!MD->getReturnType()->isReferenceType())

‎clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp

+30-4
Original file line numberDiff line numberDiff line change
@@ -11,17 +11,18 @@
1111
//
1212
//===----------------------------------------------------------------------===//
1313

14-
#include "clang/Lex/Lexer.h"
15-
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
1614
#include "clang/AST/ASTContext.h"
1715
#include "clang/AST/Attr.h"
1816
#include "clang/AST/ParentMap.h"
1917
#include "clang/AST/RecursiveASTVisitor.h"
2018
#include "clang/Analysis/Analyses/LiveVariables.h"
19+
#include "clang/Lex/Lexer.h"
20+
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
2121
#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
2222
#include "clang/StaticAnalyzer/Core/Checker.h"
2323
#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
2424
#include "llvm/ADT/BitVector.h"
25+
#include "llvm/ADT/STLExtras.h"
2526
#include "llvm/ADT/SmallString.h"
2627
#include "llvm/Support/SaveAndRestore.h"
2728

@@ -408,15 +409,17 @@ class DeadStoreObs : public LiveVariables::Observer {
408409
// Special case: check for initializations with constants.
409410
//
410411
// e.g. : int x = 0;
412+
// struct A = {0, 1};
413+
// struct B = {{0}, {1, 2}};
411414
//
412415
// If x is EVER assigned a new value later, don't issue
413416
// a warning. This is because such initialization can be
414417
// due to defensive programming.
415-
if (E->isEvaluatable(Ctx))
418+
if (isConstant(E))
416419
return;
417420

418421
if (const DeclRefExpr *DRE =
419-
dyn_cast<DeclRefExpr>(E->IgnoreParenCasts()))
422+
dyn_cast<DeclRefExpr>(E->IgnoreParenCasts()))
420423
if (const VarDecl *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
421424
// Special case: check for initialization from constant
422425
// variables.
@@ -444,6 +447,29 @@ class DeadStoreObs : public LiveVariables::Observer {
444447
}
445448
}
446449
}
450+
451+
private:
452+
/// Return true if the given init list can be interpreted as constant
453+
bool isConstant(const InitListExpr *Candidate) const {
454+
// We consider init list to be constant if each member of the list can be
455+
// interpreted as constant.
456+
return llvm::all_of(Candidate->inits(),
457+
[this](const Expr *Init) { return isConstant(Init); });
458+
}
459+
460+
/// Return true if the given expression can be interpreted as constant
461+
bool isConstant(const Expr *E) const {
462+
// It looks like E itself is a constant
463+
if (E->isEvaluatable(Ctx))
464+
return true;
465+
466+
// We should also allow defensive initialization of structs, i.e. { 0 }
467+
if (const auto *ILE = dyn_cast<InitListExpr>(E->IgnoreParenCasts())) {
468+
return isConstant(ILE);
469+
}
470+
471+
return false;
472+
}
447473
};
448474

449475
} // end anonymous namespace

‎clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp

+54-26
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,9 @@ namespace {
3434
class InnerPointerChecker
3535
: public Checker<check::DeadSymbols, check::PostCall> {
3636

37-
CallDescription AppendFn, AssignFn, ClearFn, CStrFn, DataFn, EraseFn,
38-
InsertFn, PopBackFn, PushBackFn, ReplaceFn, ReserveFn, ResizeFn,
39-
ShrinkToFitFn, SwapFn;
37+
CallDescription AppendFn, AssignFn, AddressofFn, ClearFn, CStrFn, DataFn,
38+
DataMemberFn, EraseFn, InsertFn, PopBackFn, PushBackFn, ReplaceFn,
39+
ReserveFn, ResizeFn, ShrinkToFitFn, SwapFn;
4040

4141
public:
4242
class InnerPointerBRVisitor : public BugReporterVisitor {
@@ -73,9 +73,10 @@ class InnerPointerChecker
7373
InnerPointerChecker()
7474
: AppendFn({"std", "basic_string", "append"}),
7575
AssignFn({"std", "basic_string", "assign"}),
76+
AddressofFn({"std", "addressof"}),
7677
ClearFn({"std", "basic_string", "clear"}),
77-
CStrFn({"std", "basic_string", "c_str"}),
78-
DataFn({"std", "basic_string", "data"}),
78+
CStrFn({"std", "basic_string", "c_str"}), DataFn({"std", "data"}, 1),
79+
DataMemberFn({"std", "basic_string", "data"}),
7980
EraseFn({"std", "basic_string", "erase"}),
8081
InsertFn({"std", "basic_string", "insert"}),
8182
PopBackFn({"std", "basic_string", "pop_back"}),
@@ -90,6 +91,9 @@ class InnerPointerChecker
9091
/// pointers referring to the container object's inner buffer.
9192
bool isInvalidatingMemberFunction(const CallEvent &Call) const;
9293

94+
/// Check whether the called function returns a raw inner pointer.
95+
bool isInnerPointerAccessFunction(const CallEvent &Call) const;
96+
9397
/// Mark pointer symbols associated with the given memory region released
9498
/// in the program state.
9599
void markPtrSymbolsReleased(const CallEvent &Call, ProgramStateRef State,
@@ -130,6 +134,12 @@ bool InnerPointerChecker::isInvalidatingMemberFunction(
130134
Call.isCalled(SwapFn));
131135
}
132136

137+
bool InnerPointerChecker::isInnerPointerAccessFunction(
138+
const CallEvent &Call) const {
139+
return (Call.isCalled(CStrFn) || Call.isCalled(DataFn) ||
140+
Call.isCalled(DataMemberFn));
141+
}
142+
133143
void InnerPointerChecker::markPtrSymbolsReleased(const CallEvent &Call,
134144
ProgramStateRef State,
135145
const MemRegion *MR,
@@ -172,6 +182,11 @@ void InnerPointerChecker::checkFunctionArguments(const CallEvent &Call,
172182
if (!ArgRegion)
173183
continue;
174184

185+
// std::addressof function accepts a non-const reference as an argument,
186+
// but doesn't modify it.
187+
if (Call.isCalled(AddressofFn))
188+
continue;
189+
175190
markPtrSymbolsReleased(Call, State, ArgRegion, C);
176191
}
177192
}
@@ -195,36 +210,49 @@ void InnerPointerChecker::checkPostCall(const CallEvent &Call,
195210
CheckerContext &C) const {
196211
ProgramStateRef State = C.getState();
197212

213+
// TODO: Do we need these to be typed?
214+
const TypedValueRegion *ObjRegion = nullptr;
215+
198216
if (const auto *ICall = dyn_cast<CXXInstanceCall>(&Call)) {
199-
// TODO: Do we need these to be typed?
200-
const auto *ObjRegion = dyn_cast_or_null<TypedValueRegion>(
217+
ObjRegion = dyn_cast_or_null<TypedValueRegion>(
201218
ICall->getCXXThisVal().getAsRegion());
202-
if (!ObjRegion)
203-
return;
204219

205-
if (Call.isCalled(CStrFn) || Call.isCalled(DataFn)) {
206-
SVal RawPtr = Call.getReturnValue();
207-
if (SymbolRef Sym = RawPtr.getAsSymbol(/*IncludeBaseRegions=*/true)) {
208-
// Start tracking this raw pointer by adding it to the set of symbols
209-
// associated with this container object in the program state map.
220+
// Check [string.require] / second point.
221+
if (isInvalidatingMemberFunction(Call)) {
222+
markPtrSymbolsReleased(Call, State, ObjRegion, C);
223+
return;
224+
}
225+
}
210226

211-
PtrSet::Factory &F = State->getStateManager().get_context<PtrSet>();
212-
const PtrSet *SetPtr = State->get<RawPtrMap>(ObjRegion);
213-
PtrSet Set = SetPtr ? *SetPtr : F.getEmptySet();
214-
assert(C.wasInlined || !Set.contains(Sym));
215-
Set = F.add(Set, Sym);
227+
if (isInnerPointerAccessFunction(Call)) {
216228

217-
State = State->set<RawPtrMap>(ObjRegion, Set);
218-
C.addTransition(State);
219-
}
220-
return;
229+
if (isa<SimpleFunctionCall>(Call)) {
230+
// NOTE: As of now, we only have one free access function: std::data.
231+
// If we add more functions like this in the list, hardcoded
232+
// argument index should be changed.
233+
ObjRegion =
234+
dyn_cast_or_null<TypedValueRegion>(Call.getArgSVal(0).getAsRegion());
221235
}
222236

223-
// Check [string.require] / second point.
224-
if (isInvalidatingMemberFunction(Call)) {
225-
markPtrSymbolsReleased(Call, State, ObjRegion, C);
237+
if (!ObjRegion)
226238
return;
239+
240+
SVal RawPtr = Call.getReturnValue();
241+
if (SymbolRef Sym = RawPtr.getAsSymbol(/*IncludeBaseRegions=*/true)) {
242+
// Start tracking this raw pointer by adding it to the set of symbols
243+
// associated with this container object in the program state map.
244+
245+
PtrSet::Factory &F = State->getStateManager().get_context<PtrSet>();
246+
const PtrSet *SetPtr = State->get<RawPtrMap>(ObjRegion);
247+
PtrSet Set = SetPtr ? *SetPtr : F.getEmptySet();
248+
assert(C.wasInlined || !Set.contains(Sym));
249+
Set = F.add(Set, Sym);
250+
251+
State = State->set<RawPtrMap>(ObjRegion, Set);
252+
C.addTransition(State);
227253
}
254+
255+
return;
228256
}
229257

230258
// Check [string.require] / first point.

‎clang/lib/StaticAnalyzer/Core/SValBuilder.cpp

+8
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,14 @@ SVal SValBuilder::evalBinOp(ProgramStateRef state, BinaryOperator::Opcode op,
423423
return UnknownVal();
424424
}
425425

426+
if (op == BinaryOperatorKind::BO_Cmp) {
427+
// We can't reason about C++20 spaceship operator yet.
428+
//
429+
// FIXME: Support C++20 spaceship operator.
430+
// The main problem here is that the result is not integer.
431+
return UnknownVal();
432+
}
433+
426434
if (Optional<Loc> LV = lhs.getAs<Loc>()) {
427435
if (Optional<Loc> RV = rhs.getAs<Loc>())
428436
return evalBinOpLL(state, op, *LV, *RV, type);

‎clang/test/Analysis/PR47511.cpp

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// RUN: %clang_analyze_cc1 -std=c++20 -w -analyzer-checker=core -verify %s
2+
3+
// expected-no-diagnostics
4+
5+
namespace std {
6+
struct strong_ordering {
7+
int n;
8+
constexpr operator int() const { return n; }
9+
static const strong_ordering equal, greater, less;
10+
};
11+
constexpr strong_ordering strong_ordering::equal = {0};
12+
constexpr strong_ordering strong_ordering::greater = {1};
13+
constexpr strong_ordering strong_ordering::less = {-1};
14+
} // namespace std
15+
16+
void test() {
17+
// no crash
18+
(void)(0 <=> 0);
19+
}

0 commit comments

Comments
 (0)