[clang] [analyzer] Fix crash in Stream checker when using void pointers (PR #97199)
Balazs Benics via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 1 06:43:29 PDT 2024
https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/97199
>From 81910b6d8139868304c87784416e087e2aea9f7a Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs at gmail.com>
Date: Sun, 30 Jun 2024 11:32:14 +0200
Subject: [PATCH 1/2] [analyzer] Fix crash in Stream checker when using void
pointers
We can get zero type size (thus div by zero crash) if the region is for
a 'void*' pointer.
In this patch, let's just override the void type with a char type to
avoid the crash.
Fixes
https://github.com/llvm/llvm-project/pull/93408#issuecomment-2189766510
---
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 39 ++++++++++++-------
clang/test/Analysis/stream.c | 13 +++++++
2 files changed, 38 insertions(+), 14 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 9aee7f952ad2d..0d1d50d19f4c4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -1034,16 +1034,19 @@ void StreamChecker::preWrite(const FnDescription *Desc, const CallEvent &Call,
C.addTransition(State);
}
-static std::optional<QualType> getPointeeType(const MemRegion *R) {
+static QualType getPointeeType(const MemRegion *R) {
if (!R)
- return std::nullopt;
- if (const auto *ER = dyn_cast<ElementRegion>(R))
- return ER->getElementType();
- if (const auto *TR = dyn_cast<TypedValueRegion>(R))
- return TR->getValueType();
- if (const auto *SR = dyn_cast<SymbolicRegion>(R))
- return SR->getPointeeStaticType();
- return std::nullopt;
+ return {};
+ QualType Ty = [R] {
+ if (const auto *ER = dyn_cast<ElementRegion>(R))
+ return ER->getElementType();
+ if (const auto *TR = dyn_cast<TypedValueRegion>(R))
+ return TR->getValueType();
+ if (const auto *SR = dyn_cast<SymbolicRegion>(R))
+ return SR->getPointeeStaticType();
+ return QualType{};
+ }();
+ return !Ty.isNull() ? Ty->getCanonicalTypeUnqualified() : QualType{};
}
static std::optional<NonLoc> getStartIndex(SValBuilder &SVB,
@@ -1073,7 +1076,16 @@ tryToInvalidateFReadBufferByElements(ProgramStateRef State, CheckerContext &C,
const auto *Buffer =
dyn_cast_or_null<SubRegion>(Call.getArgSVal(0).getAsRegion());
- std::optional<QualType> ElemTy = getPointeeType(Buffer);
+ const ASTContext &Ctx = C.getASTContext();
+
+ QualType ElemTy = getPointeeType(Buffer);
+
+ // Consider pointer to void as a pointer to char buffer such that it has a
+ // non-zero type size.
+ if (!ElemTy.isNull() && ElemTy == Ctx.VoidTy) {
+ ElemTy = Ctx.CharTy;
+ }
+
std::optional<SVal> StartElementIndex =
getStartIndex(C.getSValBuilder(), Buffer);
@@ -1086,10 +1098,9 @@ tryToInvalidateFReadBufferByElements(ProgramStateRef State, CheckerContext &C,
std::optional<int64_t> StartIndexVal =
getKnownValue(State, StartElementIndex.value_or(UnknownVal()));
- if (ElemTy && CountVal && Size && StartIndexVal) {
+ if (!ElemTy.isNull() && CountVal && Size && StartIndexVal) {
int64_t NumBytesRead = Size.value() * CountVal.value();
- int64_t ElemSizeInChars =
- C.getASTContext().getTypeSizeInChars(*ElemTy).getQuantity();
+ int64_t ElemSizeInChars = Ctx.getTypeSizeInChars(ElemTy).getQuantity();
bool IncompleteLastElement = (NumBytesRead % ElemSizeInChars) != 0;
int64_t NumCompleteOrIncompleteElementsRead =
NumBytesRead / ElemSizeInChars + IncompleteLastElement;
@@ -1097,7 +1108,7 @@ tryToInvalidateFReadBufferByElements(ProgramStateRef State, CheckerContext &C,
constexpr int MaxInvalidatedElementsLimit = 64;
if (NumCompleteOrIncompleteElementsRead <= MaxInvalidatedElementsLimit) {
return escapeByStartIndexAndCount(State, Call, C.blockCount(), Buffer,
- *ElemTy, *StartIndexVal,
+ ElemTy, *StartIndexVal,
NumCompleteOrIncompleteElementsRead);
}
}
diff --git a/clang/test/Analysis/stream.c b/clang/test/Analysis/stream.c
index db03d90cd8af4..0869b1b325172 100644
--- a/clang/test/Analysis/stream.c
+++ b/clang/test/Analysis/stream.c
@@ -453,3 +453,16 @@ void getline_buffer_size_negative() {
free(buffer);
fclose(file);
}
+
+void gh_93408_regression(void *buffer) {
+ FILE *f = fopen("/tmp/foo.txt", "r");
+ fread(buffer, 1, 1, f); // expected-warning {{Stream pointer might be NULL}} no-crash: void is treated as char.
+ fclose(f);
+}
+
+typedef void VOID;
+void gh_93408_regression_typedef(VOID *buffer) {
+ FILE *f = fopen("/tmp/foo.txt", "r");
+ fread(buffer, 1, 1, f); // expected-warning {{Stream pointer might be NULL}} no-crash: VOID is treated as char.
+ fclose(f);
+}
>From 8e08e4ac714ef141fef26f66852640e816bbd480 Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs at gmail.com>
Date: Mon, 1 Jul 2024 15:43:08 +0200
Subject: [PATCH 2/2] Prefer directly checking against 0
---
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 11 ++----
clang/test/Analysis/stream.c | 36 +++++++++++++++++--
2 files changed, 37 insertions(+), 10 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 0d1d50d19f4c4..530d971fc5305 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -1077,15 +1077,7 @@ tryToInvalidateFReadBufferByElements(ProgramStateRef State, CheckerContext &C,
dyn_cast_or_null<SubRegion>(Call.getArgSVal(0).getAsRegion());
const ASTContext &Ctx = C.getASTContext();
-
QualType ElemTy = getPointeeType(Buffer);
-
- // Consider pointer to void as a pointer to char buffer such that it has a
- // non-zero type size.
- if (!ElemTy.isNull() && ElemTy == Ctx.VoidTy) {
- ElemTy = Ctx.CharTy;
- }
-
std::optional<SVal> StartElementIndex =
getStartIndex(C.getSValBuilder(), Buffer);
@@ -1101,6 +1093,9 @@ tryToInvalidateFReadBufferByElements(ProgramStateRef State, CheckerContext &C,
if (!ElemTy.isNull() && CountVal && Size && StartIndexVal) {
int64_t NumBytesRead = Size.value() * CountVal.value();
int64_t ElemSizeInChars = Ctx.getTypeSizeInChars(ElemTy).getQuantity();
+ if (ElemSizeInChars == 0)
+ return nullptr;
+
bool IncompleteLastElement = (NumBytesRead % ElemSizeInChars) != 0;
int64_t NumCompleteOrIncompleteElementsRead =
NumBytesRead / ElemSizeInChars + IncompleteLastElement;
diff --git a/clang/test/Analysis/stream.c b/clang/test/Analysis/stream.c
index 0869b1b325172..c924cbd36f759 100644
--- a/clang/test/Analysis/stream.c
+++ b/clang/test/Analysis/stream.c
@@ -456,13 +456,45 @@ void getline_buffer_size_negative() {
void gh_93408_regression(void *buffer) {
FILE *f = fopen("/tmp/foo.txt", "r");
- fread(buffer, 1, 1, f); // expected-warning {{Stream pointer might be NULL}} no-crash: void is treated as char.
+ fread(buffer, 1, 1, f); // expected-warning {{Stream pointer might be NULL}} no-crash
fclose(f);
}
typedef void VOID;
void gh_93408_regression_typedef(VOID *buffer) {
FILE *f = fopen("/tmp/foo.txt", "r");
- fread(buffer, 1, 1, f); // expected-warning {{Stream pointer might be NULL}} no-crash: VOID is treated as char.
+ fread(buffer, 1, 1, f); // expected-warning {{Stream pointer might be NULL}} no-crash
+ fclose(f);
+}
+
+struct FAM {
+ int data;
+ int tail[];
+};
+
+struct FAM0 {
+ int data;
+ int tail[0];
+};
+
+void gh_93408_regression_FAM(struct FAM *p) {
+ FILE *f = fopen("/tmp/foo.txt", "r");
+ fread(p->tail, 1, 1, f); // expected-warning {{Stream pointer might be NULL}}
+ fclose(f);
+}
+
+void gh_93408_regression_FAM0(struct FAM0 *p) {
+ FILE *f = fopen("/tmp/foo.txt", "r");
+ fread(p->tail, 1, 1, f); // expected-warning {{Stream pointer might be NULL}}
+ fclose(f);
+}
+
+struct ZeroSized {
+ int data[0];
+};
+
+void gh_93408_regression_ZeroSized(struct ZeroSized *buffer) {
+ FILE *f = fopen("/tmp/foo.txt", "r");
+ fread(buffer, 1, 1, f); // expected-warning {{Stream pointer might be NULL}} no-crash
fclose(f);
}
More information about the cfe-commits
mailing list