From 2c3ca15d901f38e7e955903816f0ad298538dcf4 Mon Sep 17 00:00:00 2001 From: acqn Date: Sat, 12 Nov 2022 12:32:27 +0800 Subject: [PATCH 1/6] Fixed marking unevaluated subexpressions for conditional operator. --- src/cc65/expr.c | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/src/cc65/expr.c b/src/cc65/expr.c index 932663b18..f7180c020 100644 --- a/src/cc65/expr.c +++ b/src/cc65/expr.c @@ -3913,17 +3913,6 @@ static void hieQuest (ExprDesc* Expr) ED_Init (&Expr3); Expr3.Flags = Flags; - NextToken (); - - /* Convert non-integer constant to boolean constant, so that we may just - ** check it in the same way. - */ - if (ED_IsConstTrue (Expr)) { - ED_MakeConstBool (Expr, 1); - } else if (ED_IsConstFalse (Expr)) { - ED_MakeConstBool (Expr, 0); - } - if (!ConstantCond) { /* Condition codes not set, request a test */ ED_RequireTest (Expr); @@ -3935,6 +3924,15 @@ static void hieQuest (ExprDesc* Expr) FalseLab = GetLocalLabel (); g_falsejump (CF_NONE, FalseLab); } else { + /* Convert non-integer constant to boolean constant, so that we + ** may just check it in an easier way later. + */ + if (ED_IsConstTrue (Expr)) { + ED_MakeConstBool (Expr, 1); + } else if (ED_IsConstFalse (Expr)) { + ED_MakeConstBool (Expr, 0); + } + /* Constant boolean subexpression could still have deferred inc/dec ** operations, so just flush their side-effects at this sequence point. */ @@ -3943,9 +3941,18 @@ static void hieQuest (ExprDesc* Expr) if (Expr->IVal == 0) { /* Remember the current code position */ GetCodePos (&SkippedBranch); + + /* Expr2 is unevaluated when the condition is false */ + Expr2.Flags |= E_EVAL_UNEVAL; + } else { + /* Expr3 is unevaluated when the condition is true */ + Expr3.Flags |= E_EVAL_UNEVAL; } } + /* Skip the question mark */ + NextToken (); + /* Parse second expression. Remember for later if it is a NULL pointer ** expression, then load it into the primary. */ @@ -3977,26 +3984,22 @@ static void hieQuest (ExprDesc* Expr) /* Jump around the evaluation of the third expression */ TrueLab = GetLocalLabel (); - ConsumeColon (); - g_jump (TrueLab); /* Jump here if the first expression was false */ g_defcodelabel (FalseLab); } else { if (Expr->IVal == 0) { - /* Expr2 is unevaluated when the condition is false */ - Expr2.Flags |= E_EVAL_UNEVAL; - /* Remove the load code of Expr2 */ RemoveCode (&SkippedBranch); } else { /* Remember the current code position */ GetCodePos (&SkippedBranch); } - ConsumeColon(); } + ConsumeColon (); + /* Parse third expression. Remember for later if it is a NULL pointer ** expression, then load it into the primary. */ @@ -4022,9 +4025,6 @@ static void hieQuest (ExprDesc* Expr) Expr3.Type = PtrConversion (Expr3.Type); if (ConstantCond && Expr->IVal != 0) { - /* Expr3 is unevaluated when the condition is true */ - Expr3.Flags |= E_EVAL_UNEVAL; - /* Remove the load code of Expr3 */ RemoveCode (&SkippedBranch); } From d0c9b2de9902be5d0a33b5d14ceaa1a8144a8f9d Mon Sep 17 00:00:00 2001 From: acqn Date: Sat, 12 Nov 2022 12:34:16 +0800 Subject: [PATCH 2/6] Added basic shift count check for <<= and >>= operations. --- src/cc65/assignment.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/cc65/assignment.c b/src/cc65/assignment.c index 9834ae5d1..549e18e95 100644 --- a/src/cc65/assignment.c +++ b/src/cc65/assignment.c @@ -322,6 +322,12 @@ static void OpAssignBitField (const GenDesc* Gen, ExprDesc* Expr, const char* Op } else if (Gen->Func == g_mod) { Error ("Modulo operation with zero"); } + } else if (Gen->Func == g_asl || Gen->Func == g_asr) { + if (Expr2.IVal < 0) { + Warning ("Shift count '%ld' is negative", Expr2.IVal); + } else if (Expr2.IVal >= (long)(SizeOf (Expr->Type) * 8)) { + Warning ("Shift count '%ld' >= width of type", Expr2.IVal); + } } /* Adjust the types of the operands if needed */ @@ -502,6 +508,12 @@ static void OpAssignArithmetic (const GenDesc* Gen, ExprDesc* Expr, const char* } else if (Gen->Func == g_mod) { Error ("Modulo operation with zero"); } + } else if (Gen->Func == g_asl || Gen->Func == g_asr) { + if (Expr2.IVal < 0) { + Warning ("Shift count '%ld' is negative", Expr2.IVal); + } else if (Expr2.IVal >= (long)(SizeOf (Expr->Type) * 8)) { + Warning ("Shift count '%ld' >= width of type", Expr2.IVal); + } } Gen->Func (Flags | CF_CONST, Expr2.IVal); } From 75be73cc8d4bd24fac6714ef95169555435eeb3e Mon Sep 17 00:00:00 2001 From: acqn Date: Sat, 12 Nov 2022 12:32:27 +0800 Subject: [PATCH 3/6] Added utility functions to acquire bit width of types. --- src/cc65/assignment.c | 86 +++++++++++++++++++--------- src/cc65/datatype.c | 19 +++++++ src/cc65/datatype.h | 9 +++ src/cc65/expr.c | 117 ++++++++++++++++---------------------- src/cc65/ppexpr.c | 127 +++++++++++++++--------------------------- src/cc65/shiftexpr.c | 31 ++++++----- 6 files changed, 199 insertions(+), 190 deletions(-) diff --git a/src/cc65/assignment.c b/src/cc65/assignment.c index 549e18e95..ab501523d 100644 --- a/src/cc65/assignment.c +++ b/src/cc65/assignment.c @@ -315,19 +315,37 @@ static void OpAssignBitField (const GenDesc* Gen, ExprDesc* Expr, const char* Op } else if (Gen->Func == g_sub) { g_dec (Flags | CF_CONST, Expr2.IVal); } else { - if (Expr2.IVal == 0) { - /* Check for div by zero/mod by zero */ - if (Gen->Func == g_div) { - Error ("Division by zero"); - } else if (Gen->Func == g_mod) { - Error ("Modulo operation with zero"); - } - } else if (Gen->Func == g_asl || Gen->Func == g_asr) { - if (Expr2.IVal < 0) { - Warning ("Shift count '%ld' is negative", Expr2.IVal); - } else if (Expr2.IVal >= (long)(SizeOf (Expr->Type) * 8)) { - Warning ("Shift count '%ld' >= width of type", Expr2.IVal); - } + if (!ED_IsUneval (Expr)) { + if (Expr2.IVal == 0) { + /* Check for div by zero/mod by zero */ + if (Gen->Func == g_div) { + Error ("Division by zero"); + } else if (Gen->Func == g_mod) { + Error ("Modulo operation with zero"); + } + } else if (Gen->Func == g_asl || Gen->Func == g_asr) { + const Type* CalType = IntPromotion (Expr->Type); + unsigned ExprBits = BitSizeOf (CalType); + + /* If the shift count is greater than or equal to the width of the + ** promoted left operand, the behaviour is undefined according to + ** the standard. + */ + if (Expr2.IVal < 0) { + Warning ("Negative shift count %ld treated as %u for %s", + Expr2.IVal, + (unsigned)Expr2.IVal & (ExprBits - 1), + GetBasicTypeName (CalType)); + } else if (Expr2.IVal >= (long)ExprBits) { + Warning ("Shift count %ld >= width of %s treated as %u", + Expr2.IVal, + GetBasicTypeName (CalType), + (unsigned)Expr2.IVal & (ExprBits - 1)); + } + + /* Here we simply "wrap" the shift count around the width */ + Expr2.IVal &= ExprBits - 1; + } } /* Adjust the types of the operands if needed */ @@ -501,18 +519,36 @@ static void OpAssignArithmetic (const GenDesc* Gen, ExprDesc* Expr, const char* } else if (Gen->Func == g_sub) { g_dec (Flags | CF_CONST, Expr2.IVal); } else { - if (Expr2.IVal == 0) { - /* Check for div by zero/mod by zero */ - if (Gen->Func == g_div) { - Error ("Division by zero"); - } else if (Gen->Func == g_mod) { - Error ("Modulo operation with zero"); - } - } else if (Gen->Func == g_asl || Gen->Func == g_asr) { - if (Expr2.IVal < 0) { - Warning ("Shift count '%ld' is negative", Expr2.IVal); - } else if (Expr2.IVal >= (long)(SizeOf (Expr->Type) * 8)) { - Warning ("Shift count '%ld' >= width of type", Expr2.IVal); + if (!ED_IsUneval (Expr)) { + if (Expr2.IVal == 0 && !ED_IsUneval (Expr)) { + /* Check for div by zero/mod by zero */ + if (Gen->Func == g_div) { + Error ("Division by zero"); + } else if (Gen->Func == g_mod) { + Error ("Modulo operation with zero"); + } + } else if (Gen->Func == g_asl || Gen->Func == g_asr) { + const Type* CalType = IntPromotion (Expr->Type); + unsigned ExprBits = BitSizeOf (CalType); + + /* If the shift count is greater than or equal to the width of the + ** promoted left operand, the behaviour is undefined according to + ** the standard. + */ + if (Expr2.IVal < 0) { + Warning ("Negative shift count %ld treated as %u for %s", + Expr2.IVal, + (unsigned)Expr2.IVal & (ExprBits - 1), + GetBasicTypeName (CalType)); + } else if (Expr2.IVal >= (long)ExprBits) { + Warning ("Shift count %ld >= width of %s treated as %u", + Expr2.IVal, + GetBasicTypeName (CalType), + (unsigned)Expr2.IVal & (ExprBits - 1)); + } + + /* Here we simply "wrap" the shift count around the width */ + Expr2.IVal &= ExprBits - 1; } } Gen->Func (Flags | CF_CONST, Expr2.IVal); diff --git a/src/cc65/datatype.c b/src/cc65/datatype.c index 9a661c037..023aefaf7 100644 --- a/src/cc65/datatype.c +++ b/src/cc65/datatype.c @@ -203,6 +203,14 @@ unsigned long GetIntegerTypeMax (const Type* Type) +unsigned BitSizeOf (const Type* T) +/* Return the size (in bit-width) of a data type */ +{ + return IsTypeBitField (T) ? T->A.B.Width : CHAR_BITS * SizeOf (T); +} + + + unsigned SizeOf (const Type* T) /* Compute size (in bytes) of object represented by type array */ { @@ -288,6 +296,17 @@ unsigned PSizeOf (const Type* T) +unsigned CheckedBitSizeOf (const Type* T) +/* Return the size (in bit-width) of a data type. If the size is zero, emit an +** error and return some valid size instead (so the rest of the compiler +** doesn't have to work with invalid sizes). +*/ +{ + return IsTypeBitField (T) ? T->A.B.Width : CHAR_BITS * CheckedSizeOf (T); +} + + + unsigned CheckedSizeOf (const Type* T) /* Return the size (in bytes) of a data type. If the size is zero, emit an ** error and return some valid size instead (so the rest of the compiler diff --git a/src/cc65/datatype.h b/src/cc65/datatype.h index 5e4e2e39b..4a20422fb 100644 --- a/src/cc65/datatype.h +++ b/src/cc65/datatype.h @@ -287,12 +287,21 @@ unsigned long GetIntegerTypeMax (const Type* Type); ** The type must have a known size. */ +unsigned BitSizeOf (const Type* T); +/* Return the size (in bit-width) of a data type */ + unsigned SizeOf (const Type* T); /* Compute size (in bytes) of object represented by type array */ unsigned PSizeOf (const Type* T); /* Compute size (in bytes) of pointee object */ +unsigned CheckedBitSizeOf (const Type* T); +/* Return the size (in bit-width) of a data type. If the size is zero, emit an +** error and return some valid size instead (so the rest of the compiler +** doesn't have to work with invalid sizes). +*/ + unsigned CheckedSizeOf (const Type* T); /* Return the size (in bytes) of a data type. If the size is zero, emit an ** error and return some valid size instead (so the rest of the compiler diff --git a/src/cc65/expr.c b/src/cc65/expr.c index f7180c020..45dc9cc37 100644 --- a/src/cc65/expr.c +++ b/src/cc65/expr.c @@ -2177,6 +2177,10 @@ static void hie_internal (const GenDesc* Ops, /* List of generators */ /* Check for const operands */ if (lconst && rconst) { + /* Evaluate the result for operands */ + unsigned long Val1 = Expr->IVal; + unsigned long Val2 = Expr2.IVal; + /* Both operands are constant, remove the generated code */ RemoveCode (&Mark1); @@ -2184,80 +2188,51 @@ static void hie_internal (const GenDesc* Ops, /* List of generators */ Expr->Type = ArithmeticConvert (Expr->Type, Expr2.Type); /* Handle the op differently for signed and unsigned types */ - if (IsSignSigned (Expr->Type)) { - - /* Evaluate the result for signed operands */ - signed long Val1 = Expr->IVal; - signed long Val2 = Expr2.IVal; - switch (Tok) { - case TOK_OR: - Expr->IVal = (Val1 | Val2); - break; - case TOK_XOR: - Expr->IVal = (Val1 ^ Val2); - break; - case TOK_AND: - Expr->IVal = (Val1 & Val2); - break; - case TOK_STAR: - Expr->IVal = (Val1 * Val2); - break; - case TOK_DIV: - if (Val2 == 0) { + switch (Tok) { + case TOK_OR: + Expr->IVal = (Val1 | Val2); + break; + case TOK_XOR: + Expr->IVal = (Val1 ^ Val2); + break; + case TOK_AND: + Expr->IVal = (Val1 & Val2); + break; + case TOK_STAR: + Expr->IVal = (Val1 * Val2); + break; + case TOK_DIV: + if (Val2 == 0) { + if (!ED_IsUneval (Expr)) { Error ("Division by zero"); - Expr->IVal = 0x7FFFFFFF; + } + Expr->IVal = 0xFFFFFFFF; + } else { + /* Handle signed and unsigned operands differently */ + if (IsSignSigned (Expr->Type)) { + Expr->IVal = ((long)Val1 / (long)Val2); } else { Expr->IVal = (Val1 / Val2); } - break; - case TOK_MOD: - if (Val2 == 0) { + } + break; + case TOK_MOD: + if (Val2 == 0) { + if (!ED_IsUneval (Expr)) { Error ("Modulo operation with zero"); - Expr->IVal = 0; + } + Expr->IVal = 0; + } else { + /* Handle signed and unsigned operands differently */ + if (IsSignSigned (Expr->Type)) { + Expr->IVal = ((long)Val1 % (long)Val2); } else { Expr->IVal = (Val1 % Val2); } - break; - default: - Internal ("hie_internal: got token 0x%X\n", Tok); - } - } else { - - /* Evaluate the result for unsigned operands */ - unsigned long Val1 = Expr->IVal; - unsigned long Val2 = Expr2.IVal; - switch (Tok) { - case TOK_OR: - Expr->IVal = (Val1 | Val2); - break; - case TOK_XOR: - Expr->IVal = (Val1 ^ Val2); - break; - case TOK_AND: - Expr->IVal = (Val1 & Val2); - break; - case TOK_STAR: - Expr->IVal = (Val1 * Val2); - break; - case TOK_DIV: - if (Val2 == 0) { - Error ("Division by zero"); - Expr->IVal = 0xFFFFFFFF; - } else { - Expr->IVal = (Val1 / Val2); - } - break; - case TOK_MOD: - if (Val2 == 0) { - Error ("Modulo operation with zero"); - Expr->IVal = 0; - } else { - Expr->IVal = (Val1 % Val2); - } - break; - default: - Internal ("hie_internal: got token 0x%X\n", Tok); - } + } + break; + default: + Internal ("hie_internal: got token 0x%X\n", Tok); } /* Limit the calculated value to the range of its type */ @@ -2314,10 +2289,12 @@ static void hie_internal (const GenDesc* Ops, /* List of generators */ /* Second value is constant - check for div */ type |= CF_CONST; rtype |= CF_CONST; - if (Tok == TOK_DIV && Expr2.IVal == 0) { - Error ("Division by zero"); - } else if (Tok == TOK_MOD && Expr2.IVal == 0) { - Error ("Modulo operation with zero"); + if (Expr2.IVal == 0 && !ED_IsUneval (Expr)) { + if (Tok == TOK_DIV) { + Error ("Division by zero"); + } else if (Tok == TOK_MOD) { + Error ("Modulo operation with zero"); + } } if ((Gen->Flags & GEN_NOPUSH) != 0) { RemoveCode (&Mark2); diff --git a/src/cc65/ppexpr.c b/src/cc65/ppexpr.c index 0942dc8f8..73ec69de8 100644 --- a/src/cc65/ppexpr.c +++ b/src/cc65/ppexpr.c @@ -305,97 +305,60 @@ static void PPhie_internal (const token_t* Ops, /* List of generators */ if (PPEvaluationEnabled && !PPEvaluationFailed) { + /* Evaluate the result for operands */ + unsigned long Val1 = Expr->IVal; + unsigned long Val2 = Rhs.IVal; + /* If either side is unsigned, the result is unsigned */ Expr->Flags |= Rhs.Flags & PPEXPR_UNSIGNED; - /* Handle the op differently for signed and unsigned integers */ - if ((Expr->Flags & PPEXPR_UNSIGNED) == 0) { - - /* Evaluate the result for signed operands */ - signed long Val1 = Expr->IVal; - signed long Val2 = Rhs.IVal; - switch (Tok) { - case TOK_OR: - Expr->IVal = (Val1 | Val2); - break; - case TOK_XOR: - Expr->IVal = (Val1 ^ Val2); - break; - case TOK_AND: - Expr->IVal = (Val1 & Val2); - break; - case TOK_PLUS: - Expr->IVal = (Val1 + Val2); - break; - case TOK_MINUS: - Expr->IVal = (Val1 - Val2); - break; - case TOK_MUL: - Expr->IVal = (Val1 * Val2); - break; - case TOK_DIV: - if (Val2 == 0) { - PPError ("Division by zero"); - Expr->IVal = 0; + switch (Tok) { + case TOK_OR: + Expr->IVal = (Val1 | Val2); + break; + case TOK_XOR: + Expr->IVal = (Val1 ^ Val2); + break; + case TOK_AND: + Expr->IVal = (Val1 & Val2); + break; + case TOK_PLUS: + Expr->IVal = (Val1 + Val2); + break; + case TOK_MINUS: + Expr->IVal = (Val1 - Val2); + break; + case TOK_MUL: + Expr->IVal = (Val1 * Val2); + break; + case TOK_DIV: + if (Val2 == 0) { + PPError ("Division by zero"); + Expr->IVal = 0; + } else { + /* Handle signed and unsigned operands differently */ + if ((Expr->Flags & PPEXPR_UNSIGNED) == 0) { + Expr->IVal = ((long)Val1 / (long)Val2); } else { Expr->IVal = (Val1 / Val2); } - break; - case TOK_MOD: - if (Val2 == 0) { - PPError ("Modulo operation with zero"); - Expr->IVal = 0; + } + break; + case TOK_MOD: + if (Val2 == 0) { + PPError ("Modulo operation with zero"); + Expr->IVal = 0; + } else { + /* Handle signed and unsigned operands differently */ + if ((Expr->Flags & PPEXPR_UNSIGNED) == 0) { + Expr->IVal = ((long)Val1 % (long)Val2); } else { Expr->IVal = (Val1 % Val2); } - break; - default: - Internal ("PPhie_internal: got token 0x%X\n", Tok); - } - - } else { - - /* Evaluate the result for unsigned operands */ - unsigned long Val1 = Expr->IVal; - unsigned long Val2 = Rhs.IVal; - switch (Tok) { - case TOK_OR: - Expr->IVal = (Val1 | Val2); - break; - case TOK_XOR: - Expr->IVal = (Val1 ^ Val2); - break; - case TOK_AND: - Expr->IVal = (Val1 & Val2); - break; - case TOK_PLUS: - Expr->IVal = (Val1 + Val2); - break; - case TOK_MINUS: - Expr->IVal = (Val1 - Val2); - break; - case TOK_MUL: - Expr->IVal = (Val1 * Val2); - break; - case TOK_DIV: - if (Val2 == 0) { - PPError ("Division by zero"); - Expr->IVal = 0; - } else { - Expr->IVal = (Val1 / Val2); - } - break; - case TOK_MOD: - if (Val2 == 0) { - PPError ("Modulo operation with zero"); - Expr->IVal = 0; - } else { - Expr->IVal = (Val1 % Val2); - } - break; - default: - Internal ("PPhie_internal: got token 0x%X\n", Tok); - } + } + break; + default: + Internal ("PPhie_internal: got token 0x%X\n", Tok); } } } diff --git a/src/cc65/shiftexpr.c b/src/cc65/shiftexpr.c index 312086f3c..48426f1f2 100644 --- a/src/cc65/shiftexpr.c +++ b/src/cc65/shiftexpr.c @@ -139,22 +139,27 @@ void ShiftExpr (struct ExprDesc* Expr) /* Remove the code that pushes the rhs onto the stack. */ RemoveCode (&Mark2); - /* If the shift count is greater or equal than the bit count of - ** the operand, the behaviour is undefined according to the - ** standard. + /* If the shift count is greater than or equal to the width of the + ** promoted left operand, the behaviour is undefined according to + ** the standard. */ - if (Expr2.IVal < 0) { - - Warning ("Shift count '%ld' is negative", Expr2.IVal); - Expr2.IVal &= ExprBits - 1; - - } else if (Expr2.IVal >= (long) ExprBits) { - - Warning ("Shift count '%ld' >= width of type", Expr2.IVal); - Expr2.IVal &= ExprBits - 1; - + if (!ED_IsUneval (Expr)) { + if (Expr2.IVal < 0) { + Warning ("Negative shift count %ld treated as %u for %s", + Expr2.IVal, + (unsigned)Expr2.IVal & (ExprBits - 1), + GetBasicTypeName (ResultType)); + } else if (Expr2.IVal >= (long) ExprBits) { + Warning ("Shift count %ld >= width of %s treated as %u", + Expr2.IVal, + GetBasicTypeName (ResultType), + (unsigned)Expr2.IVal & (ExprBits - 1)); + } } + /* Here we simply "wrap" the shift count around the width */ + Expr2.IVal &= ExprBits - 1; + /* If the shift count is zero, nothing happens. If the left hand ** side is a constant, the result is constant. */ From 73897afacea77ef59c4d7c906f021c8dd6a7ca6b Mon Sep 17 00:00:00 2001 From: acqn Date: Sat, 12 Nov 2022 12:36:22 +0800 Subject: [PATCH 4/6] Additional check for out of ranges of bit-fields in bitwise-shifts. --- src/cc65/assignment.c | 13 ++++++++++++- src/cc65/shiftexpr.c | 9 +++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/cc65/assignment.c b/src/cc65/assignment.c index ab501523d..2adb1c20e 100644 --- a/src/cc65/assignment.c +++ b/src/cc65/assignment.c @@ -345,7 +345,12 @@ static void OpAssignBitField (const GenDesc* Gen, ExprDesc* Expr, const char* Op /* Here we simply "wrap" the shift count around the width */ Expr2.IVal &= ExprBits - 1; - } + + /* Additional check for bit-fields */ + if (Expr2.IVal >= (long)Expr->Type->A.B.Width) { + Warning ("Shift count %ld >= width of bit-field", Expr2.IVal); + } + } } /* Adjust the types of the operands if needed */ @@ -549,6 +554,12 @@ static void OpAssignArithmetic (const GenDesc* Gen, ExprDesc* Expr, const char* /* Here we simply "wrap" the shift count around the width */ Expr2.IVal &= ExprBits - 1; + + /* Additional check for bit width */ + if (Expr2.IVal >= (long)BitSizeOf (Expr->Type)) { + Warning ("Shift count %ld >= width of %s", + Expr2.IVal, GetBasicTypeName (Expr->Type)); + } } } Gen->Func (Flags | CF_CONST, Expr2.IVal); diff --git a/src/cc65/shiftexpr.c b/src/cc65/shiftexpr.c index 48426f1f2..b8fb70434 100644 --- a/src/cc65/shiftexpr.c +++ b/src/cc65/shiftexpr.c @@ -160,6 +160,15 @@ void ShiftExpr (struct ExprDesc* Expr) /* Here we simply "wrap" the shift count around the width */ Expr2.IVal &= ExprBits - 1; + /* Additional check for bit-fields */ + if (IsTypeBitField (Expr->Type) && + Tok == TOK_SHR && + Expr2.IVal >= (long) Expr->Type->A.B.Width) { + if (!ED_IsUneval (Expr)) { + Warning ("Right-shift count %ld >= width of bit-field", Expr2.IVal); + } + } + /* If the shift count is zero, nothing happens. If the left hand ** side is a constant, the result is constant. */ From cc177208b465cd24dfd1f1a198a26913b571086f Mon Sep 17 00:00:00 2001 From: acqn Date: Sat, 12 Nov 2022 21:05:01 +0800 Subject: [PATCH 5/6] Added tests for diagnostics in unevaluated context. --- test/misc/Makefile | 14 +++-- test/misc/bug1768.c | 149 +++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 149 insertions(+), 14 deletions(-) diff --git a/test/misc/Makefile b/test/misc/Makefile index f18f40da6..d0b8979b0 100644 --- a/test/misc/Makefile +++ b/test/misc/Makefile @@ -64,12 +64,6 @@ $(WORKDIR)/int-static-1888.$1.$2.prg: int-static-1888.c | $(WORKDIR) $(if $(QUIET),echo misc/int-static-1888.$1.$2.prg) $(NOT) $(CC65) -t sim$2 -$1 -o $$@ $$< $(NULLERR) -# should compile, but gives an error -$(WORKDIR)/bug1768.$1.$2.prg: bug1768.c | $(WORKDIR) - @echo "FIXME: " $$@ "currently does not compile." - $(if $(QUIET),echo misc/bug1768.$1.$2.prg) - $(NOT) $(CC65) -t sim$2 -$1 -o $$@ $$< $(NULLERR) - # should compile, but gives an error $(WORKDIR)/bug760.$1.$2.prg: bug760.c | $(WORKDIR) @echo "FIXME: " $$@ "currently does not compile." @@ -138,6 +132,14 @@ $(WORKDIR)/bug1265.$1.$2.prg: bug1265.c | $(WORKDIR) $(LD65) -t sim$2 -o $$@ $$(@:.prg=.o) sim$2.lib $(NULLERR) $(SIM65) $(SIM65FLAGS) $$@ $(NULLOUT) $(NULLERR) +# this one requires -Werror +$(WORKDIR)/bug1768.$1.$2.prg: bug1768.c | $(WORKDIR) + $(if $(QUIET),echo misc/bug1768.$1.$2.prg) + $(CC65) -Werror -t sim$2 -$1 -o $$(@:.prg=.s) $$< $(NULLERR) + $(CA65) -t sim$2 -o $$(@:.prg=.o) $$(@:.prg=.s) $(NULLERR) + $(LD65) -t sim$2 -o $$@ $$(@:.prg=.o) sim$2.lib $(NULLERR) + $(SIM65) $(SIM65FLAGS) $$@ $(NULLOUT) $(NULLERR) + # should compile, but then hangs in an endless loop $(WORKDIR)/endless.$1.$2.prg: endless.c | $(WORKDIR) $(if $(QUIET),echo misc/endless.$1.$2.prg) diff --git a/test/misc/bug1768.c b/test/misc/bug1768.c index 916aa64bc..35cee1049 100644 --- a/test/misc/bug1768.c +++ b/test/misc/bug1768.c @@ -1,14 +1,147 @@ +/* + Copyright 2021-2022, The cc65 Authors -#include + This software is provided "as-is", without any express or implied + warranty. In no event will the authors be held liable for any damages + arising from the use of this software. -int a = 1 || (8 / 0); -int b = 0 && (8 % 0); -int c = 1 ? 42 : (0 % 0); -int d = 1 || a / 0; -int e = 0 && b % 0; -int f = 1 ? 42 : (a %= 0, b /= 0); + Permission is granted to anyone to use this software for any purpose, + including commercial applications; and, to alter it and redistribute it + freely, subject to the following restrictions: + 1. The origin of this software must not be misrepresented; you must not + claim that you wrote the original software. If you use this software + in a product, an acknowledgment in the product documentation would be + appreciated, but is not required. + 2. Altered source versions must be plainly marked as such, and must not be + misrepresented as being the original software. + 3. This notice may not be removed or altered from any source distribution. +*/ + +/* + Test of operations in unevaluated context resulted from 'sizeof' and + short-circuited code-paths in AND, OR and conditional operations. + + See also: + https://github.com/cc65/cc65/issues/1768#issuecomment-1175221466 +*/ + +#include + +static int failures; + +#define TEST(EXPR)\ + {\ + int acc = 0;\ + acc += sizeof((EXPR), 0);\ + acc += (0 && (EXPR));\ + acc += (1 || (EXPR));\ + acc += (0 ? (EXPR) : 0);\ + acc += (1 ? 0 : (EXPR));\ + if (acc == 0) {\ + printf("acc = %d\n", acc);\ + ++failures;\ + }\ + } + +/* Division by zero/modulo with zero */ +void test_1(void) +{ + int i; + int j; + TEST((i / 0) | (j % 0)) +} + +/* Division by zero/modulo with zero */ +void test_2(void) +{ + int i; + int j; + TEST((i /= 0) | (j %= 0)) +} + +/* Shift by too wide counts */ +void test_3(void) +{ + int i; + int j; + TEST((i << 32) | (j >> 32)) +} + +/* Shift by too wide counts */ +void test_4(void) +{ + int i; + int j; + TEST((i <<= 32) | (j >>= 32)) +} + +/* Shift by negative counts */ +void test_5(void) +{ + int i; + int j; + TEST((i << -1) | (j >> -1)) +} + +/* Shift by negative counts */ +void test_6(void) +{ + int i; + int j; + TEST((i <<= -1) | (j >>= -1)) +} + +/* Shift bit-fields */ +void test_7(void) +{ + struct S { + long i : 24; /* Will be promoted to 32-bit integer in calculation */ + long j : 8; /* Will be promoted to 16-bit integer in calculation */ + } s; + long k; + + s.i = 1; + printf("%u\n", sizeof(s.i << 24)); + s.i = 2; + k = s.i << 16; + if (k != 0x00020000L) { + printf("k = %ld, expected: %ld\n", k, 0x00020000L); + } + TEST(s.j >> 16) +} + +/* Shift bit-fields */ +void test_8(void) +{ + struct S { + long i : 24; /* Will be promoted to 32-bit integer in calculation */ + long j : 8; /* Will be promoted to 16-bit integer in calculation */ + } s; + long k; + + s.i = 3; + printf("%u\n", sizeof(s.i << 24)); + s.i = 4; + k = s.i <<= 16; + if (k != 0x00040000L) { + printf("k = %ld, expected: %ld\n", k, 0x00040000L); + } + TEST(s.j >>= 8) +} + +/* Do all tests */ int main(void) { - return EXIT_SUCCESS; + test_1(); + test_2(); + test_3(); + test_4(); + test_5(); + test_6(); + test_7(); + test_8(); + + printf("Failures: %d\n", failures); + return failures; } From 9d693d2c80029461e7650c5d17bcd22f19db268a Mon Sep 17 00:00:00 2001 From: acqn Date: Sat, 12 Nov 2022 22:13:56 +0800 Subject: [PATCH 6/6] Warnings instead of errors for division-by-zero/modulo-with-zero in evaluation. --- src/cc65/assignment.c | 8 ++++---- src/cc65/expr.c | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/cc65/assignment.c b/src/cc65/assignment.c index 2adb1c20e..54ab28d4e 100644 --- a/src/cc65/assignment.c +++ b/src/cc65/assignment.c @@ -319,9 +319,9 @@ static void OpAssignBitField (const GenDesc* Gen, ExprDesc* Expr, const char* Op if (Expr2.IVal == 0) { /* Check for div by zero/mod by zero */ if (Gen->Func == g_div) { - Error ("Division by zero"); + Warning ("Division by zero"); } else if (Gen->Func == g_mod) { - Error ("Modulo operation with zero"); + Warning ("Modulo operation with zero"); } } else if (Gen->Func == g_asl || Gen->Func == g_asr) { const Type* CalType = IntPromotion (Expr->Type); @@ -528,9 +528,9 @@ static void OpAssignArithmetic (const GenDesc* Gen, ExprDesc* Expr, const char* if (Expr2.IVal == 0 && !ED_IsUneval (Expr)) { /* Check for div by zero/mod by zero */ if (Gen->Func == g_div) { - Error ("Division by zero"); + Warning ("Division by zero"); } else if (Gen->Func == g_mod) { - Error ("Modulo operation with zero"); + Warning ("Modulo operation with zero"); } } else if (Gen->Func == g_asl || Gen->Func == g_asr) { const Type* CalType = IntPromotion (Expr->Type); diff --git a/src/cc65/expr.c b/src/cc65/expr.c index 45dc9cc37..78a4c516a 100644 --- a/src/cc65/expr.c +++ b/src/cc65/expr.c @@ -2204,7 +2204,7 @@ static void hie_internal (const GenDesc* Ops, /* List of generators */ case TOK_DIV: if (Val2 == 0) { if (!ED_IsUneval (Expr)) { - Error ("Division by zero"); + Warning ("Division by zero"); } Expr->IVal = 0xFFFFFFFF; } else { @@ -2219,7 +2219,7 @@ static void hie_internal (const GenDesc* Ops, /* List of generators */ case TOK_MOD: if (Val2 == 0) { if (!ED_IsUneval (Expr)) { - Error ("Modulo operation with zero"); + Warning ("Modulo operation with zero"); } Expr->IVal = 0; } else { @@ -2291,9 +2291,9 @@ static void hie_internal (const GenDesc* Ops, /* List of generators */ rtype |= CF_CONST; if (Expr2.IVal == 0 && !ED_IsUneval (Expr)) { if (Tok == TOK_DIV) { - Error ("Division by zero"); + Warning ("Division by zero"); } else if (Tok == TOK_MOD) { - Error ("Modulo operation with zero"); + Warning ("Modulo operation with zero"); } } if ((Gen->Flags & GEN_NOPUSH) != 0) {