From 0eb38770bdc18b46e6ea34c57063a8af3e3646ec Mon Sep 17 00:00:00 2001 From: acqn Date: Mon, 13 Nov 2023 17:17:46 +0800 Subject: [PATCH 1/2] Fixed const qualifiers on named structs/unions members that should prevent assignments to the whole structs/unions. Added warning on ignored qualifiers on anonymous structs/unions. --- src/cc65/assignment.c | 7 ++-- src/cc65/declare.c | 80 ++++++++++++++++++++++++++++++++----------- src/cc65/symentry.h | 13 ++++++- test/err/bug2018.c | 25 ++++++++++++++ test/val/bug2018-ok.c | 61 +++++++++++++++++++++++++++++++++ 5 files changed, 163 insertions(+), 23 deletions(-) create mode 100644 test/err/bug2018.c create mode 100644 test/val/bug2018-ok.c diff --git a/src/cc65/assignment.c b/src/cc65/assignment.c index e0ec80fde..e51dde9ab 100644 --- a/src/cc65/assignment.c +++ b/src/cc65/assignment.c @@ -44,6 +44,7 @@ #include "scanner.h" #include "stackptr.h" #include "stdnames.h" +#include "symentry.h" #include "typecmp.h" #include "typeconv.h" @@ -82,6 +83,8 @@ static void CopyStruct (ExprDesc* LExpr, ExprDesc* RExpr) if (TypeCmp (ltype, RExpr->Type).C < TC_STRICT_COMPATIBLE) { TypeCompatibilityDiagnostic (ltype, RExpr->Type, 1, "Incompatible types in assignment to '%s' from '%s'"); + } else if (SymHasConstMember (ltype->A.S)) { + Error ("Assignment to read only variable"); } /* Do we copy the value directly using the primary? */ @@ -627,7 +630,7 @@ void OpAssign (const GenDesc* Gen, ExprDesc* Expr, const char* Op) } } else if (IsQualConst (ltype)) { /* Check for assignment to const */ - Error ("Assignment to const"); + Error ("Assignment to const variable"); } } @@ -686,7 +689,7 @@ void OpAddSubAssign (const GenDesc* Gen, ExprDesc *Expr, const char* Op) Error ("Invalid lvalue in assignment"); } else if (IsQualConst (Expr->Type)) { /* The left side must not be const qualified */ - Error ("Assignment to const"); + Error ("Assignment to const variable"); } } diff --git a/src/cc65/declare.c b/src/cc65/declare.c index 4bfd52c5e..ccea0098a 100644 --- a/src/cc65/declare.c +++ b/src/cc65/declare.c @@ -994,10 +994,14 @@ static SymEntry* ParseUnionSpec (const char* Name, unsigned* DSFlags) ** a union. */ if (IS_Get (&Standard) >= STD_CC65 && IsClassStruct (Decl.Type)) { - /* This is an anonymous struct or union. Copy the fields - ** into the current level. - */ - AnonFieldName (Decl.Ident, "field", UnionTagEntry->V.S.ACount); + /* This is an anonymous struct or union */ + AnonFieldName (Decl.Ident, GetBasicTypeName (Decl.Type), UnionTagEntry->V.S.ACount); + + /* Ignore CVR qualifiers */ + if (IsQualConst (Decl.Type) || IsQualVolatile (Decl.Type) || IsQualRestrict (Decl.Type)) { + Warning ("Anonymous %s qualifiers are ignored", GetBasicTypeName (Decl.Type)); + Decl.Type[0].C &= ~T_QUAL_CVR; + } } else { /* A non bit-field without a name is legal but useless */ Warning ("Declaration does not declare anything"); @@ -1011,13 +1015,18 @@ static SymEntry* ParseUnionSpec (const char* Name, unsigned* DSFlags) GetFullTypeName (Decl.Type)); } + /* Check for const types */ + if (IsQualConst (Decl.Type)) { + Flags |= SC_HAVECONST; + } + /* Handle sizes */ FieldSize = SizeOf (Decl.Type); if (FieldSize > UnionSize) { UnionSize = FieldSize; } - /* Add a field entry to the table. */ + /* Add a field entry to the table */ if (FieldWidth > 0) { /* For a union, allocate space for the type specified by the ** bit-field. @@ -1025,19 +1034,30 @@ static SymEntry* ParseUnionSpec (const char* Name, unsigned* DSFlags) AddBitField (Decl.Ident, Decl.Type, 0, 0, FieldWidth, SignednessSpecified); } else if (Decl.Ident[0] != '\0') { + /* Add the new field to the table */ Field = AddLocalSym (Decl.Ident, Decl.Type, SC_STRUCTFIELD, 0); - if (IsAnonName (Decl.Ident)) { - Field->V.A.ANumber = UnionTagEntry->V.S.ACount++; - AliasAnonStructFields (&Decl, Field); - } - /* Check if the field itself has a flexible array member */ + /* Check the new field for certain kinds of members */ if (IsClassStruct (Decl.Type)) { SymEntry* TagEntry = GetESUTagSym (Decl.Type); + + /* Alias the fields of the anonymous member on the current level */ + if (IsAnonName (Decl.Ident)) { + Field->V.A.ANumber = UnionTagEntry->V.S.ACount++; + AliasAnonStructFields (&Decl, Field); + } + + /* Check if the field itself has a flexible array member */ if (TagEntry && SymHasFlexibleArrayMember (TagEntry)) { Field->Flags |= SC_HAVEFAM; Flags |= SC_HAVEFAM; } + + /* Check if the field itself has a const member */ + if (TagEntry && SymHasConstMember (TagEntry)) { + Field->Flags |= SC_HAVECONST; + Flags |= SC_HAVECONST; + } } } @@ -1190,10 +1210,14 @@ static SymEntry* ParseStructSpec (const char* Name, unsigned* DSFlags) */ if (IS_Get (&Standard) >= STD_CC65 && IsClassStruct (Decl.Type)) { - /* This is an anonymous struct or union. Copy the - ** fields into the current level. - */ - AnonFieldName (Decl.Ident, "field", StructTagEntry->V.S.ACount); + /* This is an anonymous struct or union */ + AnonFieldName (Decl.Ident, GetBasicTypeName (Decl.Type), StructTagEntry->V.S.ACount); + + /* Ignore CVR qualifiers */ + if (IsQualConst (Decl.Type) || IsQualVolatile (Decl.Type) || IsQualRestrict (Decl.Type)) { + Warning ("Anonymous %s qualifiers are ignored", GetBasicTypeName (Decl.Type)); + Decl.Type[0].C &= ~T_QUAL_CVR; + } } else { /* A non bit-field without a name is legal but useless */ Warning ("Declaration does not declare anything"); @@ -1211,6 +1235,11 @@ static SymEntry* ParseStructSpec (const char* Name, unsigned* DSFlags) GetFullTypeName (Decl.Type)); } + /* Check for const types */ + if (IsQualConst (Decl.Type)) { + Flags |= SC_HAVECONST; + } + /* Add a field entry to the table */ if (FieldWidth > 0) { /* Full bytes have already been added to the StructSize, @@ -1223,24 +1252,35 @@ static SymEntry* ParseStructSpec (const char* Name, unsigned* DSFlags) FieldWidth, SignednessSpecified); BitOffs += FieldWidth; CHECK (BitOffs <= CHAR_BITS * SizeOf (Decl.Type)); - /* Add any full bytes to the struct size. */ + /* Add any full bytes to the struct size */ StructSize += BitOffs / CHAR_BITS; BitOffs %= CHAR_BITS; } else if (Decl.Ident[0] != '\0') { + /* Add the new field to the table */ Field = AddLocalSym (Decl.Ident, Decl.Type, SC_STRUCTFIELD, StructSize); - if (IsAnonName (Decl.Ident)) { - Field->V.A.ANumber = StructTagEntry->V.S.ACount++; - AliasAnonStructFields (&Decl, Field); - } - /* Check if the field itself has a flexible array member */ + /* Check the new field for certain kinds of members */ if (IsClassStruct (Decl.Type)) { SymEntry* TagEntry = GetESUTagSym (Decl.Type); + + /* Alias the fields of the anonymous member on the current level */ + if (IsAnonName (Decl.Ident)) { + Field->V.A.ANumber = StructTagEntry->V.S.ACount++; + AliasAnonStructFields (&Decl, Field); + } + + /* Check if the field itself has a flexible array member */ if (TagEntry && SymHasFlexibleArrayMember (TagEntry)) { Field->Flags |= SC_HAVEFAM; Flags |= SC_HAVEFAM; Error ("Invalid use of struct with flexible array member"); } + + /* Check if the field itself has a const member */ + if (TagEntry && SymHasConstMember (TagEntry)) { + Field->Flags |= SC_HAVECONST; + Flags |= SC_HAVECONST; + } } if (!FlexibleMember) { diff --git a/src/cc65/symentry.h b/src/cc65/symentry.h index 76a4272ae..715c036d6 100644 --- a/src/cc65/symentry.h +++ b/src/cc65/symentry.h @@ -108,6 +108,7 @@ struct CodeEntry; #define SC_ALIAS 0x01000000U /* Alias of global or anonymous field */ #define SC_FICTITIOUS 0x02000000U /* Symbol is fictitious (for error recovery) */ #define SC_HAVEFAM 0x04000000U /* Type has a Flexible Array Member */ +#define SC_HAVECONST 0x08000000U /* Type has a const member */ @@ -275,6 +276,16 @@ INLINE int SymHasFlexibleArrayMember (const SymEntry* Sym) # define SymHasFlexibleArrayMember(Sym) (((Sym)->Flags & SC_HAVEFAM) == SC_HAVEFAM) #endif +#if defined(HAVE_INLINE) +INLINE int SymHasConstMember (const SymEntry* Sym) +/* Return true if the given entry has a const member */ +{ + return ((Sym->Flags & SC_HAVECONST) == SC_HAVECONST); +} +#else +# define SymHasConstMember(Sym) (((Sym)->Flags & SC_HAVECONST) == SC_HAVECONST) +#endif + #if defined(HAVE_INLINE) INLINE const char* SymGetAsmName (const SymEntry* Sym) /* Return the assembler label name for the symbol (beware: may be NULL!) */ @@ -282,7 +293,7 @@ INLINE const char* SymGetAsmName (const SymEntry* Sym) return Sym->AsmName; } #else -# define SymGetAsmName(Sym) ((Sym)->AsmName) +# define SymGetAsmName(Sym) ((Sym)->AsmName) #endif const DeclAttr* SymGetAttr (const SymEntry* Sym, DeclAttrType AttrType); diff --git a/test/err/bug2018.c b/test/err/bug2018.c new file mode 100644 index 000000000..61892992e --- /dev/null +++ b/test/err/bug2018.c @@ -0,0 +1,25 @@ +/* Bug #2018 - Compiler has problems with const struct fields */ + +struct X { + struct { + int a; + } a; + union { + int a; + const int b; + } b; +}; + +struct X f(void) +{ + struct X x = { 42 }; + return x; +} + +int main(void) +{ + struct X x = { 0 }; + x = f(); /* Error since X is read only */ + + return 0; +} diff --git a/test/val/bug2018-ok.c b/test/val/bug2018-ok.c new file mode 100644 index 000000000..7046e1d1f --- /dev/null +++ b/test/val/bug2018-ok.c @@ -0,0 +1,61 @@ +/* Bug #2018 - Compiler has problems with const struct fields */ + +#include + +unsigned failures; + +struct X { + const struct { /* Qualifier ignored in cc65 */ + int a; + }; + const union { /* Qualifier ignored in cc65 */ + int b; + }; +}; + +union Y { + const struct { /* Qualifier ignored in cc65 */ + int a; + }; + const union { /* Qualifier ignored in cc65 */ + int b; + }; +}; + +struct X f(struct X a) +{ + struct X x = { 42 }; + return --a.a ? a : x; +} + +union Y g(union Y a) +{ + union Y y = { 42 }; + return --a.a ? a : y; +} + +int main(void) +{ + struct X x = { 1 }; + union Y y = { 1 }; + + x = f(x); /* Allowed in cc65 since X is not read only */ + y = g(y); /* Allowed in cc65 since Y is not read only */ + + if (x.a != 42) + { + ++failures; + } + + if (y.a != 42) + { + ++failures; + } + + if (failures > 0) + { + printf("failures: %u\n", failures); + } + + return failures; +} From 5332eeecc24cfca8777471fd3839f26579b8e52e Mon Sep 17 00:00:00 2001 From: acqn Date: Wed, 15 Nov 2023 18:17:36 +0800 Subject: [PATCH 2/2] Fixed empty declarations in structs/unions. --- src/cc65/declare.c | 120 +++++++++++++++++++++++------------------- test/val/bug2018-ok.c | 10 ++++ 2 files changed, 76 insertions(+), 54 deletions(-) diff --git a/src/cc65/declare.c b/src/cc65/declare.c index ccea0098a..6615998ce 100644 --- a/src/cc65/declare.c +++ b/src/cc65/declare.c @@ -983,33 +983,36 @@ static SymEntry* ParseUnionSpec (const char* Name, unsigned* DSFlags) /* Check for a bit-field declaration */ FieldWidth = ParseFieldWidth (&Decl); - /* Ignore zero sized bit fields in a union */ - if (FieldWidth == 0) { - goto NextMember; - } - - /* Check for fields without a name */ + /* Check for fields without names */ if (Decl.Ident[0] == '\0') { - /* In cc65 mode, we allow anonymous structs/unions within - ** a union. - */ - if (IS_Get (&Standard) >= STD_CC65 && IsClassStruct (Decl.Type)) { - /* This is an anonymous struct or union */ - AnonFieldName (Decl.Ident, GetBasicTypeName (Decl.Type), UnionTagEntry->V.S.ACount); + if (FieldWidth < 0) { + /* In cc65 mode, we allow anonymous structs/unions within + ** a union. + */ + SymEntry* TagEntry; + if (IS_Get (&Standard) >= STD_CC65 && + IsClassStruct (Decl.Type) && + (TagEntry = GetESUTagSym (Decl.Type)) && + SymHasAnonName (TagEntry)) { + /* This is an anonymous struct or union */ + AnonFieldName (Decl.Ident, GetBasicTypeName (Decl.Type), UnionTagEntry->V.S.ACount); - /* Ignore CVR qualifiers */ - if (IsQualConst (Decl.Type) || IsQualVolatile (Decl.Type) || IsQualRestrict (Decl.Type)) { - Warning ("Anonymous %s qualifiers are ignored", GetBasicTypeName (Decl.Type)); - Decl.Type[0].C &= ~T_QUAL_CVR; + /* Ignore CVR qualifiers */ + if (IsQualConst (Decl.Type) || IsQualVolatile (Decl.Type) || IsQualRestrict (Decl.Type)) { + Warning ("Anonymous %s qualifiers are ignored", GetBasicTypeName (Decl.Type)); + Decl.Type[0].C &= ~T_QUAL_CVR; + } + } else { + /* A non bit-field without a name is legal but useless */ + Warning ("Declaration does not declare anything"); + + goto NextMember; } - } else { - /* A non bit-field without a name is legal but useless */ - Warning ("Declaration does not declare anything"); + } else if (FieldWidth > 0) { + /* A bit-field without a name will get an anonymous one */ + AnonName (Decl.Ident, "bit-field"); } - } - - /* Check for incomplete types including 'void' */ - if (IsIncompleteType (Decl.Type)) { + } else if (IsIncompleteType (Decl.Type)) { Error ("Field '%s' has incomplete type '%s'", Decl.Ident, GetFullTypeName (Decl.Type)); @@ -1020,6 +1023,11 @@ static SymEntry* ParseUnionSpec (const char* Name, unsigned* DSFlags) Flags |= SC_HAVECONST; } + /* Ignore zero sized bit fields in a union */ + if (FieldWidth == 0) { + goto NextMember; + } + /* Handle sizes */ FieldSize = SizeOf (Decl.Type); if (FieldSize > UnionSize) { @@ -1180,36 +1188,17 @@ static SymEntry* ParseStructSpec (const char* Name, unsigned* DSFlags) } } - /* Apart from the above, a bit field with width 0 is not processed - ** further. - */ - if (FieldWidth == 0) { - goto NextMember; - } - - /* Check if this field is a flexible array member, and - ** calculate the size of the field. - */ - if (IsTypeArray (Decl.Type) && GetElementCount (Decl.Type) == UNSPECIFIED) { - /* Array with unspecified size */ - if (StructSize == 0) { - Error ("Flexible array member cannot be first struct field"); - } - FlexibleMember = 1; - Flags |= SC_HAVEFAM; - - /* Assume zero for size calculations */ - SetElementCount (Decl.Type, FLEXIBLE); - } - /* Check for fields without names */ if (Decl.Ident[0] == '\0') { if (FieldWidth < 0) { /* In cc65 mode, we allow anonymous structs/unions within ** a struct. */ - if (IS_Get (&Standard) >= STD_CC65 && IsClassStruct (Decl.Type)) { - + SymEntry* TagEntry; + if (IS_Get (&Standard) >= STD_CC65 && + IsClassStruct (Decl.Type) && + (TagEntry = GetESUTagSym (Decl.Type)) && + SymHasAnonName (TagEntry)) { /* This is an anonymous struct or union */ AnonFieldName (Decl.Ident, GetBasicTypeName (Decl.Type), StructTagEntry->V.S.ACount); @@ -1221,18 +1210,34 @@ static SymEntry* ParseStructSpec (const char* Name, unsigned* DSFlags) } else { /* A non bit-field without a name is legal but useless */ Warning ("Declaration does not declare anything"); + + goto NextMember; } - } else { + } else if (FieldWidth > 0) { /* A bit-field without a name will get an anonymous one */ AnonName (Decl.Ident, "bit-field"); } - } + } else { + /* Check if this field is a flexible array member, and + ** calculate the size of the field. + */ + if (IsTypeArray (Decl.Type) && GetElementCount (Decl.Type) == UNSPECIFIED) { + /* Array with unspecified size */ + if (StructSize == 0) { + Error ("Flexible array member cannot be first struct field"); + } + FlexibleMember = 1; + Flags |= SC_HAVEFAM; - /* Check for incomplete types including 'void' */ - if (IsIncompleteType (Decl.Type)) { - Error ("Field '%s' has incomplete type '%s'", - Decl.Ident, - GetFullTypeName (Decl.Type)); + /* Assume zero for size calculations */ + SetElementCount (Decl.Type, FLEXIBLE); + } + + if (IsIncompleteType (Decl.Type)) { + Error ("Field '%s' has incomplete type '%s'", + Decl.Ident, + GetFullTypeName (Decl.Type)); + } } /* Check for const types */ @@ -1240,6 +1245,13 @@ static SymEntry* ParseStructSpec (const char* Name, unsigned* DSFlags) Flags |= SC_HAVECONST; } + /* Apart from the above, a bit field with width 0 is not processed + ** further. + */ + if (FieldWidth == 0) { + goto NextMember; + } + /* Add a field entry to the table */ if (FieldWidth > 0) { /* Full bytes have already been added to the StructSize, diff --git a/test/val/bug2018-ok.c b/test/val/bug2018-ok.c index 7046e1d1f..567beb301 100644 --- a/test/val/bug2018-ok.c +++ b/test/val/bug2018-ok.c @@ -5,6 +5,11 @@ unsigned failures; struct X { + const int; /* Useless empty declaration */ + const void; /* Useless empty declaration */ + const struct U; /* Useless(?) declaration */ + const struct V { int a; }; /* Useless(?) declaration */ + const struct { /* Qualifier ignored in cc65 */ int a; }; @@ -14,6 +19,11 @@ struct X { }; union Y { + const int; /* Useless empty declaration */ + const void; /* Useless empty declaration */ + const union W; /* Useless(?) declaration */ + const union T { int a; }; /* Useless(?) declaration */ + const struct { /* Qualifier ignored in cc65 */ int a; };