Re: [Tinycc-devel] tccgen.c: off by one in flexible array members

[Michael Matz]

Hi,

On Wed, 9 Mar 2016, Henry Kroll wrote:

> I created a + 1 patch that seems to work, but I need to run more tests
> before committing.
>
> =====================================================
> diff --git a/tccgen.c b/tccgen.c
> index 3cd28ed..270d11c 100644
> --- a/tccgen.c
> +++ b/tccgen.c
> @@ -5847,7 +5847,7 @@ static void decl_initializer_alloc(CType *type,
> AttributeDef *ad, int r,
>              tcc_error("unknown type size");
>      }
>      if (flexible_array)
> -        size += flexible_array->type.ref->c *
> pointed_size(&flexible_array->type);
> +        size += flexible_array->type.ref->c *
> pointed_size(&flexible_array->type) + 1;
>      /* take into account specified alignment if bigger */
>      if (ad->a.aligned) {
>          if (ad->a.aligned > align)
> =====================================================
>
> Have a great day!

This still isn't quite right.  The thing is, type->ref.c is -1 for an 
unused flex array, so with your example ("char mem[]") your change 
basically boils down to "size += -1 * 1 + 1" (i.e. +=0).  But with a 
different type (say int) it's now "size += -1 * 4 + 1" (i.e. +=3).  So 
this example still fails the same way:

-----------------
#include <stdio.h>
struct w {
    char *data; int mem[];
};
int main (void) {
   char s[9]="nonono"; struct w q = {"bugs"};
   printf ("tcc has %s %s\n", s, q.data);
   return !s[0];
}
-----------------

(only changed the mem[] member type).  The +1 also looks conceptually 
wrong, we try to adjust the size by some number of elements.  The example 
works of course if you'd do
  size += (flexible_array->type.ref->c + 1)
          * pointed_size(&flexible_array->type)

But then it'll break this example: (well, not break it, but it seems it'd 
allocate more then necessary), which actually does use the flex member:

-----------------
#include <stdio.h>
struct w {
    char *data; int mem[];
};
int main (void) {
   char s[9]="nonono";
   struct w q = {"bugs", { 1 }};
   printf ("tcc has %s %s\n", s, q.data);
   return !s[0];
}
-----------------

Here type->ref.c (after parsing the initializer) will be 1, the number of 
elements used, and the original size calculation seems correct. 
type->ref.c will be 0 if the flex member is there but empty (in the 
initializer), so for that case the original is also correct.

So, I think it's more correct to special case the ref->c == -1 case only 
(don't adjust size in that case), instead of playing +-1 tricks (as in, 
it's not a off-by-one error).  Will think a bit over dinner :)


diff --git a/tccgen.c b/tccgen.c
index 270d11c..896a520 100644
--- a/tccgen.c
+++ b/tccgen.c
@@ -5846,8 +5846,10 @@ static void decl_initializer_alloc(CType *type, 
AttributeDef *ad, int r,
         if (size < 0)
             tcc_error("unknown type size");
     }
-    if (flexible_array)
-        size += flexible_array->type.ref->c * 
pointed_size(&flexible_array->type) + 1;
+    if (flexible_array
+       /* If the flex member was used in the initializer.  */
+       && flexible_array->type.ref->c > 0)
+        size += flexible_array->type.ref->c * 
pointed_size(&flexible_array->type);
     /* take into account specified alignment if bigger */
     if (ad->a.aligned) {
         if (ad->a.aligned > align)


Ciao,
Michael.