feat(stack-buffer): bound stack array indexes with ScalarEvolution on a promoted copy

CHANGELOG.md

@@ -4,6 +4,21 @@ Notable changes are grouped by release. See [all releases](https://github.com/Co
 
 ## Unreleased
 
+- An index-bounds pilot bounds `buf[i]` on a stack array, indexed directly rather than through
+  a pointer, with ScalarEvolution. It works on a copy of the function whose local variables live
+  in registers; the analyzed IR is unchanged. An index the copy proves in bounds no longer gets a
+  report, such as `buf[i + 1]` in `for (i = 0; i < 10; i += 2)`. An index that leaves the array
+  on every run of the function that returns is a Warning under the new rules
+  `StackBufferOverflow.EveryRun` and `NegativeStackIndex.EveryRun`, and its message no longer
+  says "potential". A range that only may leave the array is still a "potential" Warning, now
+  also found when a parameter guarded before the loop sets its count. An access in a loop that
+  runs no iteration gets the "this access appears unreachable" note. Where the copy does not
+  bound the index, the existing result stays.
+- Stack buffer overflow checks no longer take the store of an element's address, such as
+  `char* end = &buf[10]` or the end of a range-based `for`, for a write to that element.
+  Forming a pointer one past the end is valid C and C++. A write through such a pointer kept in
+  a local variable is not reported either: it used to be reported only by that mistake, on the
+  line that formed the address.
 - VLA sizes, local stack sizes and the "this access appears unreachable" note on buffer
   overflows read a local variable from the store that last wrote it, as MemorySSA finds it, and
   apply casts to it. They used to take the last constant stored anywhere in the function. A

CMakeLists.txt

@@ -92,6 +92,7 @@ set(STACK_ANALYZER_SOURCES
     src/analysis/NullDerefAnalysis.cpp
     src/analysis/OOBReadAnalysis.cpp
     src/analysis/ParameterDebugBinding.cpp
+    src/analysis/PromotedValues.cpp
     src/analysis/ResourceLifetimeAnalysis.cpp
     src/analysis/Reachability.cpp
     src/analysis/SizeMinusKWrites.cpp
@@ -227,6 +228,7 @@ else()
       core
       irreader
       support
+      transformutils
     )
     target_link_libraries(stack_usage_analyzer_lib
         PUBLIC

include/analysis/FunctionFacts.hpp

@@ -31,8 +31,7 @@ namespace ctrace::stack::analysis
     /// ScalarEvolution is deliberately absent. The analyzer reads -O0 IR, where a loop
     /// counter lives in a multiply-assigned alloca that SCEV cannot see through, and the
     /// arithmetic it would bound carries nsw, which @ref computeIntRanges refuses on purpose.
-    /// Wiring it in changed no result on any fixture; add it back only alongside a pass that
-    /// promotes locals to registers.
+    /// @ref PromotedValues runs it on a promoted, sanitized copy of the module instead.
     class FunctionFacts final
     {
       public:

include/analysis/PromotedValues.hpp

@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: Apache-2.0
+#pragma once
+
+#include <cstdint>
+#include <functional>
+#include <memory>
+
+namespace llvm
+{
+    class Function;
+    class Instruction;
+    class Module;
+    class Value;
+} // namespace llvm
+
+namespace ctrace::stack::analysis
+{
+    /// @brief The values an index takes where its access runs.
+    struct IndexValues
+    {
+        enum class Kind : std::uint32_t
+        {
+            Unknown,   ///< Nothing is established, or the case is outside the conditions.
+            NeverRuns, ///< The access never runs: a loop around it runs no iteration.
+            Range,     ///< Every value the index takes where the access runs is in [min, max].
+        };
+
+        std::int64_t min = 0;
+        std::int64_t max = 0;
+        Kind kind = Kind::Unknown;
+        /// Range only: on every run of the function that returns, the access runs with the
+        /// index equal to this endpoint.
+        std::uint32_t minOnEveryRun : 1 = false;
+        std::uint32_t maxOnEveryRun : 1 = false;
+        std::uint32_t reservedFlags : 30 = 0;
+    };
+
+    /// @brief Value queries about the original IR, answered on a copy of the module where
+    /// promotable locals live in registers, so ScalarEvolution sees loop counters.
+    ///
+    /// The original IR stays the input of every analysis: the copy only answers questions about
+    /// the values original instructions compute. It is sanitized so that its facts hold for the
+    /// original program:
+    /// - each promotable slot starts with an opaque value, so a read that may be uninitialized
+    ///   stays unknown instead of taking the value of a later store;
+    /// - the wrap and poison flags (nsw, nuw, exact, inbounds, nneg, disjoint, samesign),
+    ///   optnone, the mustprogress and willreturn attributes and the loop metadata are dropped,
+    ///   so no fact rests on undefined behavior or on a forward-progress promise;
+    /// - functions with lifetime markers are left out.
+    ///
+    /// Index ranges use the endpoint method: they cover affine recurrences whose no-wrap
+    /// ScalarEvolution proves, in loops that exit only from their header, with every input
+    /// bounded by the program rather than by its type. Anything else is Kind::Unknown.
+    class PromotedValues final
+    {
+      public:
+        /// Copies the declarations of @p module. A function @p shouldCopy accepts is copied and
+        /// promoted on its first query; only the last function queried is kept.
+        PromotedValues(const llvm::Module& module,
+                       const std::function<bool(const llvm::Function&)>& shouldCopy);
+        ~PromotedValues();
+
+        PromotedValues(const PromotedValues&) = delete;
+        PromotedValues& operator=(const PromotedValues&) = delete;
+
+        /// Values @p index takes where @p access runs. Both belong to the original IR. Pass the
+        /// load or store itself, not the address it uses: the conditions it runs on count.
+        [[nodiscard]] IndexValues indexValuesAt(const llvm::Instruction& access,
+                                                const llvm::Value& index);
+
+        /// Whether ScalarEvolution proves @p lower <= @p index < @p upper wherever @p access
+        /// runs, reading the index as signed.
+        [[nodiscard]] bool provesWithin(const llvm::Instruction& access, const llvm::Value& index,
+                                        std::int64_t lower, std::int64_t upper);
+
+      private:
+        struct Impl;
+        std::unique_ptr<Impl> impl_;
+    };
+} // namespace ctrace::stack::analysis

include/analysis/StackBufferAnalysis.hpp

@@ -39,7 +39,12 @@ namespace ctrace::stack::analysis
         std::uint64_t indexIsConstant : 1 = false;
         // Violation based on a lower bound (index potentially negative)
         std::uint64_t isLowerBoundViolation : 1 = false;
-        std::uint64_t reservedFlags : 61 = 0;
+        // The access runs with the index at the reported bound on every run of the function
+        // that returns.
+        std::uint64_t reachedOnEveryRun : 1 = false;
+        // A loop around the access runs no iteration, so the access never runs.
+        std::uint64_t neverRuns : 1 = false;
+        std::uint64_t reservedFlags : 59 = 0;
         // Optional : helper for sync string <- vector
         void rebuildAliasPathString(const std::string& sep = " -> ")
         {