[clang] [analyzer] Refine invalidation caused by `fread` (PR #93408)

Balazs Benics via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 6 02:01:14 PDT 2024


================
@@ -907,6 +945,73 @@ void StreamChecker::preWrite(const FnDescription *Desc, const CallEvent &Call,
   C.addTransition(State);
 }
 
+static std::optional<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;
+}
+
+static std::optional<NonLoc> getStartIndex(SValBuilder &SVB,
+                                           const MemRegion *R) {
+  if (!R)
+    return std::nullopt;
+
+  auto Zero = [&SVB] {
+    BasicValueFactory &BVF = SVB.getBasicValueFactory();
+    return nonloc::ConcreteInt(BVF.getIntValue(0, /*isUnsigned=*/false));
+  };
+
+  if (const auto *ER = dyn_cast<ElementRegion>(R))
+    return ER->getIndex();
+  if (const auto *TR = dyn_cast<TypedValueRegion>(R))
+    return Zero();
+  if (const auto *SR = dyn_cast<SymbolicRegion>(R))
+    return Zero();
+  return std::nullopt;
+}
+
+static ProgramStateRef
+tryToInvalidateFReadBufferByElements(ProgramStateRef State, CheckerContext &C,
+                                     const CallEvent &Call, NonLoc SizeVal,
+                                     NonLoc NMembVal) {
+  // Try to invalidate the individual elements.
+  const auto *Buffer =
+      dyn_cast_or_null<SubRegion>(Call.getArgSVal(0).getAsRegion());
+
+  std::optional<QualType> ElemTy = getPointeeType(Buffer);
+  std::optional<SVal> StartElementIndex =
+      getStartIndex(C.getSValBuilder(), Buffer);
+
+  // Drop the outermost ElementRegion to get the buffer.
+  if (const auto *ER = dyn_cast_or_null<ElementRegion>(Buffer))
+    Buffer = dyn_cast<SubRegion>(ER->getSuperRegion());
+
+  std::optional<int64_t> CountVal = getKnownValue(State, NMembVal);
+  std::optional<int64_t> Size = getKnownValue(State, SizeVal);
+  std::optional<int64_t> StartIndexVal =
+      getKnownValue(State, StartElementIndex.value_or(UnknownVal()));
+
+  if (ElemTy && CountVal && Size && StartIndexVal) {
+    int64_t NumBytesRead = Size.value() * CountVal.value();
+    int64_t ElemSizeInChars =
+        C.getASTContext().getTypeSizeInChars(*ElemTy).getQuantity();
+    bool DivisibleAccessSpan = (NumBytesRead % ElemSizeInChars) == 0;
+    int64_t NumElementsRead = NumBytesRead / ElemSizeInChars;
+    constexpr int MaxInvalidatedElementsLimit = 64;
+    if (DivisibleAccessSpan && NumElementsRead <= MaxInvalidatedElementsLimit) {
----------------
steakhal wrote:

Applied a patch to this code such that the last partial element is considered as a full access.
This is indeed better than doing a fallback and invalidating the full buffer.
Also modified the test to demonstrate this, but there is yet another bug in the current Store:
If we read from a byte offset of an element, we still get unknown as a result: `int buffer[2] = {2,3}` then reading from `((char*)buffer)[1]` results in unknown. [Compiler explorer](https://godbolt.org/z/dTKGnYo9W).

https://github.com/llvm/llvm-project/pull/93408


More information about the cfe-commits mailing list