From bf5b37a3b2b0a286e3cc14d1435f0ddbaa6cee4d Mon Sep 17 00:00:00 2001 From: bbbradsmith Date: Tue, 2 May 2023 21:27:02 -0400 Subject: [PATCH 1/9] Error check for internal overflow of numerical constant See bug #2026 --- src/cc65/scanner.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/cc65/scanner.c b/src/cc65/scanner.c index 36fd1301b..055c02450 100644 --- a/src/cc65/scanner.c +++ b/src/cc65/scanner.c @@ -522,6 +522,7 @@ static void NumericConst (void) char C; unsigned DigitVal; unsigned long IVal; /* Value */ + int Overflow; /* Get the pp-number first, then parse on it */ CopyPPNumber (&Src); @@ -575,6 +576,7 @@ static void NumericConst (void) /* Since we now know the correct base, convert the input into a number */ SB_SetIndex (&Src, Index); IVal = 0; + Overflow = 0; while ((C = SB_Peek (&Src)) != '\0' && (Base <= 10 ? IsDigit (C) : IsXDigit (C))) { DigitVal = HexVal (C); if (DigitVal >= Base) { @@ -582,9 +584,17 @@ static void NumericConst (void) SB_Clear (&Src); break; } - IVal = (IVal * Base) + DigitVal; + if ((((unsigned long)(IVal * Base)) / Base) != IVal) + Overflow = 1; + IVal = IVal * Base; + if (((unsigned long)(IVal + DigitVal)) < IVal) + Overflow = 1; + IVal += DigitVal; SB_Skip (&Src); } + if (Overflow) + Error ("Numerical constant \"%s\" too large for internal 32-bit representation", + SB_GetConstBuf (&Src)); /* Distinguish between integer and floating point constants */ if (!IsFloat) { From 409235aee65ce5fd807a50b32a2a4ba664aaab70 Mon Sep 17 00:00:00 2001 From: bbbradsmith Date: Tue, 2 May 2023 22:27:28 -0400 Subject: [PATCH 2/9] Optional warning for implicit constant conversion overflow --- doc/cc65.sgml | 2 ++ src/cc65/error.c | 2 ++ src/cc65/error.h | 1 + src/cc65/typeconv.c | 5 +++++ 4 files changed, 10 insertions(+) diff --git a/doc/cc65.sgml b/doc/cc65.sgml index 683249bda..5a094571a 100644 --- a/doc/cc65.sgml +++ b/doc/cc65.sgml @@ -754,6 +754,8 @@ Here is a description of all the command line options: Warn about unused function parameters. Warn about unused variables. + + Warn if numerical constant conversion implies overflow. (Disabled by default.) The full list of available warning names can be retrieved by using the diff --git a/src/cc65/error.c b/src/cc65/error.c index 3f36d9e97..6ac3e594b 100644 --- a/src/cc65/error.c +++ b/src/cc65/error.c @@ -79,6 +79,7 @@ IntStack WarnUnusedLabel = INTSTACK(1); /* - unused labels */ IntStack WarnUnusedParam = INTSTACK(1); /* - unused parameters */ IntStack WarnUnusedVar = INTSTACK(1); /* - unused variables */ IntStack WarnUnusedFunc = INTSTACK(1); /* - unused functions */ +IntStack WarnConstOverflow = INTSTACK(0); /* - overflow conversion of numerical constants */ /* Map the name of a warning to the intstack that holds its state */ typedef struct WarnMapEntry WarnMapEntry; @@ -102,6 +103,7 @@ static WarnMapEntry WarnMap[] = { { &WarnUnusedLabel, "unused-label" }, { &WarnUnusedParam, "unused-param" }, { &WarnUnusedVar, "unused-var" }, + { &WarnConstOverflow, "const-overflow" }, }; Collection DiagnosticStrBufs; diff --git a/src/cc65/error.h b/src/cc65/error.h index 7fcb03467..83be8c782 100644 --- a/src/cc65/error.h +++ b/src/cc65/error.h @@ -76,6 +76,7 @@ extern IntStack WarnUnusedLabel; /* - unused labels */ extern IntStack WarnUnusedParam; /* - unused parameters */ extern IntStack WarnUnusedVar; /* - unused variables */ extern IntStack WarnUnusedFunc; /* - unused functions */ +extern IntStack WarnConstOverflow; /* - overflow conversion of numerical constants */ /* Forward */ struct StrBuf; diff --git a/src/cc65/typeconv.c b/src/cc65/typeconv.c index f77ec3951..49dfcc597 100644 --- a/src/cc65/typeconv.c +++ b/src/cc65/typeconv.c @@ -128,6 +128,7 @@ static void DoConversion (ExprDesc* Expr, const Type* NewType) ** internally already represented by a long. */ if (NewBits <= OldBits) { + unsigned long OldVal = Expr->IVal; /* Cut the value to the new size */ Expr->IVal &= (0xFFFFFFFFUL >> (32 - NewBits)); @@ -139,6 +140,10 @@ static void DoConversion (ExprDesc* Expr, const Type* NewType) Expr->IVal |= shl_l (~0UL, NewBits); } } + + if ((OldVal != Expr->IVal) && IS_Get (&WarnConstOverflow)) { + Warning ("Implicit conversion of constant overflows %d-bit destination", NewBits); + } } /* Do the integer constant <-> absolute address conversion if necessary */ From 9a502c69dc9d3c4c29791b75d9f03796487e3cc7 Mon Sep 17 00:00:00 2001 From: bbbradsmith Date: Wed, 3 May 2023 16:46:59 -0400 Subject: [PATCH 3/9] fix tab, braces for 1-line if, Expr->Ival is signed --- src/cc65/error.c | 2 +- src/cc65/scanner.c | 9 ++++++--- src/cc65/typeconv.c | 2 +- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/cc65/error.c b/src/cc65/error.c index 6ac3e594b..39b067825 100644 --- a/src/cc65/error.c +++ b/src/cc65/error.c @@ -103,7 +103,7 @@ static WarnMapEntry WarnMap[] = { { &WarnUnusedLabel, "unused-label" }, { &WarnUnusedParam, "unused-param" }, { &WarnUnusedVar, "unused-var" }, - { &WarnConstOverflow, "const-overflow" }, + { &WarnConstOverflow, "const-overflow" }, }; Collection DiagnosticStrBufs; diff --git a/src/cc65/scanner.c b/src/cc65/scanner.c index 055c02450..ec49d0e3c 100644 --- a/src/cc65/scanner.c +++ b/src/cc65/scanner.c @@ -584,17 +584,20 @@ static void NumericConst (void) SB_Clear (&Src); break; } - if ((((unsigned long)(IVal * Base)) / Base) != IVal) + if ((((unsigned long)(IVal * Base)) / Base) != IVal) { Overflow = 1; + } IVal = IVal * Base; - if (((unsigned long)(IVal + DigitVal)) < IVal) + if (((unsigned long)(IVal + DigitVal)) < IVal) { Overflow = 1; + } IVal += DigitVal; SB_Skip (&Src); } - if (Overflow) + if (Overflow) { Error ("Numerical constant \"%s\" too large for internal 32-bit representation", SB_GetConstBuf (&Src)); + } /* Distinguish between integer and floating point constants */ if (!IsFloat) { diff --git a/src/cc65/typeconv.c b/src/cc65/typeconv.c index 49dfcc597..e1d95ff63 100644 --- a/src/cc65/typeconv.c +++ b/src/cc65/typeconv.c @@ -128,7 +128,7 @@ static void DoConversion (ExprDesc* Expr, const Type* NewType) ** internally already represented by a long. */ if (NewBits <= OldBits) { - unsigned long OldVal = Expr->IVal; + long OldVal = Expr->IVal; /* Cut the value to the new size */ Expr->IVal &= (0xFFFFFFFFUL >> (32 - NewBits)); From 49bd5681136fc4afff5f46f796eeb777c0e691d1 Mon Sep 17 00:00:00 2001 From: bbbradsmith Date: Wed, 3 May 2023 17:55:02 -0400 Subject: [PATCH 4/9] error test for integer constant too large for internal representation --- test/err/huge-integer-constant.c | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 test/err/huge-integer-constant.c diff --git a/test/err/huge-integer-constant.c b/test/err/huge-integer-constant.c new file mode 100644 index 000000000..1f423347c --- /dev/null +++ b/test/err/huge-integer-constant.c @@ -0,0 +1,7 @@ +/* too big for internal integer representation */ +unsigned long huge = 4294967296; + +int main(void) +{ + return 0; +} From e3cb8dfb9be6e4f9244fdecba6610f6a72116763 Mon Sep 17 00:00:00 2001 From: bbbradsmith Date: Wed, 3 May 2023 19:27:37 -0400 Subject: [PATCH 5/9] Numerical constant scanner requires explicitly 32-bit sized type for cross-platform consistency --- src/cc65/scanner.c | 16 +++++++++++----- test/val/common.h | 1 + test/val/cq241.c | 19 +++++++++++++++++-- 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/src/cc65/scanner.c b/src/cc65/scanner.c index ec49d0e3c..f747fb458 100644 --- a/src/cc65/scanner.c +++ b/src/cc65/scanner.c @@ -39,6 +39,7 @@ #include #include #include +#include /* common */ #include "chartype.h" @@ -151,6 +152,11 @@ static const struct Keyword { #define IT_ULONG 0x08 +/* Internal type for numeric constant scanning. +** Size must be explicit for cross-platform uniformity. +*/ +typedef uint32_t scan_t; + /*****************************************************************************/ /* code */ @@ -521,7 +527,7 @@ static void NumericConst (void) int IsFloat; char C; unsigned DigitVal; - unsigned long IVal; /* Value */ + scan_t IVal; /* Scanned value. */ int Overflow; /* Get the pp-number first, then parse on it */ @@ -584,19 +590,19 @@ static void NumericConst (void) SB_Clear (&Src); break; } - if ((((unsigned long)(IVal * Base)) / Base) != IVal) { + if (((scan_t)(IVal * Base) / Base) != IVal) { Overflow = 1; } IVal = IVal * Base; - if (((unsigned long)(IVal + DigitVal)) < IVal) { + if (((scan_t)(IVal + DigitVal)) < IVal) { Overflow = 1; } IVal += DigitVal; SB_Skip (&Src); } if (Overflow) { - Error ("Numerical constant \"%s\" too large for internal 32-bit representation", - SB_GetConstBuf (&Src)); + Error ("Numerical constant \"%s\" too large for internal %d-bit representation", + SB_GetConstBuf (&Src), (int)(sizeof(IVal)*8)); } /* Distinguish between integer and floating point constants */ diff --git a/test/val/common.h b/test/val/common.h index dada61a14..61da6c325 100644 --- a/test/val/common.h +++ b/test/val/common.h @@ -20,3 +20,4 @@ #define SIZEOF_LONG_32BIT #define UNSIGNED_CHARS #define UNSIGNED_BITFIELDS +#define INTEGER_CONSTANT_MAX_32BIT diff --git a/test/val/cq241.c b/test/val/cq241.c index 611b5a376..a6d6c5324 100644 --- a/test/val/cq241.c +++ b/test/val/cq241.c @@ -4,6 +4,14 @@ !!LICENCE!! own, freely distributeable for non-profit. read CPYRIGHT.LCC */ +/* INTEGER_CONSTANT_MAX_32BIT +** This suppresses constants longer than 32-bit, which are now an error: +** https://github.com/cc65/cc65/pull/2084 +** Because cc65's internal representation is implicitly/explicitly +** 32-bit in many places, values larger than this aren't representable, +** but also can't be checked for overflow once accepted. +*/ + #include "common.h" struct defs { @@ -62,7 +70,12 @@ long pow2(long n) { return s; } - long d[39], o[39], x[39]; +#ifndef INTEGER_CONSTANT_MAX_32BIT +#define CTCOUNT 39 +#else +#define CTCOUNT 36 +#endif + long d[CTCOUNT], o[CTCOUNT], x[CTCOUNT]; #ifndef NO_OLD_FUNC_DECL s241(pd0) @@ -212,13 +225,15 @@ int s241(struct defs *pd0) { d[33] = 1073741823; o[33] = 07777777777; x[33] = 0x3fffffff; d[34] = 1073741824; o[34] = 010000000000; x[34] = 0x40000000; d[35] = 4294967295; o[35] = 037777777777; x[35] = 0xffffffff; +#if CTCOUNT > 36 d[36] = 4294967296; o[36] = 040000000000; x[36] = 0x100000000; d[37] = 68719476735; o[37] = 0777777777777; x[37] = 0xfffffffff; d[38] = 68719476736; o[38] = 01000000000000; x[38] = 0x1000000000; +#endif /* WHEW! */ - for (j=0; j<39; j++){ + for (j=0; j Date: Wed, 3 May 2023 19:42:05 -0400 Subject: [PATCH 6/9] Suppress overflow warning when conversion is an explicit cast --- src/cc65/typeconv.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/cc65/typeconv.c b/src/cc65/typeconv.c index e1d95ff63..6bdb45b5f 100644 --- a/src/cc65/typeconv.c +++ b/src/cc65/typeconv.c @@ -55,7 +55,7 @@ -static void DoConversion (ExprDesc* Expr, const Type* NewType) +static void DoConversion (ExprDesc* Expr, const Type* NewType, int Explicit) /* Emit code to convert the given expression to a new type. */ { const Type* OldType; @@ -141,7 +141,7 @@ static void DoConversion (ExprDesc* Expr, const Type* NewType) } } - if ((OldVal != Expr->IVal) && IS_Get (&WarnConstOverflow)) { + if ((OldVal != Expr->IVal) && IS_Get (&WarnConstOverflow) && !Explicit) { Warning ("Implicit conversion of constant overflows %d-bit destination", NewBits); } } @@ -288,7 +288,7 @@ void TypeConversion (ExprDesc* Expr, const Type* NewType) /* Both types must be complete */ if (!IsIncompleteESUType (NewType) && !IsIncompleteESUType (Expr->Type)) { /* Do the actual conversion */ - DoConversion (Expr, NewType); + DoConversion (Expr, NewType, 0); } else { /* We should have already generated error elsewhere so that we ** could just silently fail here to avoid excess errors, but to @@ -335,7 +335,7 @@ void TypeCast (ExprDesc* Expr) ReplaceType (Expr, NewType); } else if (IsCastType (Expr->Type)) { /* Convert the value. The result has always the new type */ - DoConversion (Expr, NewType); + DoConversion (Expr, NewType, 1); } else { TypeCompatibilityDiagnostic (NewType, Expr->Type, 1, "Cast to incompatible type '%s' from '%s'"); From b5f255f9123dc898e494321406edd434b89a6d49 Mon Sep 17 00:00:00 2001 From: bbbradsmith Date: Wed, 3 May 2023 19:54:40 -0400 Subject: [PATCH 7/9] Test case for const-overflow warnings --- test/err/integer-const-overflow.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 test/err/integer-const-overflow.c diff --git a/test/err/integer-const-overflow.c b/test/err/integer-const-overflow.c new file mode 100644 index 000000000..37cc0f01e --- /dev/null +++ b/test/err/integer-const-overflow.c @@ -0,0 +1,20 @@ +/* Integer constant overflow warnings. */ + +/* Warnings as errors. */ +#pragma warn(error,on) + +/* Warn on const overflow */ +#pragma warn(const-overflow,on) + +unsigned char a = 256; +signed char b = 128; +unsigned char c = -129; +unsigned short int d = 0x00010000; +unsigned short int e = 0x80000000; +signed short int f = 32768L; +signed short int g = -32769L; + +int main(void) +{ + return 0; +} From ca8201a314bc0b6539a49518c334cc297e651764 Mon Sep 17 00:00:00 2001 From: bbbradsmith Date: Thu, 4 May 2023 05:44:20 -0400 Subject: [PATCH 8/9] Overflow test optimization suggested by kugelfuhr User CHAR_BIT instead of 8 --- src/cc65/scanner.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/cc65/scanner.c b/src/cc65/scanner.c index f747fb458..54ce02158 100644 --- a/src/cc65/scanner.c +++ b/src/cc65/scanner.c @@ -590,19 +590,15 @@ static void NumericConst (void) SB_Clear (&Src); break; } - if (((scan_t)(IVal * Base) / Base) != IVal) { + if (((scan_t)(IVal * Base + DigitVal) / Base) != IVal) { Overflow = 1; } - IVal = IVal * Base; - if (((scan_t)(IVal + DigitVal)) < IVal) { - Overflow = 1; - } - IVal += DigitVal; + IVal = IVal * Base + DigitVal; SB_Skip (&Src); } if (Overflow) { Error ("Numerical constant \"%s\" too large for internal %d-bit representation", - SB_GetConstBuf (&Src), (int)(sizeof(IVal)*8)); + SB_GetConstBuf (&Src), (int)(sizeof(IVal)*CHAR_BIT)); } /* Distinguish between integer and floating point constants */ From 69f4cd184779925ca399823acc56e50dcff1dc29 Mon Sep 17 00:00:00 2001 From: bbbradsmith Date: Thu, 4 May 2023 05:48:48 -0400 Subject: [PATCH 9/9] limits.h was apparently already included somewhere on windows but not linux --- src/cc65/scanner.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/cc65/scanner.c b/src/cc65/scanner.c index 54ce02158..ede77cb2c 100644 --- a/src/cc65/scanner.c +++ b/src/cc65/scanner.c @@ -40,6 +40,7 @@ #include #include #include +#include /* common */ #include "chartype.h" @@ -590,6 +591,7 @@ static void NumericConst (void) SB_Clear (&Src); break; } + /* Test result of adding digit for overflow. */ if (((scan_t)(IVal * Base + DigitVal) / Base) != IVal) { Overflow = 1; }