Skip to content

[analyzer] Fix crash in Stream checker when using void pointers #97199

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1034,16 +1034,16 @@ 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;
return {};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit strange that here you return {}, while at the end of the function QualType{} is explicitly specified.

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 {};
}

static std::optional<NonLoc> getStartIndex(SValBuilder &SVB,
Expand Down Expand Up @@ -1073,7 +1073,8 @@ 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);
std::optional<SVal> StartElementIndex =
getStartIndex(C.getSValBuilder(), Buffer);

Expand All @@ -1086,18 +1087,20 @@ 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();
if (ElemSizeInChars == 0)
return nullptr;

bool IncompleteLastElement = (NumBytesRead % ElemSizeInChars) != 0;
int64_t NumCompleteOrIncompleteElementsRead =
NumBytesRead / ElemSizeInChars + IncompleteLastElement;

constexpr int MaxInvalidatedElementsLimit = 64;
if (NumCompleteOrIncompleteElementsRead <= MaxInvalidatedElementsLimit) {
return escapeByStartIndexAndCount(State, Call, C.blockCount(), Buffer,
*ElemTy, *StartIndexVal,
ElemTy, *StartIndexVal,
NumCompleteOrIncompleteElementsRead);
}
}
Expand Down
45 changes: 45 additions & 0 deletions clang/test/Analysis/stream.c
Original file line number Diff line number Diff line change
Expand Up @@ -453,3 +453,48 @@ 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
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
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);
}
Loading