Re: [Tinycc-devel] Fwd: TCC VLA bug?

[Michael Matz]

Hello grischka,

On Tue, 7 Dec 2021, grischka wrote:

> Michael Matz wrote:
> >  So, the problematic type was:
> >
> >    int (*)[a][b]
> >
> >  That's pointer to an vla-array of an vla-array of int.  All three (inner
> >  array, outer array and pointer, but not the int) should be marked
> >  VT_VLA.
>
> Hm...,
>
> IMO one (very) invariant convention is that VT_ARRAY rsp. VT_VLA
> always also have VT_PTR.
>
> Therefor if the outer pointer would have VT_VLA as you say above,
> then it would be not a pointer to a VLA, but it would be a VLA,
> as in
>
>    int arr[z][a][b];

Right, sorry.  It really doesn't help to be imprecise here, like I was. 
In the type 'int (*)[a][b]' the outer pointer doesn't need VT_VLA, but the 
inner two array types do (and the int itself doesn't).  But I was wiggling 
around because I also wanted to avoid really looking into it :) 
TCC should generate four CType structs for this type:

  CType *t;           // the full thing, VT_PTR only, 'int (*)[a][b]'
  CType *o = t->ref;  // outer vla, VT_PTR|VT_VLA, 'int [a][b]'
  CType *i = o->ref;  // inner vla, VT_PTR|VT_VLA, 'int [b]'
  CType *e = i->ref;  // element, 'int'

And without looking I wasn't sure it really does (I still am not) ;-)

> Anyway, now that I already did look into it (which initially I was
> trying to avoid ;) I would maybe just replace all this:
>
> >              if (type1.ref->type.t & VT_VLA)
> >                  vla_runtime_pointed_size(&type1);
> >              else {
> >                  u = pointed_size(&type1);
> >                  if (u < 0)
> >                      tcc_error("unknown array element size");
> >         #if PTR_SIZE == 8
> >                  vpushll(u);
> >         #else
> >                  /* XXX: cast to int ? (long long case) */
> >                  vpushi(u);
> >         #endif
>
> by that single line:
>
>            vla_runtime_type_size(pointed_type(&vtop[-1].type), &align):

Yeah, I thought so as well when (very quickly! :) ) looking at this.  Also 
in some other places that currently only conditionally call this.

> which looks as it would do the right thing automatically also for the 
> normal array (non-vla) case (but except the "unknown array size" check).
>
> Of course one could rename "vla_runtime_type_size" to something better, 
> then.
>
> Btw, now that I already did look into it (which initially ... ;) I think
> I found 2 related problems as can be seen with this (added to the original
> test case):
>
>     // int (*arr)[h][w];
>     printf("diff = %d, size = %d/%d\n",
>         arr + 1 - arr, sizeof *arr, sizeof (*arr + 1));

Yeah, during my quick scanning of tccgen.c I also found code that made me 
think 'huh?', and imagined there could be more errors like this, 
especially with multi-dimensional VLAs.  Alas, I wanted to not get into 
tcc hacking this week :)  Temptations everywhere!


Ciao,
Michael.

> -- gr
>
>
> >  In TCC we collapse the outer array+pointer into one type (a
> >  pointer that also has VT_ARRAY/VT_VLA set), so there actually should be
> >  two levels: the inner level a VT_VLA pointing to the VT_INT (with its
> >  VLA runtime length being evaluated to sizeof(int) * b) and the outer
> >  level a VT_VLA pointing to the inner VT_VLA (and its VLA runtime length
> >  being evaluated to inner length * a).
> >
> >  I'm surprised that your patch didn't cause other problems, it seems
> >  either the testsuite isn't testing VLAs very much, or that the VT_VLA
> >  flag is set on types where it shouldn't have been (e.g. on the VT_INT
> >  type that is in the type->ref of the 'int [n]' array type).
> >
> >
> >  Ciao,
> >  Michael.
> >
> >
> >  ------------------------------------------------------------------------
> >
> >  _______________________________________________
> >  Tinycc-devel mailing list
> >  Tinycc-devel@nongnu.org
> >  https://lists.nongnu.org/mailman/listinfo/tinycc-devel
>
>
>
> _______________________________________________
> Tinycc-devel mailing list
> Tinycc-devel@nongnu.org
> https://lists.nongnu.org/mailman/listinfo/tinycc-devel