Re: [Tinycc-devel] missing check after calling type_size in classify_x86_64_arg

[Pascal Cuoq]

Hello Michael, and thanks for the guidance.

On 22 Jun 2019, at 01:17, Michael Matz <address@hidden> wrote:

Yes, there are generally two contexts, and in one of them (e.g. decls with initializers) incomplete types are temporarily valid.  So you'd either need two modes of type_size (one complaining), or two functions (or, as now, checking the size sometimes).  If you want to invest more work than just adding a check in classify_x86_64_arg, instead add a function ctype_size (c for complete) which complains on incomplete types, and use it in places where the code really needs complete types (and doesn't yet check on its own).

That is a big can of worm you have pointed me to. Here is another part of the code that seems wrong and continues to seem wrong even with the suggested change:

static void struct_layout(CType *type, AttributeDef *ad)

{

    int size, align, maxalign, offset, c, bit_pos, bit_size;

    int packed, a, bt, prevbt, prev_bit_size;

    int pcc = !tcc_state->ms_bitfields;

    int pragma_pack = *tcc_state->pack_stack_ptr;

    Sym *f;

    maxalign = 1;

    offset = 0;

    c = 0;

    bit_pos = 0;

    prevbt = VT_STRUCT; /* make it never match */

    prev_bit_size = 0;

//#define BF_DEBUG

    for (f = type->ref->next; f; f = f->next) {

        if (f->type.t & VT_BITFIELD)

            bit_size = BIT_SIZE(f->type.t);

        else

            bit_size = -1;

        // call type_size here, because t->type can be incomplete

        // if it is a flexible array member

        size = type_size(&f->type, &align);

        a = f->a.aligned ? 1 << (f->a.aligned - 1) : 0;

        packed = 0;

        if (pcc && bit_size == 0) {

            /* in pcc mode, packing does not affect zero-width bitfields */

        } else {

            /* in pcc mode, attribute packed overrides if set. */

            if (pcc && (f->a.packed || ad->a.packed))

                align = packed = 1;

            /* pragma pack overrides align if lesser and packs bitfields always */

            if (pragma_pack) {

                packed = 1;

                if (pragma_pack < align)

                    align = pragma_pack;

align starts its life as an automatic, uninitialized variable. At each iteration, the call to type_size sets it unless the call fails and leaves align's previous value in it.

My only change so far in this function is the comment “call type_size here, because t->type can be incomplete if it is a flexible array member”: I stand by this comment, because calling ctype_size here makes TCC abort while compiling pcctest.c.

Next if pcc is false and pragma_pack is true, the value of align is used in if (pragma_pack < align)

For a flexible array member of a complete element type, this currently works out fine : size_type stores the element type's alignment even if the array is a FAM.

Now consider a FAM of an incomplete enum:

struct s {

  char a;

  enum e b[];

} s;

Fortunately TCC rejects structs that do not have at least one member of a complete type before the FAM, but still, in the above example the value of align that is used in if (pragma_pack < align) is the value from the previous iteration, that is, the alignment from the previous member, and that seems awfully wrong (the program should simply be rejected before reaching that point).

To make a long story short, as the most pressing improvement to clarify the behavior of TCC with incomplete types, I find myself trying to make type_size reject each of following struct declarations:

$ cat t.c

struct s {

  char a;

  enum e b[];

} s;

struct t {

  int a[3];

  void b[];

} t;

typedef void u;

struct v {

  int a[3];

  u b[];

} v;

struct w {

  int a[3];

  struct n b[];

} w;

int printf(const char *, ...);

int main(void) {

  printf("stringlit: %zu\n", sizeof "abcd");

  printf("%zu\n", sizeof s);

  printf("%p\n", &s);

  printf("%p\n", &s.b);

  printf("%zu\n", sizeof(u));

  printf("%zu\n", sizeof t);

  printf("%zu\n", sizeof v);

  printf("%zu\n", sizeof w);

}

pascal@TrustInSoft-Box-VII:~/tinycc_mob$ ./tcc -run t.c

stringlit: 5

1

0xfd6778

0xfd6779

1

12

12

0

And the changes I currently have for this purpose are the following, but they are not sufficient to make TCC reject the declaration of w of type struct w

 ST_FUNC int type_size(CType *type, int *a)

 {

     Sym *s;

@@ -2786,11 +2798,14 @@ ST_FUNC int type_size(CType *type, int *a)

             int ts;

 

             s = type->ref;

+            if ((s->type.t & VT_BTYPE) == VT_VOID)

+                tcc_error("array of void");

             ts = type_size(&s->type, a);

-

-            if (ts < 0 && s->c < 0)

+            if (ts < 0 && s->c < 0) {

+                if (IS_ENUM(s->type.t))

+                    tcc_error("array of incomplete enum");

                 ts = -ts;

-

+            }

             return ts * s->c;

         } else {

             *a = PTR_SIZE;