Bug#1152433: 4ti2: FTBFS on ppc64el with GCC 15/16 (-O2/-O3): out-of-bounds array access in VectorAux (src/ppi/ppi.cpp)

[Viktor Pashaiev]

Source: 4ti2
Version: 1.6.15+ds-1
Severity: important
Tags: patch upstream
Forwarded: https://github.com/4ti2/4ti2/pull/61

Dear Maintainer,

4ti2 fails its build-time test suite (test/ppi/check_ppi) on ppc64el when built 
with GCC 15/16 at -O2 or -O3 (including Ubuntu's default -O3 on ppc64el, 
tracked in https://bugs.launchpad.net/ubuntu/+source/4ti2/+bug/2170794).

In src/ppi/ppi.cpp, struct VectorAux declares a fixed-size array `signed char 
Stuff[4]`, stores the vector length in `Stuff[0]`, and returns `Stuff + 1` from 
`Data()`. Accessing `v(i)` for `i >= 4` indexes `Stuff[i]` past the 4-byte 
array bound.

In `ExtendPPI()`, the loop:

  for (int p = 1; p <= (n+1)/2; p++) {
    v(p)--, v(n+1-p)--;
    ...
  }

decrements both `v(p)` and `v(n+1-p)`. Because `p` starts within `Stuff[1..3]` 
while `n+1-p` exceeds `Stuff[3]` for `n >= 3`, GCC alias analysis on ppc64el 
concludes that the two accesses cannot alias and schedules the load `lbzu` of 
`v(n+1-p)` before the store `stb` of `v(p)`. When `n + 1` is even and `p == n + 
1 - p` (first hit at `n = 3, p = 2`), `v(2)` is decremented from 0 to -1 
instead of -2, producing the invalid identity `0 -1 0 1` and causing `4ti2-ppi 
5` to output 50 vectors instead of 47.

The attached patch replaces `Stuff[4]` in `VectorAux` with an explicit 
`unsigned char len_` field and a `signed char data_[]` flexible array member. 
Verified on ppc64el at -O3 -flto=auto across ppi5 (47 vectors), ppi10 (2465 
vectors), and ppi16 (99328 vectors).

Thanks,
Viktor Pashaiev

diff --git a/src/ppi/ppi.cpp b/src/ppi/ppi.cpp
index e42f4ae..da9bb12 100644
--- a/src/ppi/ppi.cpp
+++ b/src/ppi/ppi.cpp
@@ -82,21 +82,21 @@ typedef unsigned int attr_type; /* 32 bits are enough */
 struct VectorAux {
   /* order important */
   attr_type Attribute;
-  signed char Stuff[4];
+  unsigned char len_;
+  signed char data_[];
   void *operator new(size_t s, int length) {
-    if (length < 3) length=3;
-    return malloc(s - 3 +length);
+    return malloc(s + length);
   }
   void operator delete(void *v) {
     free(v);
   }
   VectorAux(int length) { Length() = length; }
   VectorAux(const VectorAux &aux) {
-    memcpy(this, &aux, sizeof(VectorAux) -4 + 1 + aux.Length());
+    memcpy(this, &aux, sizeof(VectorAux) + aux.Length());
   }
-  inline unsigned char &Length() { return (unsigned char &) Stuff[0]; }
-  inline int Length() const { return (unsigned char) Stuff[0]; }
-  inline signed char *Data() { return Stuff+1; }
+  inline unsigned char &Length() { return len_; }
+  inline int Length() const { return len_; }
+  inline signed char *Data() { return data_; }
 };
 
 class Vector {
@@ -118,7 +118,7 @@ public:
       if (aux == NULL) 
 	aux = new(v.aux->Length()) VectorAux(*v.aux);
       else if (aux->Length() == v.aux->Length())
-	memcpy(aux, v.aux, sizeof(VectorAux) -4 + 1 + v.aux->Length());
+	memcpy(aux, v.aux, sizeof(VectorAux) + v.aux->Length());
       else {
 	delete aux;
 	aux = new(v.aux->Length()) VectorAux(*v.aux);