From 93b94d314a56a0c6d1a41a278e0a3e3e95f32edf Mon Sep 17 00:00:00 2001 From: Kugel Fuhr <98353208+kugelfuhr@users.noreply.github.com> Date: Wed, 16 Jul 2025 20:15:32 +0200 Subject: [PATCH 1/5] Implement a simple flow analysis. Tracks control flow of all statements with the exception of "switch". Outputs warnings for unreachable code. Tracks also "return" but doesn't currently make use of this information. --- libsrc/common/getopt.c | 2 +- src/cc65/error.c | 60 +++++-- src/cc65/error.h | 8 + src/cc65/function.c | 6 +- src/cc65/goto.c | 8 +- src/cc65/goto.h | 4 +- src/cc65/lineinfo.h | 2 +- src/cc65/stmt.c | 358 +++++++++++++++++++++++++++++------------ src/cc65/stmt.h | 124 +++++++++++++- src/cc65/swstmt.c | 111 ++++++++++++- src/cc65/swstmt.h | 20 ++- test/misc/goto.ref | 11 ++ 12 files changed, 575 insertions(+), 139 deletions(-) diff --git a/libsrc/common/getopt.c b/libsrc/common/getopt.c index 34b13b1e6..3c54c9d2f 100644 --- a/libsrc/common/getopt.c +++ b/libsrc/common/getopt.c @@ -29,7 +29,7 @@ int optopt; /* character checked for validity */ char *optarg; /* argument associated with option */ #define tell(s) fputs(*argv,stderr);fputs(s,stderr); \ - fputc(optopt,stderr);fputc('\n',stderr);return(BADCH); + fputc(optopt,stderr);fputc('\n',stderr);return(BADCH) int __fastcall__ getopt (int argc, char* const* argv, const char* optstring) /* Get option letter from argument vector */ diff --git a/src/cc65/error.c b/src/cc65/error.c index ae2d6f27d..97e651890 100644 --- a/src/cc65/error.c +++ b/src/cc65/error.c @@ -95,6 +95,7 @@ struct WarnMapEntry { static WarnMapEntry WarnMap[] = { /* Keep names sorted, even if it isn't used for now */ { &WarnConstComparison, "const-comparison" }, + { &WarnConstOverflow, "const-overflow" }, { &WarningsAreErrors, "error" }, { &WarnNoEffect, "no-effect" }, { &WarnPointerSign, "pointer-sign" }, @@ -108,7 +109,6 @@ static WarnMapEntry WarnMap[] = { { &WarnUnusedLabel, "unused-label" }, { &WarnUnusedParam, "unused-param" }, { &WarnUnusedVar, "unused-var" }, - { &WarnConstOverflow, "const-overflow" }, }; Collection DiagnosticStrBufs; @@ -142,7 +142,7 @@ void PrintFileInclusionInfo (const LineInfo* LI) -static LineInfo* GetDiagnosticLI (void) +LineInfo* GetDiagnosticLI (void) /* Get the line info where the diagnostic info refers to */ { if (CurTok.LI) { @@ -184,7 +184,7 @@ static unsigned GetDiagnosticLineNum (void) -void Fatal_ (const char *file, int line, const char* Format, ...) +void Fatal_ (const char* file, int line, const char* Format, ...) /* Print a message about a fatal error and die */ { va_list ap; @@ -208,7 +208,7 @@ void Fatal_ (const char *file, int line, const char* Format, ...) -void Internal_ (const char *file, int line, const char* Format, ...) +void Internal_ (const char* file, int line, const char* Format, ...) /* Print a message about an internal compiler error and die */ { va_list ap; @@ -279,7 +279,7 @@ static void IntError (errcat_t EC, LineInfo* LI, const char* Msg, va_list ap) -void LIError_ (const char *file, int line, errcat_t EC, LineInfo* LI, const char* Format, ...) +void LIError_ (const char* file, int line, errcat_t EC, LineInfo* LI, const char* Format, ...) /* Print an error message with the line info given explicitly */ { va_list ap; @@ -295,7 +295,7 @@ void LIError_ (const char *file, int line, errcat_t EC, LineInfo* LI, const char -void Error_ (const char *file, int line, const char* Format, ...) +void Error_ (const char* file, int line, const char* Format, ...) /* Print an error message */ { va_list ap; @@ -311,7 +311,7 @@ void Error_ (const char *file, int line, const char* Format, ...) -void PPError_ (const char *file, int line, const char* Format, ...) +void PPError_ (const char* file, int line, const char* Format, ...) /* Print an error message. For use within the preprocessor */ { va_list ap; @@ -370,7 +370,7 @@ static void IntWarning (errcat_t EC, LineInfo* LI, const char* Msg, va_list ap) -void LIWarning_ (const char *file, int line, errcat_t EC, LineInfo* LI, const char* Format, ...) +void LIWarning_ (const char* file, int line, errcat_t EC, LineInfo* LI, const char* Format, ...) /* Print a warning message with the line info given explicitly */ { va_list ap; @@ -386,7 +386,7 @@ void LIWarning_ (const char *file, int line, errcat_t EC, LineInfo* LI, const ch -void Warning_ (const char *file, int line, const char* Format, ...) +void Warning_ (const char* file, int line, const char* Format, ...) /* Print a warning message */ { va_list ap; @@ -402,7 +402,7 @@ void Warning_ (const char *file, int line, const char* Format, ...) -void PPWarning_ (const char *file, int line, const char* Format, ...) +void PPWarning_ (const char* file, int line, const char* Format, ...) /* Print a warning message. For use within the preprocessor */ { va_list ap; @@ -424,7 +424,45 @@ void UnreachableCodeWarning (void) */ { if (IS_Get (&WarnUnreachableCode)) { - Warning ("Unreachable code"); + + LineInfo* LI; + + /* Add special handling for compound statements if the current token + ** is from the source. Doing this here is a bit hacky but unfortunately + ** there's no better place. + */ + if (CurTok.LI && NextTok.LI) { + if (CurTok.Tok == TOK_LCURLY) { + /* Do not point to the compoung statement but to the first + ** statement within it. If the compound statement is empty + ** do not even output a warning. This fails of course for + ** nested compounds but will do the right thing in most cases. + */ + if (NextTok.Tok == TOK_RCURLY) { + return; + } + LI = NextTok.LI; + } else { + LI = CurTok.LI; + } + } else { + LI = GetCurLineInfo (); + } + + /* Now output the warning */ + LIWarning (EC_PARSER, LI, "Unreachable code"); + } +} + + + +void LIUnreachableCodeWarning (LineInfo* LI) +/* Print a warning about unreachable code at the given location if these +** warnings are enabled. +*/ +{ + if (IS_Get (&WarnUnreachableCode)) { + LIWarning (EC_PARSER, LI, "Unreachable code"); } } diff --git a/src/cc65/error.h b/src/cc65/error.h index 0a18d51a4..30c0d404e 100644 --- a/src/cc65/error.h +++ b/src/cc65/error.h @@ -103,6 +103,9 @@ struct StrBuf; void PrintFileInclusionInfo (const LineInfo* LI); /* Print hierarchy of file inclusion */ +LineInfo* GetDiagnosticLI (void); +/* Get the line info where the diagnostic info refers to */ + void Fatal_ (const char *file, int line, const char* Format, ...) attribute ((noreturn, format (printf, 3, 4))); #define Fatal(...) Fatal_(__FILE__, __LINE__, __VA_ARGS__) /* Print a message about a fatal error and die */ @@ -140,6 +143,11 @@ void UnreachableCodeWarning (void); ** warnings are enabled. */ +void LIUnreachableCodeWarning (LineInfo* LI); +/* Print a warning about unreachable code at the given location if these +** warnings are enabled. +*/ + IntStack* FindWarning (const char* Name); /* Search for a warning in the WarnMap table and return a pointer to the ** intstack that holds its state. Return NULL if there is no such warning. diff --git a/src/cc65/function.c b/src/cc65/function.c index 3123079d3..0e040d61d 100644 --- a/src/cc65/function.c +++ b/src/cc65/function.c @@ -626,10 +626,8 @@ void NewFunc (SymEntry* Func, FuncDesc* D) */ CurrentFunc->TopLevelSP = StackPtr; - /* Now process statements in this block */ - while (CurTok.Tok != TOK_RCURLY && CurTok.Tok != TOK_CEOF) { - AnyStatement (0); - } + /* Now process statements in this block checking for unreachable code */ + StatementBlock (0); /* Check if this function is missing a return value */ if (!F_HasVoidReturn (CurrentFunc) && !F_HasReturn (CurrentFunc)) { diff --git a/src/cc65/goto.c b/src/cc65/goto.c index e96ad6c4c..51fdc725d 100644 --- a/src/cc65/goto.c +++ b/src/cc65/goto.c @@ -41,10 +41,12 @@ #include "error.h" #include "exprdesc.h" #include "expr.h" +#include "function.h" #include "loadexpr.h" #include "scanner.h" #include "seqpoint.h" #include "standard.h" +#include "stmt.h" #include "symtab.h" #include "goto.h" @@ -56,8 +58,8 @@ -void GotoStatement (void) -/* Process a goto statement. */ +int GotoStatement (void) +/* Process a goto statement and return one of the SF_xxx flags from stmt.h. */ { /* Eat the "goto" */ NextToken (); @@ -165,6 +167,8 @@ void GotoStatement (void) } else { Error ("Label name expected"); } + + return SF_GOTO | SF_ANY_GOTO; } diff --git a/src/cc65/goto.h b/src/cc65/goto.h index 3ca8223e2..59a3058e9 100644 --- a/src/cc65/goto.h +++ b/src/cc65/goto.h @@ -44,8 +44,8 @@ -void GotoStatement (void); -/* Process a goto statement. */ +int GotoStatement (void); +/* Process a goto statement and return one of the SF_xxx flags from stmt.h. */ void DoLabel (void); /* Define a goto label. */ diff --git a/src/cc65/lineinfo.h b/src/cc65/lineinfo.h index 02e77cd9c..376dc761c 100644 --- a/src/cc65/lineinfo.h +++ b/src/cc65/lineinfo.h @@ -104,7 +104,7 @@ LineInfo* GetCurLineInfo (void); void UpdateCurrentLineInfo (const StrBuf* Line); /* Update the current line info - called if a new line is read */ -void RememberCheckedLI (struct LineInfo* LI); +void RememberCheckedLI (LineInfo* LI); /* Remember the latest checked line info struct */ LineInfo* GetPrevCheckedLI (void); diff --git a/src/cc65/stmt.c b/src/cc65/stmt.c index e28b844b5..51a8a6966 100644 --- a/src/cc65/stmt.c +++ b/src/cc65/stmt.c @@ -154,7 +154,7 @@ static int IfStatement (void) { unsigned Label1; unsigned TestResult; - int GotBreak; + int StmtFlags; /* Skip the if */ NextToken (); @@ -164,20 +164,27 @@ static int IfStatement (void) TestResult = TestInParens (Label1, 0); /* Parse the if body */ - GotBreak = AnyStatement (0); + StmtFlags = AnyStatement (0, 0); /* Else clause present? */ if (CurTok.Tok != TOK_ELSE) { g_defcodelabel (Label1); - /* Since there's no else clause, we're not sure, if the a break - ** statement is really executed. + /* If the test result is always true, any special statement (return, + ** break etc.) is always executed. If not, we cannot be sure. */ - return 0; + if (TestResult != TESTEXPR_TRUE) { + /* There's no else so we're not sure, if any special statements + ** are really executed. + */ + StmtFlags &= ~SF_MASK_UNREACH; + } } else { + int StmtFlags2; + /* Generate a jump around the else branch */ unsigned Label2 = GetLocalLabel (); g_jump (Label2); @@ -195,22 +202,40 @@ static int IfStatement (void) /* Define the target for the first test */ g_defcodelabel (Label1); - /* Total break only if both branches had a break. */ - GotBreak &= AnyStatement (0); + /* Parse the else body and evaluate special statements */ + StmtFlags2 = AnyStatement (0, 0); + if (TestResult == TESTEXPR_FALSE) { + /* The "else" part is always executed */ + StmtFlags = StmtFlags2; + } else if (TestResult != TESTEXPR_TRUE) { + /* If both branches have a special statement the following code is + ** unreachable and we combine the flags. Otherwise the code + ** following the "if" is always reachable. + */ + StmtFlags |= SF_Any (StmtFlags2); + if (SF_Unreach (StmtFlags) && SF_Unreach (StmtFlags2)) { + StmtFlags |= SF_Unreach (StmtFlags2); + } else { + StmtFlags &= ~SF_MASK_UNREACH; + } + } /* Generate the label for the else clause */ g_defcodelabel (Label2); - - /* Done */ - return GotBreak; } + + /* Done */ + return StmtFlags; } -static void DoStatement (void) +static int DoStatement (void) /* Handle the 'do' statement */ { + int StmtFlags; + unsigned TestResult; + /* Get the loop control labels */ unsigned LoopLabel = GetLocalLabel (); unsigned BreakLabel = GetLocalLabel (); @@ -226,14 +251,14 @@ static void DoStatement (void) g_defcodelabel (LoopLabel); /* Parse the loop body */ - AnyStatement (0); + StmtFlags = AnyStatement (0, 0); /* Output the label for a continue */ g_defcodelabel (ContinueLabel); /* Parse the end condition */ Consume (TOK_WHILE, "'while' expected"); - TestInParens (LoopLabel, 1); + TestResult = TestInParens (LoopLabel, 1); ConsumeSemi (); /* Define the break label */ @@ -241,17 +266,52 @@ static void DoStatement (void) /* Remove the loop from the loop stack */ DelLoop (); + + /* Fix the flags for the loop. */ + if (TestResult == TESTEXPR_TRUE) { + /* If the loop condition is always true, and we do not have a + ** "break" statement, the loop won't terminate. So the only valid + ** "unreach" flag is that for the endless loop. Otherwise - if there + ** is a "break" statement, the code after the loop is reachable. + */ + StmtFlags &= ~SF_MASK_UNREACH; + if (!SF_Any_Break (StmtFlags)) { + StmtFlags |= (SF_OTHER | SF_ANY_OTHER); + } + } else if (SF_Any_Break (StmtFlags)) { + /* If the loop condition is not always true, but we have a "break" + ** anywhere, the following code is reachable. + */ + StmtFlags &= ~SF_MASK_UNREACH; + } else { + /* Otherwise the last statement in the loop determines the status for + ** the following code, so we have to change "continue" into "other" + ** but apart from that flags stay as they are. + */ + if (SF_Continue (StmtFlags)) { + StmtFlags &= ~SF_CONTINUE; + StmtFlags |= (SF_OTHER | SF_ANY_OTHER); + } + } + + /* "break" and "continue" are not relevant for the following code */ + StmtFlags &= ~(SF_ANY_BREAK | SF_ANY_CONTINUE); + + /* Done */ + return StmtFlags; } -static void WhileStatement (void) +static int WhileStatement (void) /* Handle the 'while' statement */ { int PendingToken; CodeMark CondCodeStart; /* Start of condition evaluation code */ CodeMark CondCodeEnd; /* End of condition evaluation code */ CodeMark Here; /* "Here" location of code */ + unsigned TestResult; /* Result of the while loop test expression */ + int StmtFlags; /* Get the loop control labels */ unsigned LoopLabel = GetLocalLabel (); @@ -274,8 +334,15 @@ static void WhileStatement (void) /* Remember the current position */ GetCodePos (&CondCodeStart); - /* Test the loop condition */ - TestInParens (LoopLabel, 1); + /* Test the loop condition. While loops are somewhat different from other + ** loops: While an "always true" condition is used often for endless loops + ** (or loops left by "break"), an "always false" condition doesn't make + ** sense, so we check that here and warn about it. + */ + TestResult = TestInParens (LoopLabel, 1); + if (TestResult == TESTEXPR_FALSE) { + UnreachableCodeWarning (); + } /* Remember the end of the condition evaluation code */ GetCodePos (&CondCodeEnd); @@ -284,7 +351,7 @@ static void WhileStatement (void) g_defcodelabel (LoopLabel); /* Loop body */ - AnyStatement (&PendingToken); + StmtFlags = AnyStatement (&PendingToken, 0); /* Emit the while condition label */ g_defcodelabel (CondLabel); @@ -303,11 +370,34 @@ static void WhileStatement (void) /* Remove the loop from the loop stack */ DelLoop (); + + /* Fix the flags for the loop. */ + if (TestResult == TESTEXPR_TRUE) { + /* If the loop condition is always true, and we do not have a + ** "break" statement, the loop won't terminate. So the only valid + ** "unreach" flag is that for the endless loop. Otherwise - if there + ** is a "break" statement, the code after the loop is reachable. + */ + StmtFlags &= ~SF_MASK_UNREACH; + if (!SF_Any_Break (StmtFlags)) { + StmtFlags |= (SF_OTHER | SF_ANY_OTHER); + } + } else { + /* If the loop condition is not always true, the code after the loop + ** is always reachable. + */ + StmtFlags &= ~SF_MASK_UNREACH; + } + /* "break" and "continue" are not relevant for the following code */ + StmtFlags &= ~(SF_ANY_BREAK | SF_ANY_CONTINUE); + + /* Done */ + return StmtFlags; } -static void ReturnStatement (void) +static int ReturnStatement (void) /* Handle the 'return' statement */ { ExprDesc Expr; @@ -357,11 +447,14 @@ static void ReturnStatement (void) /* Output a jump to the function exit code */ g_jump (F_GetRetLab (CurrentFunc)); + + /* Done */ + return SF_RETURN | SF_ANY_RETURN; } -static void BreakStatement (void) +static int BreakStatement (void) /* Handle the 'break' statement */ { LoopDesc* L; @@ -376,7 +469,7 @@ static void BreakStatement (void) if (L == 0) { /* Error: No current loop */ Error ("'break' statement not within loop or switch"); - return; + return SF_NONE; } /* Correct the stack pointer if needed */ @@ -384,11 +477,14 @@ static void BreakStatement (void) /* Jump to the exit label of the loop */ g_jump (L->BreakLabel); + + /* Done */ + return SF_BREAK | SF_ANY_BREAK; } -static void ContinueStatement (void) +static int ContinueStatement (void) /* Handle the 'continue' statement */ { LoopDesc* L; @@ -411,7 +507,7 @@ static void ContinueStatement (void) /* Did we find it? */ if (L == 0) { Error ("'continue' statement not within a loop"); - return; + return SF_NONE; } /* Correct the stackpointer if needed */ @@ -419,17 +515,22 @@ static void ContinueStatement (void) /* Jump to next loop iteration */ g_jump (L->ContinueLabel); + + /* Done */ + return SF_CONTINUE | SF_ANY_CONTINUE; } -static void ForStatement (void) +static int ForStatement (void) /* Handle a 'for' statement */ { int HaveIncExpr; CodeMark IncExprStart; CodeMark IncExprEnd; int PendingToken; + unsigned TestResult; + int StmtFlags; /* Get several local labels needed later */ unsigned TestLabel = GetLocalLabel (); @@ -463,9 +564,10 @@ static void ForStatement (void) /* Parse the test expression */ if (CurTok.Tok != TOK_SEMI) { - Test (BodyLabel, 1); + TestResult = Test (BodyLabel, 1); g_jump (BreakLabel); } else { + TestResult = TESTEXPR_TRUE; g_jump (BodyLabel); } ConsumeSemi (); @@ -497,7 +599,7 @@ static void ForStatement (void) /* Loop body */ g_defcodelabel (BodyLabel); - AnyStatement (&PendingToken); + StmtFlags = AnyStatement (&PendingToken, 0); /* If we had an increment expression, move the code to the bottom of ** the loop. In this case we don't need to jump there at the end of @@ -520,16 +622,21 @@ static void ForStatement (void) /* Remove the loop from the loop stack */ DelLoop (); + + /* If the condition is always true, any special statements are always + ** executed. Otherwise we don't know. + */ + return (TestResult == TESTEXPR_TRUE)? StmtFlags : SF_NONE; } -static int CompoundStatement (int* PendingToken) +static int CompoundStatement (int* PendingToken, struct SwitchCtrl* Switch) /* Compound statement. Allow any number of statements inside braces. The ** function returns true if the last statement was a break or return. */ { - int GotBreak = 0; + int StmtFlags; /* Remember the stack at block entry */ int OldStack = StackPtr; @@ -544,17 +651,11 @@ static int CompoundStatement (int* PendingToken) /* Parse local variable declarations if any */ DeclareLocals (); - /* Now process statements in this block */ - while (CurTok.Tok != TOK_RCURLY) { - if (CurTok.Tok != TOK_CEOF) { - GotBreak = AnyStatement (0); - } else { - break; - } - } + /* Now process statements in this block checking for unreachable code */ + StmtFlags = StatementBlock (Switch); /* Clean up the stack if the codeflow may reach the end */ - if (!GotBreak) { + if ((StmtFlags & SF_MASK_UNREACH) == SF_NONE) { g_space (StackPtr - OldStack); } @@ -564,7 +665,6 @@ static int CompoundStatement (int* PendingToken) if (OldBlockStackSize != CollCount (&CurrentFunc->LocalsBlockStack)) { CollPop (&CurrentFunc->LocalsBlockStack); } - StackPtr = OldStack; /* Emit references to imports/exports for this block */ @@ -576,12 +676,13 @@ static int CompoundStatement (int* PendingToken) /* Skip '}' */ CheckTok (TOK_RCURLY, "'}' expected", PendingToken); - return GotBreak; + /* Done */ + return StmtFlags; } -static void Statement (int* PendingToken) +static int Statement (int* PendingToken) /* Single-line statement */ { ExprDesc Expr; @@ -615,54 +716,73 @@ static void Statement (int* PendingToken) } CheckSemi (PendingToken); + + /* Done. All special statements are handled in other subroutines. */ + return SF_NONE; } -static int ParseAnyLabels (void) -/* Return -1 if there are any labels with a statement */ -{ - unsigned PrevErrorCount = ErrorCount; - int HasLabels = 0; - for (;;) { - if (CurTok.Tok == TOK_IDENT && NextTok.Tok == TOK_COLON) { - /* C 'goto' label */ - DoLabel (); - } else if (CurTok.Tok == TOK_CASE) { - /* C 'case' label */ - CaseLabel (); - } else if (CurTok.Tok == TOK_DEFAULT) { - /* C 'default' label */ - DefaultLabel (); - } else { - /* No labels */ - break; - } - HasLabels = 1; - } - - if (HasLabels) { - if (PrevErrorCount != ErrorCount || CheckLabelWithoutStatement ()) { - return -1; - } - } - - return 0; -} - - - -int AnyStatement (int* PendingToken) -/* Statement parser. Returns 1 if the statement does a return/break, returns -** 0 otherwise. If the PendingToken pointer is not NULL, the function will -** not skip the terminating token of the statement (closing brace or -** semicolon), but store true if there is a pending token, and false if there -** is none. The token is always checked, so there is no need for the caller to -** check this token, it must be skipped, however. If the argument pointer is -** NULL, the function will skip the token. +int StatementBlock (struct SwitchCtrl* Switch) +/* Parse multiple statements within curly braces checking for unreachable +** code. Returns the SF_xxx flags for the last statement. */ { - int GotBreak = 0; + /* We want to emit an "unreachable code" warning for statements following + ** a "goto", "return" etc. But only - and this is what complicates it - + ** if the following statement is not preceeded by a label. Since the latter + ** means that a jump may go there so the statement is actually reachable. + */ + if (CurTok.Tok != TOK_RCURLY && CurTok.Tok != TOK_CEOF) { + LineInfo* LI1 = UseLineInfo (GetDiagnosticLI ()); + int StmtFlags1 = AnyStatement (0, Switch); + int UnreachableWarning = 0; + while (CurTok.Tok != TOK_RCURLY && CurTok.Tok != TOK_CEOF) { + LineInfo* LI2 = UseLineInfo (GetDiagnosticLI ()); + int StmtFlags2 = AnyStatement (0, Switch); + if (!UnreachableWarning) { + /* Check if the code is unreachable because of a preceeding + ** jump and if the code doesn't have a jump label. + */ + if ((StmtFlags1 & SF_MASK_UNREACH) != SF_NONE && + (StmtFlags2 & SF_MASK_LABEL) == SF_NONE) { + LIUnreachableCodeWarning (LI2); + UnreachableWarning = 1; + } + } + if (LI1) { + ReleaseLineInfo (LI1); + } + LI1 = LI2; + StmtFlags1 = (StmtFlags1 & SF_MASK_ANY) | StmtFlags2; + } + if (LI1) { + ReleaseLineInfo (LI1); + } + return StmtFlags1; + } else { + return SF_NONE; + } +} + + + +int AnyStatement (int* PendingToken, struct SwitchCtrl* Switch) +/* Statement parser. Returns one of the SF_xxx flags describing if the +** statement does a return/break. If the PendingToken pointer is not NULL, +** the function will not skip the terminating token of the statement (closing +** brace or semicolon), but store true if there is a pending token, and false +** if there is none. The token is always checked, so there is no need for the +** caller to check this token, it must be skipped, however. If the argument +** pointer is NULL, the function will skip the token. When called to parse a +** switch body, the switch control structure must be passed via the Switch +** argument. Otherwise it must be NULL. +*/ +{ + int LabelFlags = SF_NONE; + int StmtFlags; + unsigned PrevErrorCount; + LineInfo* LI; /* Assume no pending token */ if (PendingToken) { @@ -670,75 +790,113 @@ int AnyStatement (int* PendingToken) } /* Handle any labels. A label is always part of a statement, it does not - ** replace one. + ** replace one. If we have errors parsing labels, return without reading + ** the following statement. */ - if (ParseAnyLabels ()) { - return 0; + PrevErrorCount = ErrorCount; + while (1) { + if (CurTok.Tok == TOK_IDENT && NextTok.Tok == TOK_COLON) { + /* C 'goto' label */ + DoLabel (); + LabelFlags |= SF_LABEL_GOTO; + } else if (CurTok.Tok == TOK_CASE) { + /* C 'case' label */ + CaseLabel (); + LabelFlags |= SF_LABEL_CASE; + } else if (CurTok.Tok == TOK_DEFAULT) { + /* C 'default' label */ + DefaultLabel (); + LabelFlags |= SF_LABEL_DEFAULT; + } else { + /* No labels */ + break; + } + } + if (LabelFlags != SF_NONE) { + /* We had labels, check for errors */ + if (PrevErrorCount != ErrorCount || CheckLabelWithoutStatement ()) { + return SF_NONE; + } } + /* Remember the line info for the now following statement */ + LI = UseLineInfo (GetDiagnosticLI ()); + + /* Now look at the actual statement. */ switch (CurTok.Tok) { case TOK_IF: - GotBreak = IfStatement (); + StmtFlags = IfStatement (); break; case TOK_SWITCH: - SwitchStatement (); + StmtFlags = SwitchStatement (); break; case TOK_WHILE: - WhileStatement (); + StmtFlags = WhileStatement (); break; case TOK_DO: - DoStatement (); + StmtFlags = DoStatement (); break; case TOK_FOR: - ForStatement (); + StmtFlags = ForStatement (); break; case TOK_GOTO: - GotoStatement (); + StmtFlags = GotoStatement (); CheckSemi (PendingToken); - GotBreak = 1; break; case TOK_RETURN: - ReturnStatement (); + StmtFlags = ReturnStatement (); CheckSemi (PendingToken); - GotBreak = 1; break; case TOK_BREAK: - BreakStatement (); + StmtFlags = BreakStatement (); CheckSemi (PendingToken); - GotBreak = 1; break; case TOK_CONTINUE: - ContinueStatement (); + StmtFlags = ContinueStatement (); CheckSemi (PendingToken); - GotBreak = 1; break; case TOK_SEMI: /* Empty statement. Ignore it */ CheckSemi (PendingToken); + StmtFlags = SF_NONE; break; case TOK_LCURLY: - GotBreak = CompoundStatement (PendingToken); + StmtFlags = CompoundStatement (PendingToken, Switch); break; default: /* Simple statement */ - Statement (PendingToken); + StmtFlags = Statement (PendingToken); break; } + /* The flags for labels returned by subroutines are invalid in most cases, + ** so we remove them and use the ones determined before. + */ + StmtFlags = (StmtFlags & ~SF_MASK_LABEL) | LabelFlags; + /* Reset SQP flags */ SetSQPFlags (SQP_KEEP_NONE); - return GotBreak; + /* If we're inside a switch, do tracking of special statements */ + if (Switch) { + SwitchBodyStatement (Switch, LI, StmtFlags); + } + + /* Release the line info we remembered above */ + ReleaseLineInfo (LI); + + /* Done */ + return StmtFlags; } diff --git a/src/cc65/stmt.h b/src/cc65/stmt.h index 8cff99d92..2b91d06bf 100644 --- a/src/cc65/stmt.h +++ b/src/cc65/stmt.h @@ -38,20 +38,128 @@ +/*****************************************************************************/ +/* Data */ +/*****************************************************************************/ + + +/* Enumeration for the AnyStatement() return flags. It is used for simple flow +** analysis and tracks occurance of "return", "break", "goto" and "continue" +** (anything that jumps). The flags for the distinct statements are only set +** if they are always executed. So for example in case of an if/else statement, +** SF_RETURN is set only if both branches contain a "return". The SF_ANY flag +** is set of any of the statements occurred. So for an if/else statement, if +** one branch contains a "return" and the other a "continue" statement, neither +** SF_RETURN, nor SF_CONTINUE is set, but SF_ANY. +** There are some additional flags that tell if the statement parsed was +** preceeded by at least one label. These flags do not also set SF_ANY. +*/ +enum { + SF_NONE = 0x0000, + + /* Flags for special statements that cause the next statement to be + ** unreachable. They are only return as long as the condition is true. + ** Please note that a statement function can return more than one of + ** these flags set. For example for an "if" that has "return" in one + ** branch and "continue" in another. + */ + SF_RETURN = 0x00001, /* Code definitely returns */ + SF_BREAK = 0x00002, /* Code definitely breaks */ + SF_GOTO = 0x00004, /* Code definitely jumps */ + SF_CONTINUE = 0x00008, /* Code definitely continues */ + SF_OTHER = 0x00010, /* Endless loop, terminating func */ + SF_MASK_UNREACH = 0x000FF, /* Following code is unreachable */ + + /* Flags for the occurance of any of the conditions from above. Statements + ** will clear whatever is no longer valid when exiting. A while loop, for + ** example will never return SF_ANY_BREAK or SF_ANY_CONTINUE since that was + ** handled inside the loop. + */ + SF_ANY_RETURN = 0x00100, /* Code contains a return statement */ + SF_ANY_BREAK = 0x00200, /* Code contains a break statement */ + SF_ANY_GOTO = 0x00400, /* Code contains a goto statement */ + SF_ANY_CONTINUE = 0x00800, /* Code contains a continue statement */ + SF_ANY_OTHER = 0x01000, /* Code contains endless loop etc. */ + SF_MASK_ANY = 0x0FF00, /* Code contains any of the above */ + + /* Flags for labels */ + SF_LABEL_GOTO = 0x10000, /* Statement preceeded by goto label */ + SF_LABEL_CASE = 0x20000, /* Statement preceeded by case label */ + SF_LABEL_DEFAULT = 0x40000, /* Statement preceeded by default label */ + SF_MASK_LABEL = 0x70000, /* Mask for any label */ +}; + +/* Forward */ +struct SwitchCtrl; + + + +/*****************************************************************************/ +/* Functions to handle the flow control flags */ +/*****************************************************************************/ + + + +static inline int SF_Break (int F) +/* Return just the "break" flag in F */ +{ + return (F & SF_BREAK); +} + +static inline int SF_Continue (int F) +/* Return just the "continue" flag in F */ +{ + return (F & SF_CONTINUE); +} + +static inline int SF_Unreach (int F) +/* Return just the "unreachable" part of the given flags */ +{ + return (F & SF_MASK_UNREACH); +} + +static inline int SF_Any_Break (int F) +/* Check if there was any "break" statement */ +{ + return (F & SF_ANY_BREAK); +} + +static inline int SF_Any (int F) +/* Return just the "any" part of the given flags */ +{ + return (F & SF_MASK_ANY); +} + +static inline int SF_Label (int F) +/* Return just the "label" part of the given flags */ +{ + return (F & SF_MASK_LABEL); +} + + + + /*****************************************************************************/ /* Code */ /*****************************************************************************/ -int AnyStatement (int* PendingToken); -/* Statement parser. Returns 1 if the statement does a return/break, returns -** 0 otherwise. If the PendingToken pointer is not NULL, the function will -** not skip the terminating token of the statement (closing brace or -** semicolon), but store true if there is a pending token, and false if there -** is none. The token is always checked, so there is no need for the caller to -** check this token, it must be skipped, however. If the argument pointer is -** NULL, the function will skip the token. +int StatementBlock (struct SwitchCtrl* Switch); +/* Parse multiple statements within curly braces checking for unreachable +** code. Returns the SF_xxx flags for the last statement. +*/ + +int AnyStatement (int* PendingToken, struct SwitchCtrl* Switch); +/* Statement parser. Returns one of the SF_xxx flags describing if the +** statement does a return/break. If the PendingToken pointer is not NULL, +** the function will not skip the terminating token of the statement (closing +** brace or semicolon), but store true if there is a pending token, and false +** if there is none. The token is always checked, so there is no need for the +** caller to check this token, it must be skipped, however. If the argument +** pointer is NULL, the function will skip the token. When called to parse a +** switch body, the switch control structure must be passed via the Switch +** argument. Otherwise it must be NULL. */ diff --git a/src/cc65/swstmt.c b/src/cc65/swstmt.c index fc213c9a1..591c893a4 100644 --- a/src/cc65/swstmt.c +++ b/src/cc65/swstmt.c @@ -61,15 +61,23 @@ +/* Some bitmapped flags for use in SwitchCtrl */ +#define SC_NONE 0x0000 +#define SC_CASE 0x0001 /* We had a case label */ +#define SC_DEFAULT 0x0002 /* We had a default label */ +#define SC_MASK_LABEL 0x0003 /* Mask for the labels */ +#define SC_WEIRD 0x0004 /* Flag for a weird switch that contains code + ** outside of any label. + */ + typedef struct SwitchCtrl SwitchCtrl; struct SwitchCtrl { Collection* Nodes; /* CaseNode tree */ const Type* ExprType; /* Switch controlling expression type */ unsigned Depth; /* Number of bytes the selector type has */ unsigned DefaultLabel; /* Label for the default branch */ - - - + unsigned CtrlFlags; /* Bitmapped flags as defined above */ + int StmtFlags; /* Collected statement flags */ }; /* Pointer to current switch control struct */ @@ -77,14 +85,60 @@ static SwitchCtrl* Switch = 0; +/*****************************************************************************/ +/* Functions that work with struct SwitchExpr */ +/*****************************************************************************/ + + + +static int SC_Label (const SwitchCtrl* S) +/* Check if we had a switch label */ +{ + return (S->CtrlFlags & SC_MASK_LABEL) != SC_NONE; +} + + + +static int SC_IsWeird (const SwitchCtrl* S) +/* Check if this switch is weird */ +{ + return (S->CtrlFlags & SC_WEIRD) != SC_NONE; +} + + + +static void SC_SetCase (SwitchCtrl* S) +/* Set the current label type to "case" */ +{ + S->CtrlFlags |= SC_CASE; +} + + + +static void SC_SetDefault (SwitchCtrl* S) +/* Set the current label type to "default" */ +{ + S->CtrlFlags |= SC_DEFAULT; +} + + + +static void SC_MakeWeird (SwitchCtrl* S) +/* Mark the switch as weird */ +{ + S->CtrlFlags |= SC_WEIRD; +} + + + /*****************************************************************************/ /* Code */ /*****************************************************************************/ -void SwitchStatement (void) -/* Handle a switch statement for chars with a cmp cascade for the selector */ +int SwitchStatement (void) +/* Handle a 'switch' statement and return the corresponding SF_xxx flags */ { ExprDesc SwitchExpr; /* Switch statement expression */ CodeMark CaseCodeStart; /* Start of code marker */ @@ -92,7 +146,7 @@ void SwitchStatement (void) CodeMark SwitchCodeEnd; /* End of switch code */ unsigned ExitLabel; /* Exit label */ unsigned SwitchCodeLabel;/* Label for the switch code */ - int HaveBreak = 0; /* True if the last statement had a break */ + int StmtFlags; /* True if the last statement had a break */ int RCurlyBrace; /* True if last token is right curly brace */ SwitchCtrl* OldSwitch; /* Pointer to old switch control data */ SwitchCtrl SwitchData; /* New switch data */ @@ -136,6 +190,8 @@ void SwitchStatement (void) SwitchData.ExprType = SwitchExpr.Type; SwitchData.Depth = SizeOf (SwitchExpr.Type); SwitchData.DefaultLabel = 0; + SwitchData.CtrlFlags = SC_NONE; + SwitchData.StmtFlags = SF_NONE; OldSwitch = Switch; Switch = &SwitchData; @@ -148,7 +204,7 @@ void SwitchStatement (void) /* Parse the following statement, which may actually be a compound ** statement if there is a curly brace at the current input position */ - HaveBreak = AnyStatement (&RCurlyBrace); + StmtFlags = AnyStatement (&RCurlyBrace, Switch); /* Check if we had any labels */ if (CollCount (SwitchData.Nodes) == 0 && SwitchData.DefaultLabel == 0) { @@ -162,7 +218,7 @@ void SwitchStatement (void) ** carry the label), add a jump to the exit. If it is useless, the ** optimizer will remove it later. */ - if (!HaveBreak) { + if (SF_Unreach (StmtFlags) == SF_NONE) { g_jump (ExitLabel); } @@ -201,6 +257,11 @@ void SwitchStatement (void) if (RCurlyBrace) { NextToken (); } + + /* We only return the combined "any" flags from all the statements within + ** the switch. Minus "break" which is handled inside the switch. + */ + return SwitchData.StmtFlags & ~SF_ANY_BREAK; } @@ -268,6 +329,9 @@ void CaseLabel (void) Warning (DiagMsg, CaseExpr.IVal); } + /* Remember that we're in a case label section now */ + SC_SetCase (Switch); + } else { /* case keyword outside a switch statement */ @@ -297,6 +361,9 @@ void DefaultLabel (void) Switch->DefaultLabel = GetLocalLabel (); g_defcodelabel (Switch->DefaultLabel); + /* Remember that we're in the default label section now */ + SC_SetDefault (Switch); + } else { /* We had the default label already */ Error ("Multiple default labels in one switch"); @@ -312,3 +379,31 @@ void DefaultLabel (void) /* Skip the colon */ ConsumeColon (); } + + + +void SwitchBodyStatement (struct SwitchCtrl* S, LineInfo* LI, int StmtFlags) +/* Helper function for flow analysis. Must be called for all statements within +** a switch passing the flags for special statements. +*/ +{ + /* The control structure passed must be the current one */ + PRECONDITION (S == Switch); + + /* Handle code without a label in the switch */ + if (SC_Label (S) == SC_NONE) { + /* This is a statement that preceedes any switch labels. If the + ** switch is not already marked as weird, output and the current + ** statement has no label, output a warning about unreachable code. + */ + if (!SC_IsWeird (S)) { + if (!SF_Label (StmtFlags)) { + LIUnreachableCodeWarning (LI); + } + SC_MakeWeird (S); + } + } + + /* Collect the statement flags. */ + S->StmtFlags = SF_Any (S->StmtFlags | StmtFlags); +} diff --git a/src/cc65/swstmt.h b/src/cc65/swstmt.h index b4facf2cd..98d0cd952 100644 --- a/src/cc65/swstmt.h +++ b/src/cc65/swstmt.h @@ -38,14 +38,25 @@ +/*****************************************************************************/ +/* Data */ +/*****************************************************************************/ + + + +/* Forwards */ +struct SwitchCtrl; + + + /*****************************************************************************/ /* Code */ /*****************************************************************************/ -void SwitchStatement (void); -/* Handle a 'switch' statement */ +int SwitchStatement (void); +/* Handle a 'switch' statement and return the corresponding SF_xxx flags */ void CaseLabel (void); /* Handle a case label */ @@ -53,6 +64,11 @@ void CaseLabel (void); void DefaultLabel (void); /* Handle a default label */ +void SwitchBodyStatement (struct SwitchCtrl* Switch, LineInfo* LI, int RetFlags); +/* Helper function for flow analysis. Must be called for all statements within +** a switch passing the flags for special statements. +*/ + /* End of swstmt.h */ diff --git a/test/misc/goto.ref b/test/misc/goto.ref index 2e0819887..d1fd3a793 100644 --- a/test/misc/goto.ref +++ b/test/misc/goto.ref @@ -1,4 +1,9 @@ +goto.c:3: Warning: Unreachable code goto.c:8: Warning: Goto at line 8 to label start jumps into a block with initialization of an object that has automatic storage duration +goto.c:17: Warning: Unreachable code +goto.c:38: Warning: Unreachable code +goto.c:59: Warning: Unreachable code +goto.c:80: Warning: Unreachable code goto.c:97: Warning: Variable 'a' is defined but never used goto.c:117: Warning: Variable 'a' is defined but never used goto.c:137: Warning: Variable 'a' is defined but never used @@ -9,6 +14,7 @@ goto.c:159: Warning: Goto at line 86 to label l8 jumps into a block with initial goto.c:159: Warning: Goto at line 106 to label l8 jumps into a block with initialization of an object that has automatic storage duration goto.c:159: Warning: Goto at line 126 to label l8 jumps into a block with initialization of an object that has automatic storage duration goto.c:159: Warning: Goto at line 146 to label l8 jumps into a block with initialization of an object that has automatic storage duration +goto.c:161: Warning: Unreachable code goto.c:180: Warning: Goto at line 24 to label l9 jumps into a block with initialization of an object that has automatic storage duration goto.c:180: Warning: Goto at line 45 to label l9 jumps into a block with initialization of an object that has automatic storage duration goto.c:180: Warning: Goto at line 66 to label l9 jumps into a block with initialization of an object that has automatic storage duration @@ -17,6 +23,7 @@ goto.c:180: Warning: Goto at line 107 to label l9 jumps into a block with initia goto.c:180: Warning: Goto at line 127 to label l9 jumps into a block with initialization of an object that has automatic storage duration goto.c:180: Warning: Goto at line 147 to label l9 jumps into a block with initialization of an object that has automatic storage duration goto.c:180: Warning: Goto at line 168 to label l9 jumps into a block with initialization of an object that has automatic storage duration +goto.c:182: Warning: Unreachable code goto.c:201: Warning: Goto at line 25 to label la jumps into a block with initialization of an object that has automatic storage duration goto.c:201: Warning: Goto at line 46 to label la jumps into a block with initialization of an object that has automatic storage duration goto.c:201: Warning: Goto at line 67 to label la jumps into a block with initialization of an object that has automatic storage duration @@ -26,6 +33,7 @@ goto.c:201: Warning: Goto at line 128 to label la jumps into a block with initia goto.c:201: Warning: Goto at line 148 to label la jumps into a block with initialization of an object that has automatic storage duration goto.c:201: Warning: Goto at line 169 to label la jumps into a block with initialization of an object that has automatic storage duration goto.c:201: Warning: Goto at line 190 to label la jumps into a block with initialization of an object that has automatic storage duration +goto.c:203: Warning: Unreachable code goto.c:221: Warning: Goto at line 26 to label lb jumps into a block with initialization of an object that has automatic storage duration goto.c:221: Warning: Goto at line 47 to label lb jumps into a block with initialization of an object that has automatic storage duration goto.c:221: Warning: Goto at line 68 to label lb jumps into a block with initialization of an object that has automatic storage duration @@ -57,6 +65,7 @@ goto.c:263: Warning: Goto at line 193 to label ld jumps into a block with initia goto.c:263: Warning: Goto at line 214 to label ld jumps into a block with initialization of an object that has automatic storage duration goto.c:263: Warning: Goto at line 234 to label ld jumps into a block with initialization of an object that has automatic storage duration goto.c:263: Warning: Goto at line 254 to label ld jumps into a block with initialization of an object that has automatic storage duration +goto.c:265: Warning: Unreachable code goto.c:271: Warning: Goto at line 271 to label l8 jumps into a block with initialization of an object that has automatic storage duration goto.c:272: Warning: Goto at line 272 to label l9 jumps into a block with initialization of an object that has automatic storage duration goto.c:273: Warning: Goto at line 273 to label la jumps into a block with initialization of an object that has automatic storage duration @@ -75,6 +84,7 @@ goto.c:284: Warning: Goto at line 215 to label le jumps into a block with initia goto.c:284: Warning: Goto at line 235 to label le jumps into a block with initialization of an object that has automatic storage duration goto.c:284: Warning: Goto at line 255 to label le jumps into a block with initialization of an object that has automatic storage duration goto.c:284: Warning: Goto at line 277 to label le jumps into a block with initialization of an object that has automatic storage duration +goto.c:286: Warning: Unreachable code goto.c:292: Warning: Goto at line 292 to label l8 jumps into a block with initialization of an object that has automatic storage duration goto.c:293: Warning: Goto at line 293 to label l9 jumps into a block with initialization of an object that has automatic storage duration goto.c:294: Warning: Goto at line 294 to label la jumps into a block with initialization of an object that has automatic storage duration @@ -94,6 +104,7 @@ goto.c:305: Warning: Goto at line 236 to label lf jumps into a block with initia goto.c:305: Warning: Goto at line 256 to label lf jumps into a block with initialization of an object that has automatic storage duration goto.c:305: Warning: Goto at line 278 to label lf jumps into a block with initialization of an object that has automatic storage duration goto.c:305: Warning: Goto at line 299 to label lf jumps into a block with initialization of an object that has automatic storage duration +goto.c:307: Warning: Unreachable code goto.c:313: Warning: Goto at line 313 to label l8 jumps into a block with initialization of an object that has automatic storage duration goto.c:314: Warning: Goto at line 314 to label l9 jumps into a block with initialization of an object that has automatic storage duration goto.c:315: Warning: Goto at line 315 to label la jumps into a block with initialization of an object that has automatic storage duration From f13284d3f82edfdf699f512c89b9a2067d88b7b7 Mon Sep 17 00:00:00 2001 From: Kugel Fuhr <98353208+kugelfuhr@users.noreply.github.com> Date: Thu, 17 Jul 2025 15:41:17 +0200 Subject: [PATCH 2/5] Move the warning about unreachable code out of the Test() function and into the callers. This has the effect that the location for the warning is much more precise than before. --- src/cc65/error.c | 46 ++++++++++++++++++++++----------------------- src/cc65/stmt.c | 5 +++++ src/cc65/testexpr.c | 10 +++------- src/cc65/testexpr.h | 5 ++--- 4 files changed, 33 insertions(+), 33 deletions(-) diff --git a/src/cc65/error.c b/src/cc65/error.c index 97e651890..a5cf4e3e8 100644 --- a/src/cc65/error.c +++ b/src/cc65/error.c @@ -425,31 +425,31 @@ void UnreachableCodeWarning (void) { if (IS_Get (&WarnUnreachableCode)) { - LineInfo* LI; + LineInfo* LI; - /* Add special handling for compound statements if the current token - ** is from the source. Doing this here is a bit hacky but unfortunately - ** there's no better place. - */ - if (CurTok.LI && NextTok.LI) { - if (CurTok.Tok == TOK_LCURLY) { - /* Do not point to the compoung statement but to the first - ** statement within it. If the compound statement is empty - ** do not even output a warning. This fails of course for - ** nested compounds but will do the right thing in most cases. - */ - if (NextTok.Tok == TOK_RCURLY) { - return; - } - LI = NextTok.LI; - } else { - LI = CurTok.LI; - } - } else { - LI = GetCurLineInfo (); - } + /* Add special handling for compound statements if the current token + ** is from the source. Doing this here is a bit hacky but unfortunately + ** there's no better place. + */ + if (CurTok.LI && NextTok.LI) { + if (CurTok.Tok == TOK_LCURLY) { + /* Do not point to the compoung statement but to the first + ** statement within it. If the compound statement is empty + ** do not even output a warning. This fails of course for + ** nested compounds but will do the right thing in most cases. + */ + if (NextTok.Tok == TOK_RCURLY) { + return; + } + LI = NextTok.LI; + } else { + LI = CurTok.LI; + } + } else { + LI = GetCurLineInfo (); + } - /* Now output the warning */ + /* Now output the warning */ LIWarning (EC_PARSER, LI, "Unreachable code"); } } diff --git a/src/cc65/stmt.c b/src/cc65/stmt.c index 51a8a6966..20618d8bb 100644 --- a/src/cc65/stmt.c +++ b/src/cc65/stmt.c @@ -163,6 +163,11 @@ static int IfStatement (void) Label1 = GetLocalLabel (); TestResult = TestInParens (Label1, 0); + /* Output a warning if the condition is always false */ + if (TestResult == TESTEXPR_FALSE) { + UnreachableCodeWarning (); + } + /* Parse the if body */ StmtFlags = AnyStatement (0, 0); diff --git a/src/cc65/testexpr.c b/src/cc65/testexpr.c index ced563d01..85355f07e 100644 --- a/src/cc65/testexpr.c +++ b/src/cc65/testexpr.c @@ -53,7 +53,7 @@ unsigned Test (unsigned Label, int Invert) /* Evaluate a boolean test expression and jump depending on the result of ** the test and on Invert. The function returns one of the TESTEXPR_xx codes -** defined above. If the jump is always true, a warning is output. +** defined above. */ { ExprDesc Expr; @@ -74,10 +74,7 @@ unsigned Test (unsigned Label, int Invert) Result = (Expr.IVal != 0) ? TESTEXPR_TRUE : TESTEXPR_FALSE; /* Constant rvalue */ - if (!Invert && Expr.IVal == 0) { - g_jump (Label); - UnreachableCodeWarning (); - } else if (Invert && Expr.IVal != 0) { + if ((!Invert && Expr.IVal == 0) || (Invert && Expr.IVal != 0)) { g_jump (Label); } @@ -125,8 +122,7 @@ unsigned Test (unsigned Label, int Invert) unsigned TestInParens (unsigned Label, int Invert) /* Evaluate a boolean test expression in parenthesis and jump depending on ** the result of the test * and on Invert. The function returns one of the -** TESTEXPR_xx codes defined above. If the jump is always true, a warning is -** output. +** TESTEXPR_xx codes defined above. */ { unsigned Result; diff --git a/src/cc65/testexpr.h b/src/cc65/testexpr.h index 84b957af2..8563838eb 100644 --- a/src/cc65/testexpr.h +++ b/src/cc65/testexpr.h @@ -59,14 +59,13 @@ unsigned Test (unsigned Label, int Invert); /* Evaluate a boolean test expression and jump depending on the result of ** the test and on Invert. The function returns one of the TESTEXPR_xx codes -** defined above. If the jump is always true, a warning is output. +** defined above. */ unsigned TestInParens (unsigned Label, int Invert); /* Evaluate a boolean test expression in parenthesis and jump depending on ** the result of the test * and on Invert. The function returns one of the -** TESTEXPR_xx codes defined above. If the jump is always true, a warning is -** output. +** TESTEXPR_xx codes defined above. */ From ed54e9b1688185b98f7a73b0471e6d1448b72574 Mon Sep 17 00:00:00 2001 From: Kugel Fuhr <98353208+kugelfuhr@users.noreply.github.com> Date: Thu, 17 Jul 2025 16:00:59 +0200 Subject: [PATCH 3/5] Added several flow control tests. --- test/misc/Makefile | 32 ++++++++++++- test/misc/bug2655.ref | 4 +- test/misc/flow-do-01.c | 87 ++++++++++++++++++++++++++++++++++++ test/misc/flow-do-01.ref | 5 +++ test/misc/flow-if-01.c | 62 +++++++++++++++++++++++++ test/misc/flow-if-01.ref | 3 ++ test/misc/flow-if-02.c | 49 ++++++++++++++++++++ test/misc/flow-if-02.ref | 2 + test/misc/flow-switch-01.c | 68 ++++++++++++++++++++++++++++ test/misc/flow-switch-01.ref | 1 + test/misc/flow-while-01.c | 70 +++++++++++++++++++++++++++++ test/misc/flow-while-01.ref | 3 ++ test/misc/flow-while-02.c | 74 ++++++++++++++++++++++++++++++ test/misc/flow-while-02.ref | 6 +++ 14 files changed, 463 insertions(+), 3 deletions(-) create mode 100644 test/misc/flow-do-01.c create mode 100644 test/misc/flow-do-01.ref create mode 100644 test/misc/flow-if-01.c create mode 100644 test/misc/flow-if-01.ref create mode 100644 test/misc/flow-if-02.c create mode 100644 test/misc/flow-if-02.ref create mode 100644 test/misc/flow-switch-01.c create mode 100644 test/misc/flow-switch-01.ref create mode 100644 test/misc/flow-while-01.c create mode 100644 test/misc/flow-while-01.ref create mode 100644 test/misc/flow-while-02.c create mode 100644 test/misc/flow-while-02.ref diff --git a/test/misc/Makefile b/test/misc/Makefile index 6a5611c68..3e8b0c157 100644 --- a/test/misc/Makefile +++ b/test/misc/Makefile @@ -152,7 +152,37 @@ $(WORKDIR)/bug2655.$1.$2.prg: bug2655.c $(ISEQUAL) | $(WORKDIR) $(CC65) -t sim$2 -$1 -o $$@ $$< $(NULLOUT) 2>$(WORKDIR)/bug2655.$1.$2.out $(ISEQUAL) $(WORKDIR)/bug2655.$1.$2.out bug2655.ref -$(WORKDIR)/limits.$1.$2.prg: limits.c $(ISEQUAL) | $(WORKDIR) +$(WORKDIR)/flow-do-01.$1.$2.prg: flow-do-01.c $(ISEQUAL) | $(WORKDIR) + $(if $(QUIET),echo misc/flow-do-01.$1.$2.prg) + $(CC65) -t sim$2 -$1 -o $$@ $$< $(NULLOUT) 2>$(WORKDIR)/flow-do-01.$1.$2.out + $(ISEQUAL) $(WORKDIR)/flow-do-01.$1.$2.out flow-do-01.ref + +$(WORKDIR)/flow-if-01.$1.$2.prg: flow-if-01.c $(ISEQUAL) | $(WORKDIR) + $(if $(QUIET),echo misc/flow-if-01.$1.$2.prg) + $(CC65) -t sim$2 -$1 -o $$@ $$< $(NULLOUT) 2>$(WORKDIR)/flow-if-01.$1.$2.out + $(ISEQUAL) $(WORKDIR)/flow-if-01.$1.$2.out flow-if-01.ref + +$(WORKDIR)/flow-if-02.$1.$2.prg: flow-if-02.c $(ISEQUAL) | $(WORKDIR) + $(if $(QUIET),echo misc/flow-if-02.$1.$2.prg) + $(CC65) -t sim$2 -$1 -o $$@ $$< $(NULLOUT) 2>$(WORKDIR)/flow-if-02.$1.$2.out + $(ISEQUAL) $(WORKDIR)/flow-if-02.$1.$2.out flow-if-02.ref + +$(WORKDIR)/flow-switch-01.$1.$2.prg: flow-switch-01.c $(ISEQUAL) | $(WORKDIR) + $(if $(QUIET),echo misc/flow-switch-01.$1.$2.prg) + $(CC65) -t sim$2 -$1 -o $$@ $$< $(NULLOUT) 2>$(WORKDIR)/flow-switch-01.$1.$2.out + $(ISEQUAL) $(WORKDIR)/flow-switch-01.$1.$2.out flow-switch-01.ref + +$(WORKDIR)/flow-while-01.$1.$2.prg: flow-while-01.c $(ISEQUAL) | $(WORKDIR) + $(if $(QUIET),echo misc/flow-while-01.$1.$2.prg) + $(CC65) -t sim$2 -$1 -o $$@ $$< $(NULLOUT) 2>$(WORKDIR)/flow-while-01.$1.$2.out + $(ISEQUAL) $(WORKDIR)/flow-while-01.$1.$2.out flow-while-01.ref + +$(WORKDIR)/flow-while-02.$1.$2.prg: flow-while-02.c $(ISEQUAL) | $(WORKDIR) + $(if $(QUIET),echo misc/flow-while-02.$1.$2.prg) + $(CC65) -t sim$2 -$1 -o $$@ $$< $(NULLOUT) 2>$(WORKDIR)/flow-while-02.$1.$2.out + $(ISEQUAL) $(WORKDIR)/flow-while-02.$1.$2.out flow-while-02.ref + +$(WORKDIR)/limits.$1.$2.prg: limits.c $(ISEQUAL) | $(WORKDIR) $(if $(QUIET),echo misc/limits.$1.$2.prg) $(CC65) -t sim$2 -$1 -o $$(@:.prg=.s) $$< $(NULLOUT) $(CATERR) $(CA65) -t sim$2 -o $$(@:.prg=.o) $$(@:.prg=.s) $(NULLERR) diff --git a/test/misc/bug2655.ref b/test/misc/bug2655.ref index 99e2d6698..7b1fd2e86 100644 --- a/test/misc/bug2655.ref +++ b/test/misc/bug2655.ref @@ -1,2 +1,2 @@ -bug2655.c:5: Warning: Unreachable code -bug2655.c:12: Warning: Unreachable code +bug2655.c:6: Warning: Unreachable code +bug2655.c:13: Warning: Unreachable code diff --git a/test/misc/flow-do-01.c b/test/misc/flow-do-01.c new file mode 100644 index 000000000..7d4cb86e2 --- /dev/null +++ b/test/misc/flow-do-01.c @@ -0,0 +1,87 @@ +int a; + +static int f1(void) +{ + do { + return a; + } while (a == 2); + /* Unreachable */ + a = 2; + return a; +} + +static int f2(void) +{ + do { + return a; + } while (1); + /* Unreachable */ + a = 2; + return a; +} + +static int f3(void) +{ + do { + return a; + } while (0); + /* Unreachable */ + a = 2; + return a; +} + +static int f4(void) +{ + do { + continue; /* Turns do/while into an endless loop */ + } while (0); + /* Unreachable */ + a = 2; + return a; +} + +static int f5(void) +{ + do { + if (a == 2) { + break; + } + return a; + } while (0); + /* Reachable */ + a = 2; + return a; +} + +static int f6(void) +{ + do { + if (a == 2) { + break; + } + continue; + } while (0); + /* Reachable */ + a = 2; + return a; +} + +static int f7(void) +{ + do { + if (a == 2) { + return a; + } else { + continue; + } + } while (1); + /* Unreachable */ + a = 2; + return a; +} + +int main(void) +{ + return f1() + f2() + f3() + f4() + f5() + f6() + f7(); +} + diff --git a/test/misc/flow-do-01.ref b/test/misc/flow-do-01.ref new file mode 100644 index 000000000..d082b9406 --- /dev/null +++ b/test/misc/flow-do-01.ref @@ -0,0 +1,5 @@ +flow-do-01.c:9: Warning: Unreachable code +flow-do-01.c:19: Warning: Unreachable code +flow-do-01.c:29: Warning: Unreachable code +flow-do-01.c:39: Warning: Unreachable code +flow-do-01.c:79: Warning: Unreachable code diff --git a/test/misc/flow-if-01.c b/test/misc/flow-if-01.c new file mode 100644 index 000000000..533a55c11 --- /dev/null +++ b/test/misc/flow-if-01.c @@ -0,0 +1,62 @@ +int a, b; + +static int f1(void) +{ + if (a == 1) { + return 1; + } + /* Reachable */ + a = 2; + return a; +} + +static int f2(void) +{ + if (a == 1) { + a = 2; + } else { + return 1; + } + /* Reachable */ + return a; +} + +static int f3(void) +{ + if (a == 1) { + return 1; + } else { + a = 2; + } + /* Reachable */ + return a; +} + +static int f4(void) +{ + if (a == 1) { + return 1; + } else { + return 0; + } + /* Unreachable */ + a = 2; +} + +static int f5(void) +{ + if (1) { + return 1; + } else { + /* Unreachable */ + return 0; + } + /* Unreachable */ + a = 2; +} + +int main(void) +{ + return f1() + f2() + f3() + f4() + f5(); +} + diff --git a/test/misc/flow-if-01.ref b/test/misc/flow-if-01.ref new file mode 100644 index 000000000..f7c547d23 --- /dev/null +++ b/test/misc/flow-if-01.ref @@ -0,0 +1,3 @@ +flow-if-01.c:43: Warning: Unreachable code +flow-if-01.c:52: Warning: Unreachable code +flow-if-01.c:55: Warning: Unreachable code diff --git a/test/misc/flow-if-02.c b/test/misc/flow-if-02.c new file mode 100644 index 000000000..6e30d9d5f --- /dev/null +++ b/test/misc/flow-if-02.c @@ -0,0 +1,49 @@ +int a, b; + +static int f1(void) +{ + if (0) { + /* Unreachable but no warning */ + } else { + a = 2; + } + return a; +} + +static int f2(void) +{ + if (0) { + /* Unreachable */ + a = 1; + } else { + a = 2; + } + return a; +} + +static int f3(void) +{ + if (1) { + a = 2; + } else { + /* Unreachable but no warning */ + } + return a; +} + +static int f4(void) +{ + if (1) { + a = 2; + } else { + /* Unreachable */ + a = 1; + } + return a; +} + +int main(void) +{ + return f1() + f2() + f3() + f4(); +} + diff --git a/test/misc/flow-if-02.ref b/test/misc/flow-if-02.ref new file mode 100644 index 000000000..91547afd7 --- /dev/null +++ b/test/misc/flow-if-02.ref @@ -0,0 +1,2 @@ +flow-if-02.c:17: Warning: Unreachable code +flow-if-02.c:40: Warning: Unreachable code diff --git a/test/misc/flow-switch-01.c b/test/misc/flow-switch-01.c new file mode 100644 index 000000000..b55c2f70f --- /dev/null +++ b/test/misc/flow-switch-01.c @@ -0,0 +1,68 @@ +int a; + +static int f1(void) +{ + switch (a) { + /* Unreachable */ + a = 3; + case 1: + a = 2; + break; + case 2: + a = 1; + break; + default: + a = 0; + break; + } + /* Reachable */ + return a; +} + +static int f2(void) +{ + switch (a) { + /* Reachable */ +L: a = 3; + case 1: + goto L; + case 2: + a = 1; + break; + default: + a = 0; + break; + } + /* Reachable */ + return a; +} + +static int f3(void) +{ + switch (a) { + case 1: return a; + case 2: return a+1; + default: return a+2; + } + /* Unreachable but no warning */ + return a; +} + +static int f4(void) +{ + switch (a) { + /* No warning */ + do { + case 1: ++a; continue; + case 2: return a+1; + default: return a+2; + } while (1); + } + /* Unreachable but no warning */ + return a; +} + +int main(void) +{ + return f1() + f2() + f3() + f4(); +} diff --git a/test/misc/flow-switch-01.ref b/test/misc/flow-switch-01.ref new file mode 100644 index 000000000..61391476b --- /dev/null +++ b/test/misc/flow-switch-01.ref @@ -0,0 +1 @@ +flow-switch-01.c:7: Warning: Unreachable code diff --git a/test/misc/flow-while-01.c b/test/misc/flow-while-01.c new file mode 100644 index 000000000..5ba4f8d9f --- /dev/null +++ b/test/misc/flow-while-01.c @@ -0,0 +1,70 @@ +int a; + +static int f1(void) +{ + while (1) { + ++a; + } + /* Unreachable */ + a = 2; + return a; +} + +static int f2(void) +{ + while (1) { + ++a; + if (a == 5) break; + } + /* Reachable */ + a = 2; + return a; +} + +static int f3(void) +{ + while (1) { + ++a; + return a; + } + /* Unreachable */ + a = 2; +} + +static int f4(void) +{ + while (0) { + /* Unreachable */ + ++a; + return a; + } + /* Reachable */ + a = 2; + return 0; +} + +static int f5(void) +{ + while (1) { + ++a; + if (a == 4) goto L; + return a; + } + /* Reachable via L */ +L: a = 2; +} + +static int f6(void) +{ + while (0) { + /* Unreachable but no warning */ + } + a = 2; + return a; +} + +int main(void) +{ + return f1() + f2() + f3() + f4() + f5() + f6(); +} + diff --git a/test/misc/flow-while-01.ref b/test/misc/flow-while-01.ref new file mode 100644 index 000000000..489d92ad1 --- /dev/null +++ b/test/misc/flow-while-01.ref @@ -0,0 +1,3 @@ +flow-while-01.c:9: Warning: Unreachable code +flow-while-01.c:31: Warning: Unreachable code +flow-while-01.c:38: Warning: Unreachable code diff --git a/test/misc/flow-while-02.c b/test/misc/flow-while-02.c new file mode 100644 index 000000000..11ea2c0c4 --- /dev/null +++ b/test/misc/flow-while-02.c @@ -0,0 +1,74 @@ +int a; + +static int f1(void) +{ + while (1) { + while (0) { + /* Unreachable */ + ++a; + } + } + /* Unreachable */ + a = 2; + return a; +} + +static int f2(void) +{ + while (1) { + do { + return a; + } while (0); + /* Unreachable */ + break; + } + /* Unreachable but no warning */ + a = 2; + return a; +} + +static int f3(void) +{ + do { + while (1) { + break; + } + } while (1); + /* Unreachable */ + a = 2; + return a; +} + +static int f4(void) +{ + do { + while (1) { + return a; + } + } while (0); + /* Unreachable */ + a = 2; + return a; +} + +static int f5(void) +{ + do { + do { + if (a == 2) { + return a; + } else { + continue; + } + } while (0); + } while (0); + /* Unreachable */ + a = 2; + return a; +} + +int main(void) +{ + return f1()/ + f2() + f3() + f4() + f5(); +} + diff --git a/test/misc/flow-while-02.ref b/test/misc/flow-while-02.ref new file mode 100644 index 000000000..2609f395a --- /dev/null +++ b/test/misc/flow-while-02.ref @@ -0,0 +1,6 @@ +flow-while-02.c:8: Warning: Unreachable code +flow-while-02.c:12: Warning: Unreachable code +flow-while-02.c:23: Warning: Unreachable code +flow-while-02.c:38: Warning: Unreachable code +flow-while-02.c:50: Warning: Unreachable code +flow-while-02.c:66: Warning: Unreachable code From 6d45a9412767eb7850ef5891b5732474bab37b4e Mon Sep 17 00:00:00 2001 From: Kugel Fuhr <98353208+kugelfuhr@users.noreply.github.com> Date: Thu, 17 Jul 2025 16:12:48 +0200 Subject: [PATCH 4/5] Do not output a warning about a missing "return" in a function if the function exit is unreachable. --- src/cc65/error.c | 2 +- src/cc65/function.c | 7 +++++-- test/misc/Makefile | 4 ++-- test/misc/flow-do-01.c | 8 ++++---- test/misc/flow-if-02.c | 4 ++-- test/misc/flow-while-01.c | 6 +++--- 6 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/cc65/error.c b/src/cc65/error.c index a5cf4e3e8..ebd3a55c6 100644 --- a/src/cc65/error.c +++ b/src/cc65/error.c @@ -433,7 +433,7 @@ void UnreachableCodeWarning (void) */ if (CurTok.LI && NextTok.LI) { if (CurTok.Tok == TOK_LCURLY) { - /* Do not point to the compoung statement but to the first + /* Do not point to the compound statement but to the first ** statement within it. If the compound statement is empty ** do not even output a warning. This fails of course for ** nested compounds but will do the right thing in most cases. diff --git a/src/cc65/function.c b/src/cc65/function.c index 0e040d61d..c14372d43 100644 --- a/src/cc65/function.c +++ b/src/cc65/function.c @@ -453,6 +453,7 @@ void NewFunc (SymEntry* Func, FuncDesc* D) SymEntry* Param; const Type* RType; /* Real type used for struct parameters */ const Type* ReturnType; /* Return type */ + int StmtFlags; /* Flow control flags for the function compound */ /* Remember this function descriptor used for definition */ GetFuncDesc (Func->Type)->FuncDef = D; @@ -627,10 +628,12 @@ void NewFunc (SymEntry* Func, FuncDesc* D) CurrentFunc->TopLevelSP = StackPtr; /* Now process statements in this block checking for unreachable code */ - StatementBlock (0); + StmtFlags = StatementBlock (0); /* Check if this function is missing a return value */ - if (!F_HasVoidReturn (CurrentFunc) && !F_HasReturn (CurrentFunc)) { + if (!SF_Unreach (StmtFlags) && + !F_HasVoidReturn (CurrentFunc) && + !F_HasReturn (CurrentFunc)) { /* If this is the main function in a C99 environment returning an int, ** let it always return zero. Otherwise output a warning. */ diff --git a/test/misc/Makefile b/test/misc/Makefile index 3e8b0c157..a595d82a9 100644 --- a/test/misc/Makefile +++ b/test/misc/Makefile @@ -156,7 +156,7 @@ $(WORKDIR)/flow-do-01.$1.$2.prg: flow-do-01.c $(ISEQUAL) | $(WORKDIR) $(if $(QUIET),echo misc/flow-do-01.$1.$2.prg) $(CC65) -t sim$2 -$1 -o $$@ $$< $(NULLOUT) 2>$(WORKDIR)/flow-do-01.$1.$2.out $(ISEQUAL) $(WORKDIR)/flow-do-01.$1.$2.out flow-do-01.ref - + $(WORKDIR)/flow-if-01.$1.$2.prg: flow-if-01.c $(ISEQUAL) | $(WORKDIR) $(if $(QUIET),echo misc/flow-if-01.$1.$2.prg) $(CC65) -t sim$2 -$1 -o $$@ $$< $(NULLOUT) 2>$(WORKDIR)/flow-if-01.$1.$2.out @@ -182,7 +182,7 @@ $(WORKDIR)/flow-while-02.$1.$2.prg: flow-while-02.c $(ISEQUAL) | $(WORKDIR) $(CC65) -t sim$2 -$1 -o $$@ $$< $(NULLOUT) 2>$(WORKDIR)/flow-while-02.$1.$2.out $(ISEQUAL) $(WORKDIR)/flow-while-02.$1.$2.out flow-while-02.ref -$(WORKDIR)/limits.$1.$2.prg: limits.c $(ISEQUAL) | $(WORKDIR) +$(WORKDIR)/limits.$1.$2.prg: limits.c $(ISEQUAL) | $(WORKDIR) $(if $(QUIET),echo misc/limits.$1.$2.prg) $(CC65) -t sim$2 -$1 -o $$(@:.prg=.s) $$< $(NULLOUT) $(CATERR) $(CA65) -t sim$2 -o $$(@:.prg=.o) $$(@:.prg=.s) $(NULLERR) diff --git a/test/misc/flow-do-01.c b/test/misc/flow-do-01.c index 7d4cb86e2..d1997311f 100644 --- a/test/misc/flow-do-01.c +++ b/test/misc/flow-do-01.c @@ -43,7 +43,7 @@ static int f4(void) static int f5(void) { do { - if (a == 2) { + if (a == 2) { break; } return a; @@ -56,7 +56,7 @@ static int f5(void) static int f6(void) { do { - if (a == 2) { + if (a == 2) { break; } continue; @@ -69,11 +69,11 @@ static int f6(void) static int f7(void) { do { - if (a == 2) { + if (a == 2) { return a; } else { continue; - } + } } while (1); /* Unreachable */ a = 2; diff --git a/test/misc/flow-if-02.c b/test/misc/flow-if-02.c index 6e30d9d5f..ff60e5ce0 100644 --- a/test/misc/flow-if-02.c +++ b/test/misc/flow-if-02.c @@ -14,7 +14,7 @@ static int f2(void) { if (0) { /* Unreachable */ - a = 1; + a = 1; } else { a = 2; } @@ -37,7 +37,7 @@ static int f4(void) a = 2; } else { /* Unreachable */ - a = 1; + a = 1; } return a; } diff --git a/test/misc/flow-while-01.c b/test/misc/flow-while-01.c index 5ba4f8d9f..91d1454a9 100644 --- a/test/misc/flow-while-01.c +++ b/test/misc/flow-while-01.c @@ -47,7 +47,7 @@ static int f5(void) { while (1) { ++a; - if (a == 4) goto L; + if (a == 4) goto L; return a; } /* Reachable via L */ @@ -56,8 +56,8 @@ L: a = 2; static int f6(void) { - while (0) { - /* Unreachable but no warning */ + while (0) { + /* Unreachable but no warning */ } a = 2; return a; From 61f3e43fb676641eee0e2d4c66dc7fe62272f4a8 Mon Sep 17 00:00:00 2001 From: Kugel Fuhr <98353208+kugelfuhr@users.noreply.github.com> Date: Thu, 17 Jul 2025 20:52:33 +0200 Subject: [PATCH 5/5] Rewrote an outdated comment. --- src/cc65/stmt.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/cc65/stmt.h b/src/cc65/stmt.h index 2b91d06bf..eaaf235d5 100644 --- a/src/cc65/stmt.h +++ b/src/cc65/stmt.h @@ -47,12 +47,13 @@ ** analysis and tracks occurance of "return", "break", "goto" and "continue" ** (anything that jumps). The flags for the distinct statements are only set ** if they are always executed. So for example in case of an if/else statement, -** SF_RETURN is set only if both branches contain a "return". The SF_ANY flag -** is set of any of the statements occurred. So for an if/else statement, if -** one branch contains a "return" and the other a "continue" statement, neither -** SF_RETURN, nor SF_CONTINUE is set, but SF_ANY. +** SF_RETURN is set only if both branches contain a "return". The SF_ANY_xxx +** flags are set if any of the statements occurred. So for an if/else +** statement, if one branch contains a "return" and the other a "continue" +** statement, neither SF_RETURN, nor SF_CONTINUE is set, but SF_ANY_RETURN and +** SF_ANY_CONTINUE. ** There are some additional flags that tell if the statement parsed was -** preceeded by at least one label. These flags do not also set SF_ANY. +** preceeded by at least one label. */ enum { SF_NONE = 0x0000, @@ -138,7 +139,6 @@ static inline int SF_Label (int F) - /*****************************************************************************/ /* Code */ /*****************************************************************************/