phyBrackets created this revision.
Herald added subscribers: ASDenysPetrov, martong, dkrupp, donat.nagy,
Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
phyBrackets requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Few weeks back I was experimenting with reading the uninitialized values from
src , which is actually a bug but the CSA seems to give up at that point . I
was curious about that and I pinged @steakhal on the discord and according to
him this seems to be a genuine issue and needs to be fix. So I goes with fixing
this bug and thanks to @steakhal who help me creating this patch. This feature
seems to break some tests but this was the genuine problem and the broken tests
also needs to fix in certain manner. I add a test but yeah we need more
tests,I'll try to add more tests.Thanks
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D120489
Files:
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
clang/test/Analysis/bstring.c
Index: clang/test/Analysis/bstring.c
===================================================================
--- clang/test/Analysis/bstring.c
+++ clang/test/Analysis/bstring.c
@@ -70,6 +70,11 @@
#endif /* VARIANT */
+void top(char *dst) {
+ char buf[10];
+ memcpy(dst, buf, 10); // expected-warning{{Bytes string function accesses uninitialized/garbage values}}
+ (void)buf;
+}
void memcpy0 () {
char src[] = {1, 2, 3, 4};
@@ -297,9 +302,12 @@
int dst[5] = {0};
int *p;
- p = mempcpy(dst, src, 4 * sizeof(int));
+ p = mempcpy(dst, src, 4 * sizeof(int)); // expected-warning{{Bytes string function accesses uninitialized/garbage values}}
+ // FIXME: This behaviour is actually Unexpected and needs to be fix,
+ // mempcpy seems to consider the src buffered byte as uninitialized
+ // and returning undef which is actually not the case It should return something like Unknown .
- clang_analyzer_eval(p == &dst[4]); // expected-warning{{TRUE}}
+ clang_analyzer_eval(p == &dst[4]); // no-warning (above is fatal)
}
struct st {
@@ -314,9 +322,10 @@
struct st *p2;
p1 = (&s2) + 1;
- p2 = mempcpy(&s2, &s1, sizeof(struct st));
-
- clang_analyzer_eval(p1 == p2); // expected-warning{{TRUE}}
+ p2 = mempcpy(&s2, &s1, sizeof(struct st)); // expected-warning{{Bytes string function accesses uninitialized/garbage values}}
+ // FIXME: It seems same as mempcpy14() case.
+
+ clang_analyzer_eval(p1 == p2); // no-warning (above is fatal)
}
void mempcpy16() {
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -80,7 +80,7 @@
check::RegionChanges
> {
mutable std::unique_ptr<BugType> BT_Null, BT_Bounds, BT_Overlap,
- BT_NotCString, BT_AdditionOverflow;
+ BT_NotCString, BT_AdditionOverflow, BT_UninitRead;
mutable const char *CurrentFunctionDescription;
@@ -92,11 +92,13 @@
DefaultBool CheckCStringOutOfBounds;
DefaultBool CheckCStringBufferOverlap;
DefaultBool CheckCStringNotNullTerm;
+ DefaultBool CheckCStringUninitializedRead;
CheckerNameRef CheckNameCStringNullArg;
CheckerNameRef CheckNameCStringOutOfBounds;
CheckerNameRef CheckNameCStringBufferOverlap;
CheckerNameRef CheckNameCStringNotNullTerm;
+ CheckerNameRef CheckNameCStringUninitializedRead;
};
CStringChecksFilter Filter;
@@ -257,6 +259,8 @@
void emitNotCStringBug(CheckerContext &C, ProgramStateRef State,
const Stmt *S, StringRef WarningMsg) const;
void emitAdditionOverflowBug(CheckerContext &C, ProgramStateRef State) const;
+ void emitUninitializedRead(CheckerContext &C, ProgramStateRef State,
+ const Expr *E) const;
ProgramStateRef checkAdditionOverflow(CheckerContext &C,
ProgramStateRef state,
@@ -368,6 +372,15 @@
return nullptr;
}
+ // Ensure that we wouldn't read uninitialized value.
+ if (Access == AccessKind::read) {
+ if (StInBound->getSVal(ER).isUndef()) {
+ llvm::errs() << "Reading from " << ER << "\n";
+ emitUninitializedRead(C, StInBound, Buffer.Expression);
+ return nullptr;
+ }
+ }
+
// Array bound check succeeded. From this point forward the array bound
// should always succeed.
return StInBound;
@@ -421,6 +434,7 @@
SVal BufEnd =
svalBuilder.evalBinOpLN(State, BO_Add, *BufLoc, LastOffset, PtrTy);
+ State = CheckLocation(C, State, Buffer, BufStart, Access);
State = CheckLocation(C, State, Buffer, BufEnd, Access);
// If the buffer isn't large enough, abort.
@@ -580,6 +594,27 @@
}
}
+void CStringChecker::emitUninitializedRead(CheckerContext &C,
+ ProgramStateRef State,
+ const Expr *E) const {
+ if (ExplodedNode *N = C.generateErrorNode(State)) {
+ State->dump();
+ const char *Msg =
+ "Bytes string function accesses uninitialized/garbage values";
+ if (!BT_UninitRead)
+ BT_UninitRead.reset(
+ new BuiltinBug(Filter.CheckNameCStringUninitializedRead,
+ "Accessing unitialized/garbage values", Msg));
+
+ BuiltinBug *BT = static_cast<BuiltinBug *>(BT_UninitRead.get());
+
+ auto Report = std::make_unique<PathSensitiveBugReport>(*BT, Msg, N);
+ Report->addRange(E->getSourceRange());
+ bugreporter::trackExpressionValue(N, E, *Report);
+ C.emitReport(std::move(Report));
+ }
+}
+
void CStringChecker::emitOutOfBoundsBug(CheckerContext &C,
ProgramStateRef State, const Stmt *S,
StringRef WarningMsg) const {
@@ -2460,3 +2495,4 @@
REGISTER_CHECKER(CStringOutOfBounds)
REGISTER_CHECKER(CStringBufferOverlap)
REGISTER_CHECKER(CStringNotNullTerm)
+REGISTER_CHECKER(CStringUninitializedRead)
\ No newline at end of file
Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -471,7 +471,12 @@
HelpText<"Check for arguments which are not null-terminating strings">,
Dependencies<[CStringModeling]>,
Documentation<HasAlphaDocumentation>;
-
+
+def CStringUninitializedRead : Checker<"UninitializedRead">,
+ HelpText<"Checks if the string manipulation function would read uninitialized bytes">,
+ Dependencies<[CStringModeling]>,
+ Documentation<HasAlphaDocumentation>;
+
} // end "alpha.unix.cstring"
let ParentPackage = Unix in {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
