shylie/llvm-project
1
0
Fork 0
Code Issues Pull Requests Actions 6 Packages Projects Releases Wiki Activity

[analyzer] Fix analyzer warnings on analyzer.

Browse Source 
Write tests for the actual crash that was found. Write comments and refactor
code around 17 style bugs and suppress 3 false positives.

Differential Revision: https://reviews.llvm.org/D66847

llvm-svn: 370246
This commit is contained in:
Artem Dergachev 2019-08-28 18:44:38 +00:00
parent 0909a392f3
commit 630f7daf80
21 changed files with 134 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
View File

@ -169,6 +169,7 @@ public:
Kind getKind() const { return kind; }
template<typename RegionTy> const RegionTy* getAs() const;
template<typename RegionTy> const RegionTy* castAs() const;
virtual bool isBoundable() const { return false; }
@ -1231,6 +1232,11 @@ const RegionTy* MemRegion::getAs() const {
return nullptr;
}
template<typename RegionTy>
const RegionTy* MemRegion::castAs() const {
return cast<RegionTy>(this);
}
//===----------------------------------------------------------------------===//
// MemRegionManager - Factory object for creating regions.
//===----------------------------------------------------------------------===//

27
clang/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
View File

@ -156,14 +156,21 @@ bool WalkAST::containsBadStrlcpyStrlcatPattern(const CallExpr *CE) {
const Expr *DstArg = CE->getArg(0);
const Expr *LenArg = CE->getArg(2);
const auto *DstArgDecl = dyn_cast<DeclRefExpr>(DstArg->IgnoreParenImpCasts());
const auto *LenArgDecl = dyn_cast<DeclRefExpr>(LenArg->IgnoreParenLValueCasts());
const auto *DstArgDRE = dyn_cast<DeclRefExpr>(DstArg->IgnoreParenImpCasts());
const auto *LenArgDRE =
dyn_cast<DeclRefExpr>(LenArg->IgnoreParenLValueCasts());
uint64_t DstOff = 0;
if (isSizeof(LenArg, DstArg))
return false;
// - size_t dstlen = sizeof(dst)
if (LenArgDecl) {
const auto *LenArgVal = dyn_cast<VarDecl>(LenArgDecl->getDecl());
if (LenArgDRE) {
const auto *LenArgVal = dyn_cast<VarDecl>(LenArgDRE->getDecl());
// If it's an EnumConstantDecl instead, then we're missing out on something.
if (!LenArgVal) {
assert(isa<EnumConstantDecl>(LenArgDRE->getDecl()));
return false;
}
if (LenArgVal->getInit())
LenArg = LenArgVal->getInit();
}
@ -177,9 +184,10 @@ bool WalkAST::containsBadStrlcpyStrlcatPattern(const CallExpr *CE) {
// Case when there is pointer arithmetic on the destination buffer
// especially when we offset from the base decreasing the
// buffer length accordingly.
if (!DstArgDecl) {
if (const auto *BE = dyn_cast<BinaryOperator>(DstArg->IgnoreParenImpCasts())) {
DstArgDecl = dyn_cast<DeclRefExpr>(BE->getLHS()->IgnoreParenImpCasts());
if (!DstArgDRE) {
if (const auto *BE =
dyn_cast<BinaryOperator>(DstArg->IgnoreParenImpCasts())) {
DstArgDRE = dyn_cast<DeclRefExpr>(BE->getLHS()->IgnoreParenImpCasts());
if (BE->getOpcode() == BO_Add) {
if ((IL = dyn_cast<IntegerLiteral>(BE->getRHS()->IgnoreParenImpCasts()))) {
DstOff = IL->getValue().getZExtValue();
@ -187,8 +195,9 @@ bool WalkAST::containsBadStrlcpyStrlcatPattern(const CallExpr *CE) {
}
}
}
if (DstArgDecl) {
if (const auto *Buffer = dyn_cast<ConstantArrayType>(DstArgDecl->getType())) {
if (DstArgDRE) {
if (const auto *Buffer =
dyn_cast<ConstantArrayType>(DstArgDRE->getType())) {
ASTContext &C = BR.getContext();
uint64_t BufferLen = C.getTypeSize(Buffer) / 8;
auto RemainingBufferLen = BufferLen - DstOff;

9
clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
View File

@ -204,6 +204,8 @@ void WalkAST::VisitForStmt(ForStmt *FS) {
// Implements: CERT security coding advisory FLP-30.
//===----------------------------------------------------------------------===//
// Returns either 'x' or 'y', depending on which one of them is incremented
// in 'expr', or nullptr if none of them is incremented.
static const DeclRefExpr*
getIncrementedVar(const Expr *expr, const VarDecl *x, const VarDecl *y) {
expr = expr->IgnoreParenCasts();
@ -289,14 +291,15 @@ void WalkAST::checkLoopConditionForFloat(const ForStmt *FS) {
// Does either variable appear in increment?
const DeclRefExpr *drInc = getIncrementedVar(increment, vdLHS, vdRHS);
if (!drInc)
return;
const VarDecl *vdInc = cast<VarDecl>(drInc->getDecl());
assert(vdInc && (vdInc == vdLHS || vdInc == vdRHS));
// Emit the error. First figure out which DeclRefExpr in the condition
// referenced the compared variable.
assert(drInc->getDecl());
const DeclRefExpr *drCond = vdLHS == drInc->getDecl() ? drLHS : drRHS;
const DeclRefExpr *drCond = vdLHS == vdInc ? drLHS : drRHS;
SmallVector<SourceRange, 2> ranges;
SmallString<256> sbuf;

15
clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
View File

@ -394,11 +394,11 @@ static const ObjCObjectPointerType *getMostInformativeDerivedClassImpl(
}
const auto *SuperOfTo =
To->getObjectType()->getSuperClassType()->getAs<ObjCObjectType>();
To->getObjectType()->getSuperClassType()->castAs<ObjCObjectType>();
assert(SuperOfTo);
QualType SuperPtrOfToQual =
C.getObjCObjectPointerType(QualType(SuperOfTo, 0));
const auto *SuperPtrOfTo = SuperPtrOfToQual->getAs<ObjCObjectPointerType>();
const auto *SuperPtrOfTo = SuperPtrOfToQual->castAs<ObjCObjectPointerType>();
if (To->isUnspecialized())
return getMostInformativeDerivedClassImpl(From, SuperPtrOfTo, SuperPtrOfTo,
C);
@ -827,16 +827,15 @@ void DynamicTypePropagation::checkPostObjCMessage(const ObjCMethodCall &M,
if (MessageExpr->getReceiverKind() == ObjCMessageExpr::Class &&
Sel.getAsString() == "class") {
QualType ReceiverType = MessageExpr->getClassReceiver();
const auto *ReceiverClassType = ReceiverType->getAs<ObjCObjectType>();
const auto *ReceiverClassType = ReceiverType->castAs<ObjCObjectType>();
if (!ReceiverClassType->isSpecialized())
return;
QualType ReceiverClassPointerType =
C.getASTContext().getObjCObjectPointerType(
QualType(ReceiverClassType, 0));
if (!ReceiverClassType->isSpecialized())
return;
const auto *InferredType =
ReceiverClassPointerType->getAs<ObjCObjectPointerType>();
assert(InferredType);
ReceiverClassPointerType->castAs<ObjCObjectPointerType>();
State = State->set<MostSpecializedTypeArgsMap>(RetSym, InferredType);
C.addTransition(State);

3
clang/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
View File

@ -567,7 +567,8 @@ void IteratorChecker::checkPostCall(const CallEvent &Call,
if (Func->isOverloadedOperator()) {
const auto Op = Func->getOverloadedOperator();
if (isAssignmentOperator(Op)) {
const auto *InstCall = dyn_cast<CXXInstanceCall>(&Call);
// Overloaded 'operator=' must be a non-static member function.
const auto *InstCall = cast<CXXInstanceCall>(&Call);
if (cast<CXXMethodDecl>(Func)->isMoveAssignmentOperator()) {
handleAssign(C, InstCall->getCXXThisVal(), Call.getOriginExpr(),
Call.getArgSVal(0));

23
clang/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
View File

@ -882,18 +882,17 @@ void NonLocalizedStringChecker::checkPreObjCMessage(const ObjCMethodCall &msg,
void NonLocalizedStringChecker::checkPreCall(const CallEvent &Call,
CheckerContext &C) const {
const Decl *D = Call.getDecl();
if (D && isa<FunctionDecl>(D)) {
const FunctionDecl *FD = dyn_cast<FunctionDecl>(D);
auto formals = FD->parameters();
for (unsigned i = 0,
ei = std::min(unsigned(formals.size()), Call.getNumArgs());
i != ei; ++i) {
if (isAnnotatedAsTakingLocalized(formals[i])) {
auto actual = Call.getArgSVal(i);
if (hasNonLocalizedState(actual, C)) {
reportLocalizationError(actual, Call, C, i + 1);
}
const auto *FD = dyn_cast_or_null<FunctionDecl>(Call.getDecl());
if (!FD)
return;
auto formals = FD->parameters();
for (unsigned i = 0, ei = std::min(static_cast<unsigned>(formals.size()),
Call.getNumArgs()); i != ei; ++i) {
if (isAnnotatedAsTakingLocalized(formals[i])) {
auto actual = Call.getArgSVal(i);
if (hasNonLocalizedState(actual, C)) {
reportLocalizationError(actual, Call, C, i + 1);
}
}
}

4
clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp
View File

@ -91,11 +91,13 @@ PathDiagnosticPieceRef MPIBugReporter::RequestNodeVisitor::VisitNode(
return nullptr;
const Request *const Req = N->getState()->get<RequestMap>(RequestRegion);
assert(Req && "The region must be tracked and alive, given that we've "
"just emitted a report against it");
const Request *const PrevReq =
N->getFirstPred()->getState()->get<RequestMap>(RequestRegion);
// Check if request was previously unused or in a different state.
if ((Req && !PrevReq) || (Req->CurrentState != PrevReq->CurrentState)) {
if (!PrevReq || (Req->CurrentState != PrevReq->CurrentState)) {
IsNodeFound = true;
ProgramPoint P = N->getFirstPred()->getLocation();

13
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
View File

@ -1132,14 +1132,13 @@ ProgramStateRef MallocChecker::addExtentSize(CheckerContext &C,
// Store the extent size for the (symbolic)region
// containing the elements.
Region = Target.getAsRegion()
->getAs<SubRegion>()
->castAs<SubRegion>()
->StripCasts()
->getAs<SubRegion>();
->castAs<SubRegion>();
} else {
ElementCount = svalBuilder.makeIntVal(1, true);
Region = Target.getAsRegion()->getAs<SubRegion>();
Region = Target.getAsRegion()->castAs<SubRegion>();
}
assert(Region);
// Set the region's extent equal to the Size in Bytes.
QualType ElementType = NE->getAllocatedType();
@ -3066,8 +3065,12 @@ PathDiagnosticPieceRef MallocChecker::MallocBugVisitor::VisitNode(
}
}
if (Msg.empty())
if (Msg.empty()) {
// Silence a memory leak warning by MallocChecker in MallocChecker.cpp :)
assert(!StackHint && "Memory leak!");
return nullptr;
}
assert(StackHint);
// Generate the extra diagnostic.

2
clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
View File

@ -183,7 +183,7 @@ public:
QualType CastedType = i->CastedExpr->getType();
if (!CastedType->isPointerType())
continue;
QualType PointeeType = CastedType->getAs<PointerType>()->getPointeeType();
QualType PointeeType = CastedType->getPointeeType();
if (PointeeType->isVoidType())
continue;

10
clang/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp
View File

@ -119,12 +119,12 @@ const MemRegion *PointerArithChecker::getArrayRegion(const MemRegion *Region,
AllocKind &AKind,
CheckerContext &C) const {
assert(Region);
while (Region->getKind() == MemRegion::Kind::CXXBaseObjectRegionKind) {
Region = Region->getAs<CXXBaseObjectRegion>()->getSuperRegion();
while (const auto *BaseRegion = dyn_cast<CXXBaseObjectRegion>(Region)) {
Region = BaseRegion->getSuperRegion();
Polymorphic = true;
}
if (Region->getKind() == MemRegion::Kind::ElementRegionKind) {
Region = Region->getAs<ElementRegion>()->getSuperRegion();
if (const auto *ElemRegion = dyn_cast<ElementRegion>(Region)) {
Region = ElemRegion->getSuperRegion();
}
ProgramStateRef State = C.getState();
@ -137,7 +137,7 @@ const MemRegion *PointerArithChecker::getArrayRegion(const MemRegion *Region,
}
// When the region is symbolic and we do not have any information about it,
// assume that this is an array to avoid false positives.
if (Region->getKind() == MemRegion::Kind::SymbolicRegionKind)
if (isa<SymbolicRegion>(Region))
return Region;
// No AllocKind stored and not symbolic, assume that it points to a single

2
clang/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp
View File

@ -85,7 +85,7 @@ void UndefinedAssignmentChecker::checkBind(SVal location, SVal val,
}
if (const DeclStmt *DS = dyn_cast<DeclStmt>(StoreE)) {
const VarDecl *VD = dyn_cast<VarDecl>(DS->getSingleDecl());
const VarDecl *VD = cast<VarDecl>(DS->getSingleDecl());
ex = VD->getInit();
}

9
clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
View File

@ -260,12 +260,13 @@ static llvm::Optional<DereferenceInfo> dereference(ProgramStateRef State,
break;
}
while (R->getAs<CXXBaseObjectRegion>()) {
while (isa<CXXBaseObjectRegion>(R)) {
NeedsCastBack = true;
if (!isa<TypedValueRegion>(R->getSuperRegion()))
const auto *SuperR = dyn_cast<TypedValueRegion>(R->getSuperRegion());
if (!SuperR)
break;
R = R->getSuperRegion()->getAs<TypedValueRegion>();
R = SuperR;
}
return DereferenceInfo{R, NeedsCastBack, /*IsCyclic*/ false};

3
clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
View File

@ -104,7 +104,8 @@ void VirtualCallChecker::checkPreCall(const CallEvent &Call,
return;
ProgramStateRef State = C.getState();
const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
// Member calls are always represented by a call-expression.
const auto *CE = cast<CallExpr>(Call.getOriginExpr());
if (!isVirtualCall(CE))
return;

2
clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
View File

@ -91,7 +91,7 @@ parseAssignment(const Stmt *S) {
} else if (auto PD = dyn_cast_or_null<DeclStmt>(S)) {
// Initialization
assert(PD->isSingleDecl() && "We process decls one by one");
VD = dyn_cast_or_null<VarDecl>(PD->getSingleDecl());
VD = cast<VarDecl>(PD->getSingleDecl());
RHS = VD->getAnyInitializer();
}

3
clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
View File

@ -850,8 +850,7 @@ VisitOffsetOfExpr(const OffsetOfExpr *OOE,
if (OOE->EvaluateAsInt(Result, getContext())) {
APSInt IV = Result.Val.getInt();
assert(IV.getBitWidth() == getContext().getTypeSize(OOE->getType()));
assert(OOE->getType()->isBuiltinType());
assert(OOE->getType()->getAs<BuiltinType>()->isInteger());
assert(OOE->getType()->castAs<BuiltinType>()->isInteger());
assert(IV.isSigned() == OOE->getType()->isSignedIntegerType());
SVal X = svalBuilder.makeIntVal(IV);
B.generateNode(OOE, Pred,

5
clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
View File

@ -803,9 +803,8 @@ void ExprEngine::VisitCXXNewExpr(const CXXNewExpr *CNE, ExplodedNode *Pred,
if (CNE->isArray()) {
// FIXME: allocating an array requires simulating the constructors.
// For now, just return a symbolicated region.
if (const SubRegion *NewReg =
dyn_cast_or_null<SubRegion>(symVal.getAsRegion())) {
QualType ObjTy = CNE->getType()->getAs<PointerType>()->getPointeeType();
if (const auto *NewReg = cast_or_null<SubRegion>(symVal.getAsRegion())) {
QualType ObjTy = CNE->getType()->getPointeeType();
const ElementRegion *EleReg =
getStoreManager().GetElementZeroRegion(NewReg, ObjTy);
Result = loc::MemRegionVal(EleReg);

3
clang/lib/StaticAnalyzer/Core/MemRegion.cpp
View File

@ -1075,7 +1075,7 @@ MemRegionManager::getCXXBaseObjectRegion(const CXXRecordDecl *RD,
const SubRegion *Super,
bool IsVirtual) {
if (isa<TypedValueRegion>(Super)) {
assert(isValidBaseClass(RD, dyn_cast<TypedValueRegion>(Super), IsVirtual));
assert(isValidBaseClass(RD, cast<TypedValueRegion>(Super), IsVirtual));
(void)&isValidBaseClass;
if (IsVirtual) {
@ -1426,6 +1426,7 @@ static RegionOffset calculateOffset(const MemRegion *R) {
case MemRegion::FieldRegionKind: {
const auto *FR = cast<FieldRegion>(R);
R = FR->getSuperRegion();
assert(R);
const RecordDecl *RD = FR->getDecl()->getParent();
if (RD->isUnion() || !RD->isCompleteDefinition()) {

5
clang/lib/StaticAnalyzer/Core/RegionStore.cpp
View File

@ -2291,8 +2291,7 @@ RegionBindingsRef RegionStoreManager::bindVector(RegionBindingsConstRef B,
const TypedValueRegion* R,
SVal V) {
QualType T = R->getValueType();
assert(T->isVectorType());
const VectorType *VT = T->getAs<VectorType>(); // Use getAs for typedefs.
const VectorType *VT = T->castAs<VectorType>(); // Use castAs for typedefs.
// Handle lazy compound values and symbolic values.
if (V.getAs<nonloc::LazyCompoundVal>() || V.getAs<nonloc::SymbolVal>())
@ -2377,7 +2376,7 @@ RegionBindingsRef RegionStoreManager::bindStruct(RegionBindingsConstRef B,
QualType T = R->getValueType();
assert(T->isStructureOrClassType());
const RecordType* RT = T->getAs<RecordType>();
const RecordType* RT = T->castAs<RecordType>();
const RecordDecl *RD = RT->getDecl();
if (!RD->isCompleteDefinition())

19
clang/test/Analysis/cstring-syntax-weird.c Normal file
View File

@ -0,0 +1,19 @@
// RUN: %clang_analyze_cc1 -w -analyzer-checker=unix.cstring.BadSizeArg \
// RUN: -verify %s
// expected-no-diagnostics
typedef __SIZE_TYPE__ size_t;
// The last parameter is normally size_t but the test is about the abnormal
// situation when it's not a size_t.
size_t strlcpy(char *, const char *, int);
enum WeirdDecl {
AStrangeWayToSpecifyStringLengthCorrectly = 10UL,
AStrangeWayToSpecifyStringLengthIncorrectly = 5UL
};
void testWeirdDecls(const char *src) {
char dst[10];
strlcpy(dst, src, AStrangeWayToSpecifyStringLengthCorrectly); // no-crash
strlcpy(dst, src, AStrangeWayToSpecifyStringLengthIncorrectly); // no-crash // no-warning
}

17
clang/test/Analysis/cstring-syntax-weird2.c Normal file
View File

@ -0,0 +1,17 @@
// RUN: %clang_analyze_cc1 -w -analyzer-checker=unix.cstring.BadSizeArg \
// RUN: -verify %s
// expected-no-diagnostics
typedef __SIZE_TYPE__ size_t;
// The last parameter is normally size_t but the test is about the abnormal
// situation when it's not a size_t.
size_t strlcpy(char *, const char *, void (*)());
void foo();
void testWeirdDecls(const char *src) {
char dst[10];
strlcpy(dst, src, foo); // no-crash
strlcpy(dst, src, &foo); // no-crash
}

19
clang/test/Analysis/cstring-syntax.c
View File

@ -1,7 +1,18 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=unix.cstring.BadSizeArg -analyzer-store=region -Wno-strncat-size -Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument -Wno-sizeof-pointer-memaccess -verify %s
// RUN: %clang_analyze_cc1 -triple armv7-a15-linux -analyzer-checker=unix.cstring.BadSizeArg -analyzer-store=region -Wno-strncat-size -Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument -Wno-sizeof-pointer-memaccess -verify %s
// RUN: %clang_analyze_cc1 -triple aarch64_be-none-linux-gnu -analyzer-checker=unix.cstring.BadSizeArg -analyzer-store=region -Wno-strncat-size -Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument -Wno-sizeof-pointer-memaccess -verify %s
// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -analyzer-checker=unix.cstring.BadSizeArg -analyzer-store=region -Wno-strncat-size -Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument -Wno-sizeof-pointer-memaccess -verify %s
// RUN: %clang_analyze_cc1 -analyzer-checker=unix.cstring.BadSizeArg -verify %s\
// RUN: -Wno-strncat-size -Wno-sizeof-pointer-memaccess \
// RUN: -Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument
// RUN: %clang_analyze_cc1 -analyzer-checker=unix.cstring.BadSizeArg -verify %s\
// RUN: -Wno-strncat-size -Wno-sizeof-pointer-memaccess \
// RUN: -Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument\
// RUN: -triple armv7-a15-linux
// RUN: %clang_analyze_cc1 -analyzer-checker=unix.cstring.BadSizeArg -verify %s\
// RUN: -Wno-strncat-size -Wno-sizeof-pointer-memaccess \
// RUN: -Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument\
// RUN: -triple aarch64_be-none-linux-gnu
// RUN: %clang_analyze_cc1 -analyzer-checker=unix.cstring.BadSizeArg -verify %s\
// RUN: -Wno-strncat-size -Wno-sizeof-pointer-memaccess \
// RUN: -Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument\
// RUN: -triple i386-apple-darwin10
typedef __SIZE_TYPE__ size_t;
char *strncat(char *, const char *, size_t);