[PATCH] D107339: [analyzer] Retrieve a character from StringLiteral as an initializer for constant arrays.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 366083.
ASDenysPetrov added a comment.

Rebased.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107339/new/

https://reviews.llvm.org/D107339

Files:
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp


Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -443,6 +443,8 @@
   Optional<SVal> getSValFromInitListExprByIndex(const InitListExpr *ILE,
                                                 const llvm::APSInt &Idx,
                                                 QualType ElemT);
+  SVal getSValFromStringLiteralByIndex(const StringLiteral *SL,
+                                       const llvm::APSInt &Idx, QualType ElemT);
 
 public: // Part of public interface to class.
 
@@ -1649,7 +1651,9 @@
   if (const auto *ILE = dyn_cast<InitListExpr>(Init))
     return getSValFromInitListExprByIndex(ILE, Idx, ElemT);
 
-  // FIXME: Handle StringLiteral.
+  // Handle StringLiteral.
+  if (const auto *SL = dyn_cast<StringLiteral>(Init))
+    return getSValFromStringLiteralByIndex(SL, Idx, ElemT);
 
   // FIXME: Handle CompoundLiteralExpr.
 
@@ -1679,6 +1683,22 @@
   return svalBuilder.getConstantVal(E);
 }
 
+SVal RegionStoreManager::getSValFromStringLiteralByIndex(
+    const StringLiteral *SL, const llvm::APSInt &Idx, QualType ElemT) {
+  assert(SL && "StringLiteral should not be null");
+  // If index is out of bounds, return Undef.
+  const int64_t I = Idx.getExtValue();
+  if (Idx.isSigned() && I < 0)
+    return UndefinedVal();
+  // Technically, only i == length is guaranteed to be null.
+  // However, such overflows should be caught before reaching this point;
+  // the only time such an access would be made is if a string literal was
+  // used to initialize a larger array.
+  uint32_t Code =
+      (static_cast<uint64_t>(I) >= SL->getLength()) ? 0 : SL->getCodeUnit(I);
+  return svalBuilder.makeIntVal(Code, ElemT);
+}
+
 SVal RegionStoreManager::getBindingForElement(RegionBindingsConstRef B,
                                               const ElementRegion* R) {
   // Check if the region has a binding.
@@ -1695,21 +1715,10 @@
     if (!Ctx.hasSameUnqualifiedType(T, R->getElementType()))
       return UnknownVal();
 
-    const StringLiteral *Str = StrR->getStringLiteral();
     SVal Idx = R->getIndex();
     if (Optional<nonloc::ConcreteInt> CI = Idx.getAs<nonloc::ConcreteInt>()) {
-      int64_t i = CI->getValue().getSExtValue();
-      // Abort on string underrun.  This can be possible by arbitrary
-      // clients of getBindingForElement().
-      if (i < 0)
-        return UndefinedVal();
-      int64_t length = Str->getLength();
-      // Technically, only i == length is guaranteed to be null.
-      // However, such overflows should be caught before reaching this point;
-      // the only time such an access would be made is if a string literal was
-      // used to initialize a larger array.
-      char c = (i >= length) ? '\0' : Str->getCodeUnit(i);
-      return svalBuilder.makeIntVal(c, T);
+      const StringLiteral *Str = StrR->getStringLiteral();
+      return getSValFromStringLiteralByIndex(Str, CI->getValue(), T);
     }
   } else if (isa<ElementRegion>(superR) || isa<VarRegion>(superR)) {
     const VarRegion *VR = nullptr;
@@ -2237,10 +2246,11 @@
 RegionBindingsRef RegionStoreManager::bindArray(RegionBindingsConstRef B,
                                                 const TypedValueRegion *R,
                                                 SVal Init) {
-  // Ignore binding `InitListExpr` to arrays of const type,
-  // since we can directly retrieve values from initializer using
+  // Ignore binding `InitListExpr` and `StringLiteral` to arrays of const type,
+  // since we can directly retrieve values from initializers using
   // `getConstantValFromConstArrayInitializer`.For example:
   //   const int arr[42] = { 1, 2, 3 };
+  //   const char arr[42] = "123";
   // The init values of this array will never change, so we don't have to
   // store them additionally in the RegionStore.
   if (const auto *VR = dyn_cast<VarRegion>(R)) {
@@ -2248,9 +2258,9 @@
     // Ignore only arrays which values can't change.
     if (VD->getType().isConstQualified()) {
       const Expr *Init = VD->getAnyInitializer();
-      // FIXME: Ignore `StringLiteral` and `CompoundLiteralExpr` as well when
-      // `getConstantValFromConstArrayInitializer` supports them.
-      if (isa_and_nonnull<InitListExpr>(Init))
+      // FIXME: Ignore `CompoundLiteralExpr` as well when
+      // `getConstantValFromConstArrayInitializer` supports it.
+      if (Init && (isa<InitListExpr>(Init) || isa<StringLiteral>(Init)))
         return B;
     }
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D107339.366083.patch
Type: text/x-patch
Size: 4592 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210812/944a5d51/attachment-0001.bin>