From ba500fd48b489d7051d9116a4b7b2fc50361ea46 Mon Sep 17 00:00:00 2001 From: Hugo Date: Mon, 26 Oct 2026 06:57:24 +0100 Subject: [PATCH 01/11] test(stack-buffer): a clique of pointer variables must resolve in bounded time Eight pointers chained from one buffer and then assigned from each other under a condition form one strongly connected component with 7! paths between its members. StackBufferOverflow and MultipleStores used to walk every path, with a scan of the function at each step, and never finished on such functions (#215: zlib crc32.c, cppcheck simplecpp.cpp). The last pointer is chosen by a conditional operator, so the fixture also covers a PHI of pointers, which no fixture exercised before. --- test/bound-storage/alias-cycle-terminates.c | 155 ++++++++++++++++++++ 1 file changed, 155 insertions(+) create mode 100644 test/bound-storage/alias-cycle-terminates.c diff --git a/test/bound-storage/alias-cycle-terminates.c b/test/bound-storage/alias-cycle-terminates.c new file mode 100644 index 0000000..99e6987 --- /dev/null +++ b/test/bound-storage/alias-cycle-terminates.c @@ -0,0 +1,155 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// Eight pointers in a chain from one buffer, then each assigned from every other one under a +// condition: one strongly connected component where only `p0` points to the array directly. +// The paths between them number 7! = 5040, and each one used to cost a scan of the function: +// StackBufferOverflow and MultipleStores took hours on such functions (#215, zlib crc32.c). +// The last pointer is chosen by a conditional operator, which is a PHI of pointers in the IR. + +void cycle(int n) +{ + char buf[16] = {0}; + char* p0 = buf; + char* p1 = p0; + char* p2 = p1; + char* p3 = p2; + char* p4 = p3; + char* p5 = p4; + char* p6 = p5; + char* p7 = p6; + if (n == 1) + p0 = p1; + if (n == 2) + p0 = p2; + if (n == 3) + p0 = p3; + if (n == 4) + p0 = p4; + if (n == 5) + p0 = p5; + if (n == 6) + p0 = p6; + if (n == 7) + p0 = p7; + if (n == 8) + p1 = p0; + if (n == 10) + p1 = p2; + if (n == 11) + p1 = p3; + if (n == 12) + p1 = p4; + if (n == 13) + p1 = p5; + if (n == 14) + p1 = p6; + if (n == 15) + p1 = p7; + if (n == 16) + p2 = p0; + if (n == 17) + p2 = p1; + if (n == 19) + p2 = p3; + if (n == 20) + p2 = p4; + if (n == 21) + p2 = p5; + if (n == 22) + p2 = p6; + if (n == 23) + p2 = p7; + if (n == 24) + p3 = p0; + if (n == 25) + p3 = p1; + if (n == 26) + p3 = p2; + if (n == 28) + p3 = p4; + if (n == 29) + p3 = p5; + if (n == 30) + p3 = p6; + if (n == 31) + p3 = p7; + if (n == 32) + p4 = p0; + if (n == 33) + p4 = p1; + if (n == 34) + p4 = p2; + if (n == 35) + p4 = p3; + if (n == 37) + p4 = p5; + if (n == 38) + p4 = p6; + if (n == 39) + p4 = p7; + if (n == 40) + p5 = p0; + if (n == 41) + p5 = p1; + if (n == 42) + p5 = p2; + if (n == 43) + p5 = p3; + if (n == 44) + p5 = p4; + if (n == 46) + p5 = p6; + if (n == 47) + p5 = p7; + if (n == 48) + p6 = p0; + if (n == 49) + p6 = p1; + if (n == 50) + p6 = p2; + if (n == 51) + p6 = p3; + if (n == 52) + p6 = p4; + if (n == 53) + p6 = p5; + if (n == 55) + p6 = p7; + if (n == 56) + p7 = p0; + if (n == 57) + p7 = p1; + if (n == 58) + p7 = p2; + if (n == 59) + p7 = p3; + if (n == 60) + p7 = p4; + if (n == 61) + p7 = p5; + if (n == 62) + p7 = p6; + char* q = n < 32 ? p0 : p1; + + // at line 140, column 15 + // [ !!Warn ] potential stack buffer overflow on variable 'buf' (size 16) + // ↳ alias path: buf -> arraydecay -> p0 + // ↳ index variable may go up to 19 (array last valid index: 15) + // ↳ (this is a write access) + for (int i = 0; i < 20; ++i) + p0[i] = 1; + + // at line 148, column 14 + // [ !!Warn ] potential stack buffer overflow on variable 'buf' (size 16) + // ↳ alias path: buf -> arraydecay -> p0 -> cond -> q + // ↳ index variable may go up to 19 (array last valid index: 15) + // ↳ (this is a write access) + for (int i = 0; i < 20; ++i) + q[i] = 1; +} + +int main(void) +{ + cycle(3); + return 0; +} From 8dc3a2ad717990b3bfe740a57837b6f3546b96f1 Mon Sep 17 00:00:00 2001 From: Hugo Date: Mon, 26 Oct 2026 06:57:24 +0100 Subject: [PATCH 02/11] test(stack-buffer): a pointer assigned around a cycle keeps its array Two pointers assigned from each other: resolving the first reaches the second, which reaches the first again while it is still being resolved. A resolver that remembers what it computed under that cut resolves the second pointer to nothing and loses the write through it. --- test/bound-storage/alias-cycle.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 test/bound-storage/alias-cycle.c diff --git a/test/bound-storage/alias-cycle.c b/test/bound-storage/alias-cycle.c new file mode 100644 index 0000000..d944ce6 --- /dev/null +++ b/test/bound-storage/alias-cycle.c @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// Two pointers assigned from each other. Resolving `p` reaches `q`, which reaches `p` again +// while `p` is still being resolved. A result remembered for `q` at that moment would say +// "nothing", and the write through `q` would go unreported (#215, first review). + +void cycle(int n) +{ + char buf[16] = {0}; + char* p = buf; + char* q = p; + p = q; + p[0] = n; + + // at line 21, column 14 + // [ !!Warn ] potential stack buffer overflow on variable 'buf' (size 16) + // ↳ alias path: buf -> arraydecay -> p -> q + // ↳ index variable may go up to 19 (array last valid index: 15) + // ↳ (this is a write access) + for (int i = 0; i < 20; ++i) + q[i] = 1; +} + +int main(void) +{ + cycle(3); + return 0; +} From 0479e2b93f5f1943e9e1bd39944c4e60c1e8e890 Mon Sep 17 00:00:00 2001 From: Hugo Date: Mon, 26 Oct 2026 06:57:24 +0100 Subject: [PATCH 03/11] test(stack-buffer): a chain of more than 64 pointers keeps its array A safe access through the last of 66 chained pointers is resolved first. The old resolver gave up past 64 pointers; results remembered under that cut must not survive the query, or the write through the second pointer goes unreported. The safe access now counts as a store into the buffer too, which MultipleStores reports. --- test/bound-storage/alias-chain-deep.c | 97 +++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 test/bound-storage/alias-chain-deep.c diff --git a/test/bound-storage/alias-chain-deep.c b/test/bound-storage/alias-chain-deep.c new file mode 100644 index 0000000..50b3e17 --- /dev/null +++ b/test/bound-storage/alias-chain-deep.c @@ -0,0 +1,97 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// A chain of 66 pointers. The safe access through the last one is resolved first; the old +// resolver gave up past 64 pointers, and a resolver that remembered the results computed +// under that cut lost the report on the write through `p1` (#215, first review). + +void cycle(int n) +{ + char buf[16] = {0}; + char* p0 = buf; + char* p1 = p0; + char* p2 = p1; + char* p3 = p2; + char* p4 = p3; + char* p5 = p4; + char* p6 = p5; + char* p7 = p6; + char* p8 = p7; + char* p9 = p8; + char* p10 = p9; + char* p11 = p10; + char* p12 = p11; + char* p13 = p12; + char* p14 = p13; + char* p15 = p14; + char* p16 = p15; + char* p17 = p16; + char* p18 = p17; + char* p19 = p18; + char* p20 = p19; + char* p21 = p20; + char* p22 = p21; + char* p23 = p22; + char* p24 = p23; + char* p25 = p24; + char* p26 = p25; + char* p27 = p26; + char* p28 = p27; + char* p29 = p28; + char* p30 = p29; + char* p31 = p30; + char* p32 = p31; + char* p33 = p32; + char* p34 = p33; + char* p35 = p34; + char* p36 = p35; + char* p37 = p36; + char* p38 = p37; + char* p39 = p38; + char* p40 = p39; + char* p41 = p40; + char* p42 = p41; + char* p43 = p42; + char* p44 = p43; + char* p45 = p44; + char* p46 = p45; + char* p47 = p46; + char* p48 = p47; + char* p49 = p48; + char* p50 = p49; + char* p51 = p50; + char* p52 = p51; + char* p53 = p52; + char* p54 = p53; + char* p55 = p54; + char* p56 = p55; + char* p57 = p56; + char* p58 = p57; + char* p59 = p58; + char* p60 = p59; + char* p61 = p60; + char* p62 = p61; + char* p63 = p62; + char* p64 = p63; + char* p65 = p64; + p65[0] = n; + + // at line 84, column 15 + // [ !!Warn ] potential stack buffer overflow on variable 'buf' (size 16) + // ↳ alias path: buf -> arraydecay -> p0 -> p1 + // ↳ index variable may go up to 19 (array last valid index: 15) + // ↳ (this is a write access) + for (int i = 0; i < 20; ++i) + p1[i] = 1; +} + +int main(void) +{ + cycle(3); + return 0; +} + +// The safe access through `p65` counts as a store into `buf` too: the resolver no longer gives +// up past 64 pointers. +// at line 9, column 1 +// [ !Info! ] multiple stores to stack buffer 'buf' in this function (2 store instruction(s), 2 distinct index expression(s)) +// [ !Info! ] stores use different index expressions; verify indices are correct and non-overlapping From 275ce8a6e96b5f92f871c3805ed1fa5c814c571c Mon Sep 17 00:00:00 2001 From: Hugo Date: Mon, 26 Oct 2026 06:57:25 +0100 Subject: [PATCH 04/11] test(stack-buffer): a cycle that reaches two arrays resolves to no array Two pointers to arrays of different sizes, assigned from each other under a condition, form one component that reaches both arrays. Several arrays mean no array, as for any pointer, so neither write is reported, even though both arrays overflow. The old resolver reported each pointer against its own array; the result of a component now depends on the function alone. --- test/bound-storage/alias-cycle-two-arrays.c | 46 +++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 test/bound-storage/alias-cycle-two-arrays.c diff --git a/test/bound-storage/alias-cycle-two-arrays.c b/test/bound-storage/alias-cycle-two-arrays.c new file mode 100644 index 0000000..dba3a05 --- /dev/null +++ b/test/bound-storage/alias-cycle-two-arrays.c @@ -0,0 +1,46 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// Two pointers to two arrays of different sizes, assigned from each other under a condition: +// one strongly connected component that reaches two arrays. The resolver reports no array for +// it, so neither write is reported, even though both arrays overflow at index 39. The old +// resolver reported each pointer against its own array; the component's result now depends +// on the function alone, and several arrays mean no array (#215, second review). + +// not contains: potential stack buffer overflow + +void both_arrays(int n) +{ + char a[16] = {0}; + char b[32] = {0}; + char* p = a; + char* q = b; + if (n) + p = q; + else + q = p; + p[0] = 0; + for (int i = 0; i < 40; ++i) + q[i] = 1; +} + +void both_arrays_reverse_query(int n) +{ + char a[16] = {0}; + char b[32] = {0}; + char* p = a; + char* q = b; + if (n) + p = q; + else + q = p; + q[0] = 0; + for (int i = 0; i < 40; ++i) + p[i] = 1; +} + +int main(void) +{ + both_arrays(0); + both_arrays_reverse_query(1); + return 0; +} From ccddeae5eb73da9eac04573619eb24d254c63aa5 Mon Sep 17 00:00:00 2001 From: Hugo Date: Mon, 26 Oct 2026 06:57:25 +0100 Subject: [PATCH 05/11] perf(stack-buffer): resolve pointer variables once per function resolveArrayAllocaFromPointer traced a pointer back to its stack array by recursion: the recursion stack only cut the current path, nothing remembered a result, and every visit of a pointer variable scanned the whole function for the stores into it. With k pointer variables assigned from one another it enumerated (k-1)! paths; zlib's crc32.c and cppcheck's simplecpp.cpp never finished (#215). ArrayAllocaResolver replaces it, one instance per function. It indexes the stores into each alloca once, then resolves pointer variables and PHIs with an iterative Tarjan walk: a strongly connected component is finished only after every component it reaches, so its result is computed from final results and does not depend on the query that started the walk. A component resolves to the one array its members' sources lead to; a source that leads nowhere is ignored, two arrays make it ambiguous. Alias paths are kept as witnesses, one segment per node, and copied once per query. There is no depth limit any more. On acyclic alias graphs the results and alias paths are those of the old resolver. A cycle that reaches two arrays now resolves to no array, where each pointer used to be reported against its own array. Fixes #215. --- src/analysis/StackBufferAnalysis.cpp | 407 +++++++++++++++++---------- 1 file changed, 259 insertions(+), 148 deletions(-) diff --git a/src/analysis/StackBufferAnalysis.cpp b/src/analysis/StackBufferAnalysis.cpp index 2bf7841..8a975d2 100644 --- a/src/analysis/StackBufferAnalysis.cpp +++ b/src/analysis/StackBufferAnalysis.cpp @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -33,21 +34,6 @@ namespace ctrace::stack::analysis { using SmtFeasibility = smt::SmtFeasibility; - struct RecursionGuard - { - llvm::SmallPtrSetImpl& set; - const llvm::Value* value; - RecursionGuard(llvm::SmallPtrSetImpl& s, const llvm::Value* v) - : set(s), value(v) - { - set.insert(value); - } - ~RecursionGuard() - { - set.erase(value); - } - }; - struct AnalysisComplexityBudgets { std::size_t pointerStoreScanInstrThreshold = 0; @@ -224,159 +210,294 @@ namespace ctrace::stack::analysis return count; } - static const llvm::AllocaInst* resolveArrayAllocaFromPointerInternal( - const llvm::Value* V, llvm::Function& F, std::vector& path, - llvm::SmallPtrSetImpl& recursionStack, int depth, - bool allowPointerStoreScan) + /// Resolves a pointer to the stack array it indexes, through the function's local + /// pointer variables (`char* p = buf; p[i]`) and pointer PHIs. One instance serves one + /// function: it indexes the stores into each alloca once, and resolves every pointer + /// variable and PHI once, by strongly connected components, so that a node's result + /// depends on the function alone and not on the query that reached it (#215). The cost + /// is bounded by the function's size. + class ArrayAllocaResolver { - using namespace llvm; - - if (!V) - return nullptr; - if (depth > 64) - return nullptr; - if (recursionStack.contains(V)) - return nullptr; + public: + ArrayAllocaResolver(const llvm::Function& F, bool allowPointerStoreScan) + { + using namespace llvm; + // Without the scan, a pointer variable has no known source and resolves to nothing. + if (!allowPointerStoreScan) + return; + for (const BasicBlock& BB : F) + { + for (const Instruction& I : BB) + { + const auto* SI = dyn_cast(&I); + if (!SI) + continue; + if (const auto* AI = dyn_cast(SI->getPointerOperand())) + storedValues_[AI].push_back(SI->getValueOperand()); + } + } + } - RecursionGuard guard(recursionStack, V); + /// The array alloca `V` points into, or nullptr when none is known or several are. + /// Appends to `path` the names met on the way, the array's last. + const llvm::AllocaInst* resolve(const llvm::Value* V, std::vector& path) + { + const Target target = walk(V); + path.insert(path.end(), target.names.begin(), target.names.end()); + if (!target.node) + return target.array; + + resolveFrom(target.node); + const Node* node = &nodes_[target.node]; + if (!node->alloca) + return nullptr; + for (;; node = &nodes_[node->next]) + { + path.insert(path.end(), node->segment.begin(), node->segment.end()); + if (!node->next) + return node->alloca; + } + } - auto isArrayAlloca = [](const AllocaInst* AI) -> bool + private: + // Where following casts, GEPs and loads from a value ends: a stack array, a node (a + // local pointer variable or a PHI), or neither when the heuristic does not follow. + struct Target { - // Consider a "stack buffer" as: - // - real arrays, - // - array-typed allocas (VLA in IR), - // - structs containing array fields. - if (AI->isArrayAllocation()) - return true; - return isArrayBackedType(AI->getAllocatedType()); + const llvm::AllocaInst* array = nullptr; + const llvm::Value* node = nullptr; + std::vector names; // the names met on the way, the target's last }; - // Avoid weird aliasing loops - SmallPtrSet visited; - const Value* cur = V; + struct Node + { + const llvm::AllocaInst* alloca = nullptr; // the result, once done + // The witness of the result, for the alias path: the names from the chosen + // source to its target, and that target when it is a node. + const llvm::Value* next = nullptr; + std::vector segment; + std::vector targets; // what each source leads to, until done + std::uint64_t index : 31 = 0; // Tarjan's discovery index, 0 until visited + std::uint64_t low : 31 = 0; + std::uint64_t onStack : 1 = 0; + std::uint64_t done : 1 = 0; + }; - while (cur && !visited.contains(cur)) + Target walk(const llvm::Value* V) const { - visited.insert(cur); - if (cur->hasName()) - path.push_back(cur->getName().str()); + using namespace llvm; - // Case 1: we hit an alloca. - if (auto* AI = dyn_cast(cur)) + Target target; + SmallPtrSet visited; + const Value* cur = V; + while (cur && visited.insert(cur).second) { - if (isArrayAlloca(AI)) + if (cur->hasName()) + target.names.push_back(cur->getName().str()); + + if (const auto* AI = dyn_cast(cur)) { - // Stack buffer alloca (array): final target. - return AI; + // A stack buffer: a real array, an array alloca (VLA in IR) or a struct + // with an array field. Anything else is a local pointer variable + // (char* ptr; char** pp; ...): what is stored into it says where it points. + if (AI->isArrayAllocation() || isArrayBackedType(AI->getAllocatedType())) + target.array = AI; + else + target.node = AI; + return target; } - - // Otherwise, it's very likely a local pointer variable - // (char *ptr; char **pp; etc.). Walk stores into this variable - // to see what values get assigned, and try to trace back to - // a real array alloca. - if (!allowPointerStoreScan) - return nullptr; - const AllocaInst* foundAI = nullptr; - - for (BasicBlock& BB : F) + if (const auto* BC = dyn_cast(cur)) { - for (Instruction& I : BB) - { - auto* SI = dyn_cast(&I); - if (!SI) - continue; - if (SI->getPointerOperand() != AI) - continue; - - const Value* storedPtr = SI->getValueOperand(); - std::vector subPath; - const AllocaInst* cand = resolveArrayAllocaFromPointerInternal( - storedPtr, F, subPath, recursionStack, depth + 1, - allowPointerStoreScan); - if (!cand) - continue; - - if (!foundAI) - { - foundAI = cand; - // Append subPath to path - path.insert(path.end(), subPath.begin(), subPath.end()); - } - else if (foundAI != cand) - { - // Multiple different bases: ambiguous aliasing, - // prefer to stop rather than be wrong. - return nullptr; - } - } + cur = BC->getOperand(0); + continue; + } + if (const auto* GEP = dyn_cast(cur)) + { + cur = GEP->getPointerOperand(); + continue; } - return foundAI; + // A load of a pointer: walk up to its container (char* ptr = buf; (*pp)[i]). + if (const auto* LI = dyn_cast(cur)) + { + cur = LI->getPointerOperand(); + continue; + } + if (isa(cur)) + { + target.node = cur; + return target; + } + // Arguments, globals, calls: the heuristic stops here. + break; } + return target; + } + + // The values a node takes: those stored into the variable, or the PHI's incoming values. + llvm::SmallVector sourcesOf(const llvm::Value* node) const + { + using namespace llvm; - // Case 2: bitcast -> follow the operand. - if (auto* BC = dyn_cast(cur)) + SmallVector sources; + if (const auto* PN = dyn_cast(node)) { - cur = BC->getOperand(0); - continue; + for (const Use& incoming : PN->incoming_values()) + sources.push_back(incoming.get()); } - - // Case 3: GEP -> follow the base pointer. - if (auto* GEP = dyn_cast(cur)) + else if (auto stored = storedValues_.find(cast(node)); + stored != storedValues_.end()) { - cur = GEP->getPointerOperand(); - continue; + sources = stored->second; } + return sources; + } + + // Resolves `start` and every node it reaches, with Tarjan's algorithm: a strongly + // connected component is finished only once every component it reaches is, so its + // result is computed from final results and is final itself, whatever the query + // that started the walk. Iterative, so that a long chain of pointer variables costs + // no stack. + void resolveFrom(const llvm::Value* start) + { + using namespace llvm; + + if (nodes_[start].index != 0) + return; // done: no query starts while a walk is in progress - // Case 4: load of a pointer. Typical example: - // char *ptr = test; - // char *p2 = ptr; - // char **pp = &ptr; - // (*pp)[i] = ... - // - // Walk up to the pointer "container" (local variable, or other value) - // by following the load operand. - if (auto* LI = dyn_cast(cur)) + struct Frame { - cur = LI->getPointerOperand(); - continue; + const Value* node; + std::size_t nextTarget; + }; + SmallVector frames; + SmallVector component; // Tarjan's stack + const auto enter = [&](const Value* value) + { + Node& node = nodes_[value]; + node.index = node.low = ++counter_; + node.onStack = 1; + for (const Value* source : sourcesOf(value)) + node.targets.push_back(walk(source)); + component.push_back(value); + frames.push_back({value, 0}); + }; + + enter(start); + while (!frames.empty()) + { + Frame& frame = frames.back(); + Node& node = nodes_[frame.node]; + if (frame.nextTarget < node.targets.size()) + { + const Value* next = node.targets[frame.nextTarget++].node; + if (!next) + continue; + const Node& child = nodes_[next]; + if (child.index == 0) + enter(next); + else if (child.onStack) + node.low = std::min(node.low, child.index); + continue; + } + + if (node.low == node.index) + finishComponent(component, frame.node); + const std::uint64_t low = node.low; + frames.pop_back(); + if (!frames.empty()) + { + Node& parent = nodes_[frames.back().node]; + parent.low = std::min(parent.low, low); + } } + } + + // Pops the component rooted at `root` and gives every member its result: the one + // array the members' sources lead to, directly or through a finished node. A source + // that leads to nothing is ignored; two different arrays make the component ambiguous. + void finishComponent(llvm::SmallVectorImpl& stack, + const llvm::Value* root) + { + using namespace llvm; - // Case 5: PHI of pointers (merge of aliases): - // try to resolve each incoming and ensure they - // all point to the same array alloca. - if (auto* PN = dyn_cast(cur)) + SmallVector members; + do + { + members.push_back(stack.back()); + stack.pop_back(); + } while (members.back() != root); + + // A member of this component is not done yet: its sources speak for it. + const auto arrayOf = [&](const Target& target) -> const AllocaInst* + { + if (target.array || !target.node) + return target.array; + const Node& child = nodes_[target.node]; + return child.done ? child.alloca : nullptr; + }; + + const AllocaInst* found = nullptr; + bool ambiguous = false; + for (const Value* member : members) { - const AllocaInst* foundAI = nullptr; - std::vector phiPath; - for (unsigned i = 0; i < PN->getNumIncomingValues(); ++i) + for (const Target& target : nodes_[member].targets) { - const Value* inV = PN->getIncomingValue(i); - std::vector subPath; - const AllocaInst* cand = resolveArrayAllocaFromPointerInternal( - inV, F, subPath, recursionStack, depth + 1, allowPointerStoreScan); - if (!cand) + const AllocaInst* array = arrayOf(target); + if (!array) continue; - if (!foundAI) - { - foundAI = cand; - phiPath = subPath; - } - else if (foundAI != cand) + if (!found) + found = array; + else if (found != array) + ambiguous = true; + } + } + if (ambiguous) + found = nullptr; + + // Witnesses: a member takes its first source that leads to the array, directly or + // through a member settled in an earlier round. A round settles the members one + // step further from the array, so the path does not depend on the member order. + bool progress = found != nullptr; + while (progress) + { + SmallVector settled; + for (const Value* member : members) + { + Node& node = nodes_[member]; + if (node.done) + continue; + for (const Target& target : node.targets) { - // PHI mixes multiple different bases: too ambiguous. - return nullptr; + if (arrayOf(target) != found) + continue; + node.segment = target.names; + node.next = target.node; + settled.push_back(member); + break; } } - path.insert(path.end(), phiPath.begin(), phiPath.end()); - return foundAI; + for (const Value* member : settled) + { + nodes_[member].alloca = found; + nodes_[member].done = 1; + } + progress = !settled.empty(); } - // Other cases (arguments, complex globals, etc.): stop the heuristic. - break; + for (const Value* member : members) + { + Node& node = nodes_[member]; + node.onStack = 0; + node.done = 1; + node.targets = {}; + } } - return nullptr; - } + std::unordered_map> + storedValues_; + std::unordered_map nodes_; + std::uint64_t counter_ = 0; + }; static std::optional isAllocaArrayByDebugInfo(const llvm::AllocaInst* AI, const llvm::Function& F) @@ -447,16 +568,6 @@ namespace ctrace::stack::analysis return false; } - static const llvm::AllocaInst* resolveArrayAllocaFromPointer(const llvm::Value* V, - llvm::Function& F, - std::vector& path, - bool allowPointerStoreScan) - { - llvm::SmallPtrSet recursionStack; - return resolveArrayAllocaFromPointerInternal(V, F, path, recursionStack, 0, - allowPointerStoreScan); - } - static const llvm::StoreInst* findUniqueStoreToKeyInBlock(const llvm::BasicBlock& block, const llvm::Value* key) { @@ -758,6 +869,7 @@ namespace ctrace::stack::analysis return; const bool allowPointerStoreScan = instructionCount <= budgets.pointerStoreScanInstrThreshold; + ArrayAllocaResolver resolver(F, allowPointerStoreScan); const FunctionFacts facts(F); const ProgramPointRanges ranges(F, facts); std::size_t analyzedGEPs = 0; @@ -785,8 +897,7 @@ namespace ctrace::stack::analysis cached.computed = true; std::vector resolvedPath; - cached.alloca = - resolveArrayAllocaFromPointer(basePtr, F, resolvedPath, allowPointerStoreScan); + cached.alloca = resolver.resolve(basePtr, resolvedPath); if (!cached.alloca) cached.global = resolveArrayGlobalFromPointer(basePtr); @@ -1173,6 +1284,7 @@ namespace ctrace::stack::analysis return; const bool allowPointerStoreScan = instructionCount <= budgets.pointerStoreScanInstrThreshold; + ArrayAllocaResolver resolver(F, allowPointerStoreScan); std::size_t analyzedStores = 0; struct Info @@ -1203,8 +1315,7 @@ namespace ctrace::stack::analysis // Walk back to the base to find a stack array alloca. const Value* basePtr = GEP->getPointerOperand(); std::vector dummyAliasPath; - const AllocaInst* AI = resolveArrayAllocaFromPointer(basePtr, F, dummyAliasPath, - allowPointerStoreScan); + const AllocaInst* AI = resolver.resolve(basePtr, dummyAliasPath); if (!AI) continue; From 6f8691ceece655a9c9dff99b658e34b47cf9f5d7 Mon Sep 17 00:00:00 2001 From: Hugo Date: Mon, 26 Oct 2026 06:57:25 +0100 Subject: [PATCH 06/11] perf(stack-buffer): settle alias path witnesses level by level The witnesses of a component were settled by rounds that rescanned every member: on a ring of n pointers with one array source, n rounds. A level worklist over the reverse dependencies settles each member once and follows each edge between members once, with the same choice of witness, the first source among those settled at a lower level, so the alias paths are unchanged. Ring of 8000 pointers: the StackBufferOverflow step goes from 595 ms to 5 ms. --- src/analysis/StackBufferAnalysis.cpp | 51 +++++++++++++++++++++------- 1 file changed, 38 insertions(+), 13 deletions(-) diff --git a/src/analysis/StackBufferAnalysis.cpp b/src/analysis/StackBufferAnalysis.cpp index 8a975d2..b07b96c 100644 --- a/src/analysis/StackBufferAnalysis.cpp +++ b/src/analysis/StackBufferAnalysis.cpp @@ -214,8 +214,10 @@ namespace ctrace::stack::analysis /// pointer variables (`char* p = buf; p[i]`) and pointer PHIs. One instance serves one /// function: it indexes the stores into each alloca once, and resolves every pointer /// variable and PHI once, by strongly connected components, so that a node's result - /// depends on the function alone and not on the query that reached it (#215). The cost - /// is bounded by the function's size. + /// depends on the function alone and not on the query that reached it (#215). The work is + /// proportional to the function's pointer variables, PHIs and stores: each is visited + /// once by the walk and once more to settle its alias path witness. A query then + /// copies its alias path once. class ArrayAllocaResolver { public: @@ -454,34 +456,57 @@ namespace ctrace::stack::analysis if (ambiguous) found = nullptr; - // Witnesses: a member takes its first source that leads to the array, directly or - // through a member settled in an earlier round. A round settles the members one - // step further from the array, so the path does not depend on the member order. - bool progress = found != nullptr; - while (progress) + // Witnesses, for the alias path. Level 0: the members with a source that leads + // to the array directly or through a node outside the component. Level k + 1: + // the members not settled yet with a source that leads to a member of level k. + // A member takes its first source among those settled at a lower level, so the + // path depends neither on the member order nor on the query. Each member is + // settled once and each edge between members is followed once. + std::unordered_map> dependents; + SmallVector level; + if (found) { - SmallVector settled; for (const Value* member : members) + { + bool direct = false; + for (const Target& target : nodes_[member].targets) + { + if (target.node && nodes_[target.node].onStack) + dependents[target.node].push_back(member); + else if (arrayOf(target) == found) + direct = true; + } + if (direct) + level.push_back(member); + } + } + SmallPtrSet queued(level.begin(), level.end()); + while (!level.empty()) + { + for (const Value* member : level) { Node& node = nodes_[member]; - if (node.done) - continue; for (const Target& target : node.targets) { if (arrayOf(target) != found) continue; node.segment = target.names; node.next = target.node; - settled.push_back(member); break; } } - for (const Value* member : settled) + SmallVector next; + for (const Value* member : level) { nodes_[member].alloca = found; nodes_[member].done = 1; + for (const Value* dependent : dependents[member]) + { + if (queued.insert(dependent).second) + next.push_back(dependent); + } } - progress = !settled.empty(); + level = std::move(next); } for (const Value* member : members) From 10978f874ec12f98bb6b91c4b8e9906eaee902a0 Mon Sep 17 00:00:00 2001 From: Hugo Date: Mon, 26 Oct 2026 06:57:25 +0100 Subject: [PATCH 07/11] docs(changelog): announce the bounded pointer resolution --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 50c326b..ad32fea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,13 @@ Notable changes are grouped by release. See [all releases](https://github.com/Co ## Unreleased +- StackBufferOverflow and MultipleStoresToStackBuffer resolve each pointer variable to its + stack array once per function, by strongly connected components: pointer variables + assigned from one another no longer make the analysis walk every path between them, + which never finished on zlib's `crc32.c` and cppcheck's `simplecpp.cpp` (#215). Pointers + assigned from one another in a cycle that reaches two different arrays now resolve to no + array, as any pointer with several possible arrays does, where each used to be reported + against its own array. - Added an authors file and linked the repository documents from the README. - SARIF rules carry a title (`shortDescription`) and a description (`fullDescription`) instead of their id, and `describeRule` (`RuleCatalog.hpp`) gives them to programs linking the analyzer. From 19035cef60b7f40025425ba8c9b7062155fc952c Mon Sep 17 00:00:00 2001 From: Hugo Date: Tue, 27 Oct 2026 00:21:08 +0100 Subject: [PATCH 08/11] test(stack-buffer): every pointer of a three-pointer ring finds its array Three pointers assigned in a ring from one buffer form one strongly connected component where only the first points to the array directly. The fixture writes through each pointer in two functions, in opposite orders, and expects the same overflows with the same alias paths in both: a result must not depend on the walk that first reached the pointer. The three writes also make MultipleStores report the buffer of each function, so both users of the resolver are exercised. --- .../alias-cycle-three-pointers.c | 93 +++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 test/bound-storage/alias-cycle-three-pointers.c diff --git a/test/bound-storage/alias-cycle-three-pointers.c b/test/bound-storage/alias-cycle-three-pointers.c new file mode 100644 index 0000000..111a55a --- /dev/null +++ b/test/bound-storage/alias-cycle-three-pointers.c @@ -0,0 +1,93 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// Three pointers assigned in a ring from one buffer: one strongly connected component where only +// `p` points to the array directly, `q` through `p` and `r` through `q`. Every pointer must find +// the array, and the order of the accesses must not matter: `ring_pqr` writes through p, q and +// r, `ring_rqp` through r, q and p, and both report the same overflows with the same alias +// paths. The three writes also make MultipleStores report the buffer of each function. + +// strict-expectation-details: true + +void ring_pqr(int n) +{ + char buf[16] = {0}; + char* p = buf; + char* q = p; + char* r = q; + if (n) + p = r; + + // at line 26, column 14 + // [ !!Warn ] potential stack buffer overflow on variable 'buf' (size 16) + // ↳ alias path: buf -> arraydecay -> p + // ↳ index variable may go up to 19 (array last valid index: 15) + // ↳ (this is a write access) + for (int i = 0; i < 20; ++i) + p[i] = 1; + + // at line 34, column 14 + // [ !!Warn ] potential stack buffer overflow on variable 'buf' (size 16) + // ↳ alias path: buf -> arraydecay -> p -> q + // ↳ index variable may go up to 19 (array last valid index: 15) + // ↳ (this is a write access) + for (int i = 0; i < 20; ++i) + q[i] = 2; + + // at line 42, column 14 + // [ !!Warn ] potential stack buffer overflow on variable 'buf' (size 16) + // ↳ alias path: buf -> arraydecay -> p -> q -> r + // ↳ index variable may go up to 19 (array last valid index: 15) + // ↳ (this is a write access) + for (int i = 0; i < 20; ++i) + r[i] = 3; +} + +void ring_rqp(int n) +{ + char buf[16] = {0}; + char* p = buf; + char* q = p; + char* r = q; + if (n) + p = r; + + // at line 60, column 14 + // [ !!Warn ] potential stack buffer overflow on variable 'buf' (size 16) + // ↳ alias path: buf -> arraydecay -> p -> q -> r + // ↳ index variable may go up to 19 (array last valid index: 15) + // ↳ (this is a write access) + for (int i = 0; i < 20; ++i) + r[i] = 1; + + // at line 68, column 14 + // [ !!Warn ] potential stack buffer overflow on variable 'buf' (size 16) + // ↳ alias path: buf -> arraydecay -> p -> q + // ↳ index variable may go up to 19 (array last valid index: 15) + // ↳ (this is a write access) + for (int i = 0; i < 20; ++i) + q[i] = 2; + + // at line 76, column 14 + // [ !!Warn ] potential stack buffer overflow on variable 'buf' (size 16) + // ↳ alias path: buf -> arraydecay -> p + // ↳ index variable may go up to 19 (array last valid index: 15) + // ↳ (this is a write access) + for (int i = 0; i < 20; ++i) + p[i] = 3; +} + +int main(void) +{ + ring_pqr(0); + ring_rqp(1); + return 0; +} + +// MultipleStores sees the three writes of each function as stores into its buffer. +// at line 13, column 1 +// [ !Info! ] multiple stores to stack buffer 'buf' in this function (3 store instruction(s), 3 distinct index expression(s)) +// [ !Info! ] stores use different index expressions; verify indices are correct and non-overlapping + +// at line 47, column 1 +// [ !Info! ] multiple stores to stack buffer 'buf' in this function (3 store instruction(s), 3 distinct index expression(s)) +// [ !Info! ] stores use different index expressions; verify indices are correct and non-overlapping From bb8222fc6c4e6472f14240d901d8c66a03d2b1e1 Mon Sep 17 00:00:00 2001 From: Hugo Date: Tue, 27 Oct 2026 00:21:08 +0100 Subject: [PATCH 09/11] test(stack-buffer): a cycle through PHIs of pointers finds its array A pointer takes a conditional operator over another pointer and the buffer, and the other pointer is reassigned in a loop from a conditional operator over the first and itself: the component holds two pointer variables and two PHIs, and one alias path goes through a PHI. Both writes must be reported and MultipleStores must see both stores. --- test/bound-storage/alias-cycle-phi.c | 44 ++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 test/bound-storage/alias-cycle-phi.c diff --git a/test/bound-storage/alias-cycle-phi.c b/test/bound-storage/alias-cycle-phi.c new file mode 100644 index 0000000..be8c924 --- /dev/null +++ b/test/bound-storage/alias-cycle-phi.c @@ -0,0 +1,44 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// A cycle that goes through PHIs of pointers: `q` takes a conditional operator over `p` and the +// buffer, and `p` is reassigned in a loop from a conditional operator over `q` and itself. The +// component holds the two pointer variables and the two PHIs, and the alias path of `q` goes +// through a PHI (`cond`). Both writes must be reported and MultipleStores must see both stores. + +// strict-expectation-details: true + +void phi_cycle(int n) +{ + char buf[16] = {0}; + char* p = buf; + char* q = (n & 1) ? p : buf; + for (int i = 0; i < n; ++i) + p = (i & 1) ? q : p; + + // at line 24, column 14 + // [ !!Warn ] potential stack buffer overflow on variable 'buf' (size 16) + // ↳ alias path: buf -> arraydecay -> p + // ↳ index variable may go up to 19 (array last valid index: 15) + // ↳ (this is a write access) + for (int i = 0; i < 20; ++i) + p[i] = 1; + + // at line 32, column 14 + // [ !!Warn ] potential stack buffer overflow on variable 'buf' (size 16) + // ↳ alias path: buf -> arraydecay1 -> cond -> q + // ↳ index variable may go up to 19 (array last valid index: 15) + // ↳ (this is a write access) + for (int i = 0; i < 20; ++i) + q[i] = 2; +} + +int main(void) +{ + phi_cycle(3); + return 0; +} + +// MultipleStores sees both writes as stores into the buffer. +// at line 12, column 1 +// [ !Info! ] multiple stores to stack buffer 'buf' in this function (2 store instruction(s), 2 distinct index expression(s)) +// [ !Info! ] stores use different index expressions; verify indices are correct and non-overlapping From f88e4627c7ff888fc10b3440e803e4db8cf676f9 Mon Sep 17 00:00:00 2001 From: Hugo Date: Tue, 27 Oct 2026 00:21:08 +0100 Subject: [PATCH 10/11] test(stack-buffer): state the two-array cycle trade-off and add its control case The fixture now says what it pins: a component that reaches two arrays resolves to no array, so the writes through it go unreported even though both arrays overflow. This is a deliberate trade-off that follows the contract for any pointer with several possible arrays, not a gain in detection. The control case has the same shape with both pointers from one array, and its overflow must be reported, with its alias path and the MultipleStores note. --- test/bound-storage/alias-cycle-two-arrays.c | 42 ++++++++++++++++++--- 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/test/bound-storage/alias-cycle-two-arrays.c b/test/bound-storage/alias-cycle-two-arrays.c index dba3a05..1c4e03d 100644 --- a/test/bound-storage/alias-cycle-two-arrays.c +++ b/test/bound-storage/alias-cycle-two-arrays.c @@ -1,12 +1,16 @@ // SPDX-License-Identifier: Apache-2.0 // -// Two pointers to two arrays of different sizes, assigned from each other under a condition: -// one strongly connected component that reaches two arrays. The resolver reports no array for -// it, so neither write is reported, even though both arrays overflow at index 39. The old -// resolver reported each pointer against its own array; the component's result now depends -// on the function alone, and several arrays mean no array (#215, second review). +// Pointers assigned from each other under a condition form one strongly connected component. +// When the component reaches two arrays of different sizes, the resolver reports no array for +// it, as it does for any pointer that may point into several arrays, so neither write through +// `p` or `q` is reported even though both arrays overflow at index 39. This is a deliberate +// trade-off, not a gain in detection: the old resolver reported each pointer against its own +// array, which depended on the order of the walk. The control case `same_array` has the same +// shape with both pointers from one array, and its overflow must be reported. -// not contains: potential stack buffer overflow +// strict-expectation-details: true +// not contains: overflow on variable 'a' (size 16) +// not contains: overflow on variable 'b' (size 32) void both_arrays(int n) { @@ -38,9 +42,35 @@ void both_arrays_reverse_query(int n) p[i] = 1; } +void same_array(int n) +{ + char c[16] = {0}; + char* p = c; + char* q = c; + if (n) + p = q; + else + q = p; + p[0] = 0; + + // at line 62, column 14 + // [ !!Warn ] potential stack buffer overflow on variable 'c' (size 16) + // ↳ alias path: c -> arraydecay1 -> q + // ↳ index variable may go up to 39 (array last valid index: 15) + // ↳ (this is a write access) + for (int i = 0; i < 40; ++i) + q[i] = 1; +} + int main(void) { both_arrays(0); both_arrays_reverse_query(1); + same_array(0); return 0; } + +// In the control case, MultipleStores sees both writes as stores into the one array. +// at line 47, column 1 +// [ !Info! ] multiple stores to stack buffer 'c' in this function (2 store instruction(s), 2 distinct index expression(s)) +// [ !Info! ] stores use different index expressions; verify indices are correct and non-overlapping From 972cecf9f127bfd4218ca1c23ae4edf3e2c2d073 Mon Sep 17 00:00:00 2001 From: Hugo Date: Tue, 27 Oct 2026 00:21:08 +0100 Subject: [PATCH 11/11] test(stack-buffer): resolve a loop-carried PHI cycle and a ring of 5000 pointers Two unit tests call StackBufferOverflow and MultipleStores directly. A hand-written module holds a loop whose two pointer PHIs take each other's value, the cycle mem2reg leaves in optimized code: both PHIs must resolve to the buffer through its decay, and both stores must count. A ring of 5000 pointer variables with one array source is built with IRBuilder, as -O0 code leaves it. Every pointer must find the array, 2500 and 4999 steps away included, the alias paths must not depend on the order of the accesses, and the walk must finish on a component of that size. A time bound would be fragile; the generous CI timeout stands for termination. --- test/unit/alias_phi_cycle_input.ll | 26 +++++ test/unit/analyzer_module_unit_tests.cpp | 128 +++++++++++++++++++++++ 2 files changed, 154 insertions(+) create mode 100644 test/unit/alias_phi_cycle_input.ll diff --git a/test/unit/alias_phi_cycle_input.ll b/test/unit/alias_phi_cycle_input.ll new file mode 100644 index 0000000..cac32b2 --- /dev/null +++ b/test/unit/alias_phi_cycle_input.ll @@ -0,0 +1,26 @@ +; SPDX-License-Identifier: Apache-2.0 +; A loop whose two pointer PHIs take each other's value: the cycle p -> q -> p that mem2reg +; leaves in optimized code. Both PHIs must resolve to buf through its decay, and the two stores +; through them must count as stores into buf. + +define void @phi_cycle(i32 %n) { +entry: + %buf = alloca [16 x i8], align 1 + %start = getelementptr inbounds [16 x i8], ptr %buf, i64 0, i64 0 + br label %loop + +loop: + %i = phi i32 [ 0, %entry ], [ %i.next, %loop ] + %p = phi ptr [ %start, %entry ], [ %q, %loop ] + %q = phi ptr [ %start, %entry ], [ %p, %loop ] + %p.slot = getelementptr inbounds i8, ptr %p, i64 20 + store i8 0, ptr %p.slot, align 1 + %q.slot = getelementptr inbounds i8, ptr %q, i64 21 + store i8 1, ptr %q.slot, align 1 + %i.next = add nsw i32 %i, 1 + %more = icmp slt i32 %i.next, %n + br i1 %more, label %loop, label %exit + +exit: + ret void +} diff --git a/test/unit/analyzer_module_unit_tests.cpp b/test/unit/analyzer_module_unit_tests.cpp index 3a98dfe..698b961 100644 --- a/test/unit/analyzer_module_unit_tests.cpp +++ b/test/unit/analyzer_module_unit_tests.cpp @@ -45,6 +45,7 @@ #include #include #include +#include #include #include #include @@ -2752,6 +2753,131 @@ namespace } return report.failures == 0; } + /// A loop whose two pointer PHIs take each other's value, as mem2reg leaves optimized code: + /// a cycle through PHIs. Both analyses that use the resolver must find the array through it. + bool testStackBufferPhiCycle(const std::filesystem::path& repoRoot, TestReport& report) + { + using namespace ctrace::stack::analysis; + const ctrace::stack::AnalysisConfig config; + LoadedModule loaded; + std::string loadError; + if (!loadModuleFromSource(repoRoot / "test/unit/alias_phi_cycle_input.ll", config, loaded, + loadError)) + { + report.expect(false, + "stack-buffer PHI cycle setup: failed to load module: " + loadError); + return false; + } + const auto all = [](const llvm::Function&) { return true; }; + + std::map pathByIndex; + for (const StackBufferOverflowIssue& issue : + analyzeStackBufferOverflows(*loaded.module, all, config)) + { + if (issue.indexIsConstant && issue.varName == "buf") + pathByIndex[issue.indexOrUpperBound] = issue.aliasPath; + } + report.expect(pathByIndex.size() == 2 && pathByIndex[20] == "buf -> start -> p" && + pathByIndex[21] == "buf -> start -> q", + "StackBufferOverflow: each PHI of a cycle resolves to the array through " + "its decay"); + + const std::vector stores = + analyzeMultipleStores(*loaded.module, all, config); + report.expect(stores.size() == 1 && stores[0].varName == "buf" && + stores[0].storeCount == 2 && stores[0].distinctIndexCount == 2, + "MultipleStores: both stores through the PHI cycle reach the buffer"); + return report.failures == 0; + } + + /// A ring of pointer variables with one array source, as -O0 code leaves it: p0 = buf, + /// p1 = p0, ..., p0 = p(n-1). Every pointer must find the array, however far from it; the + /// order of the accesses must not change the alias paths; and the walk must finish on a + /// component of thousands of nodes. + bool testStackBufferPointerRing(TestReport& report) + { + using namespace llvm; + using namespace ctrace::stack::analysis; + constexpr unsigned kRing = 5000; + const unsigned accessed[] = {0, kRing / 2, kRing - 1}; + + LLVMContext context; + Module module("pointer_ring", context); + const auto build = [&](const char* name, bool reversed) + { + Function* F = Function::Create(FunctionType::get(Type::getVoidTy(context), false), + Function::ExternalLinkage, name, module); + IRBuilder<> builder(BasicBlock::Create(context, "entry", F)); + Type* bufType = ArrayType::get(builder.getInt8Ty(), 16); + AllocaInst* buf = builder.CreateAlloca(bufType, nullptr, "buf"); + std::vector pointers; + for (unsigned i = 0; i < kRing; ++i) + pointers.push_back( + builder.CreateAlloca(builder.getPtrTy(), nullptr, "p" + std::to_string(i))); + Value* decay = builder.CreateInBoundsGEP( + bufType, buf, {builder.getInt64(0), builder.getInt64(0)}, "arraydecay"); + builder.CreateStore(decay, pointers[0]); + for (unsigned i = 1; i < kRing; ++i) + builder.CreateStore(builder.CreateLoad(builder.getPtrTy(), pointers[i - 1]), + pointers[i]); + builder.CreateStore(builder.CreateLoad(builder.getPtrTy(), pointers[kRing - 1]), + pointers[0]); + for (unsigned a = 0; a < 3; ++a) + { + // One out-of-bounds constant index per access (16, 17, 18), so the accesses can + // be told apart and MultipleStores sees distinct index expressions. + const unsigned which = reversed ? 2 - a : a; + Value* pointer = builder.CreateLoad(builder.getPtrTy(), pointers[accessed[which]]); + Value* slot = builder.CreateInBoundsGEP(builder.getInt8Ty(), pointer, + builder.getInt64(16 + which)); + builder.CreateStore(builder.getInt8(1), slot); + } + builder.CreateRetVoid(); + }; + build("ring", false); + build("ring_reversed", true); + + const ctrace::stack::AnalysisConfig config; + const auto pathsOf = [&](const char* name) + { + std::map> paths; + for (const StackBufferOverflowIssue& issue : analyzeStackBufferOverflows( + module, [&](const Function& F) { return F.getName() == name; }, config)) + { + if (issue.indexIsConstant && issue.varName == "buf") + paths[issue.indexOrUpperBound] = issue.aliasPathVec; + } + return paths; + }; + const auto ring = pathsOf("ring"); + const auto reversed = pathsOf("ring_reversed"); + const auto reaches = [&](std::uint64_t index, unsigned k) + { + const auto path = ring.find(index); + return path != ring.end() && path->second.size() == k + 3 && + path->second.front() == "p" + std::to_string(k) && path->second.back() == "buf"; + }; + report.expect(ring.size() == 3 && reaches(16, 0) && reaches(17, kRing / 2) && + reaches(18, kRing - 1), + "StackBufferOverflow: every pointer of a ring of 5000 finds the array, " + "2500 and 4999 steps away included"); + report.expect(reversed == ring, "StackBufferOverflow: the alias paths of a ring do not " + "depend on the order of the accesses"); + + const std::vector stores = + analyzeMultipleStores(module, [](const Function&) { return true; }, config); + report.expect(stores.size() == 2 && std::all_of(stores.begin(), stores.end(), + [](const MultipleStoreIssue& issue) + { + return issue.varName == "buf" && + issue.storeCount == 3 && + issue.distinctIndexCount == 3; + }), + "MultipleStores: the three stores through the ring reach the buffer in " + "both functions"); + return report.failures == 0; + } + } // namespace int main(int argc, char** argv) @@ -2795,6 +2921,8 @@ int main(int argc, char** argv) (void)testZ3SymbolsWithOneNameStayApart(report); #endif (void)testDuplicateIfIndexWidths(repoRoot, report); + (void)testStackBufferPhiCycle(repoRoot, report); + (void)testStackBufferPointerRing(report); (void)testResourceModelConditions(report); (void)testOwnershipFactCollector(repoRoot, report); (void)testOwnershipCollectorExceptions(repoRoot, report);