Cc65: Bad struct/union size calculation

[Ymhr6]

This struct resolve in sizeof = 7.
With other compiler this struct resolve in bigger size.
This compiler I think should resolve the struct in sizeof = 16

typedef struct Cl_Read_FieldMethod
{
    unsigned short int access_flags;
    unsigned short int name_index;
    unsigned short int descriptor_index;
    union
    {
        struct
        {
            unsigned short int max_stack;
            unsigned short int max_locals;
            unsigned long int code_length;
            unsigned char* code;
        };
        unsigned char field_constant;
    };
} Cl_Read_FieldMethod;

[mrdudz]

ugh, how ugly, not surprised the compiler fails at this :=P

if you really want to to use it like this, i'd try filling up both "alternatives" in the union to the same size. i guess the compiler will only take the "unsigned char field_constant;" because it expects all members in the union to have the same access size (which then results in sizeof=7 in this case indeed)

[Ymhr6]

The problem is that the code that access for example member max_locals, will access it with an offset of 8 from the base pointer.... (and the size reserved is 7.....)

[Marrin]

@mrdudz Why are you not surprised? Just picking the size of one (more or less arbitrary) alternative in a union instead of the largest is what I find surprising, because all other C compilers I've used so far, get this right.

[mrdudz]

because its something that is error-prone, probably not so commonly used feature, and i have seen unions breaking things in other compilers before :)

what does cc65 make out of it when you put the "unsigned char field_constant" first before the union?

[Ymhr6]

@mrdudz : putting field_constant as first member of the union change nothing, sizeof = 7.
I think this is a bug, because all other compiler I tried are fine with this kind of struct/union.

[mrdudz]

think this is a bug, because all other compiler I tried are fine with this kind of struct/union.

sure, its a bug, no doubt :)

[Ymhr6]

Found the bug.
declare.c
function ParseUnionDecl

Rows 656-678

/* Check for fields without a name */
            if (Decl.Ident[0] == '\0') {
                /* In cc65 mode, we allow anonymous structs/unions within
                ** a struct.
                */
                if (IS_Get (&Standard) >= STD_CC65 && IsClassStruct (Decl.Type)) {
                    /* This is an anonymous struct or union. Copy the fields
                    ** into the current level.
                    */
                    CopyAnonStructFields (&Decl, 0);

                } else {
                    /* A non bit-field without a name is legal but useless */
                    Warning ("Declaration does not declare anything");
                }
                goto NextMember;
            }

            /* Handle sizes */
            FieldSize = CheckedSizeOf (Decl.Type);
            if (FieldSize > UnionSize) {
                UnionSize = FieldSize;
            }

When the field is without a name (the struct in the union), execute the function CopyAnonStructFields and then goto NextMember, without running the code for the UnionSize update.

A quick fix could be substitute row 665 from

CopyAnonStructFields (&Decl, 0);

to

FieldSize = CopyAnonStructFields (&Decl, 0);
if (FieldSize > UnionSize) {
    UnionSize = FieldSize;
}

[oliverschmidt]

@Ymhr6: Thanks for going beyond a pure bug report by actually locating the root cause and proposing a fix :-))