perf(stack-buffer): resolve pointer variables once per function (#215)

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.

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

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

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;
+}