[clang] 2e81f7d - [analyzer] Fix crash in Stream checker when using void pointers (#97199)

[via cfe-commits]

Author: Balazs Benics
Date: 2024-07-01T17:33:44+02:00
New Revision: 2e81f7db1f1c161a1683bcad55e0c04677a17a9d

URL: https://github.com/llvm/llvm-project/commit/2e81f7db1f1c161a1683bcad55e0c04677a17a9d
DIFF: https://github.com/llvm/llvm-project/commit/2e81f7db1f1c161a1683bcad55e0c04677a17a9d.diff

LOG: [analyzer] Fix crash in Stream checker when using void pointers (#97199)

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

Added: 
    

Modified: 
    clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
    clang/test/Analysis/stream.c

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 9aee7f952ad2d..e8d538388e56c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -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 {};
   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,
@@ -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);
 
@@ -1086,10 +1087,12 @@ 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;
@@ -1097,7 +1100,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..c924cbd36f759 100644
--- a/clang/test/Analysis/stream.c
+++ b/clang/test/Analysis/stream.c
@@ -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);
+}