Complete unreachable checking for switch statements.

src/cc65/stmt.c

@@ -793,10 +793,13 @@ int StatementBlock (struct SwitchCtrl* Switch)
                 Unreachable = SF_Unreach (StmtFlags1);
             }
             /* If the previous statement made this one unreachable, but this
-            ** one has a label, it is not unreachable.
+            ** one has a label, it is not unreachable. If we've output a
+            ** warning before, reset the warning flag so a new warning is
+            ** output if code becomes unreachable again.
             */
             if (Unreachable && SF_Label (StmtFlags2)) {
                 Unreachable = 0;
+                Warning = 0;
             }
             /* If this statement is unreachable but not the empty statement,
             ** and we didn't give a warning before, to that now

src/cc65/stmt.h

@@ -128,6 +128,12 @@ static inline int SF_Any_Break (int F)
     return (F & SF_ANY_BREAK);
 }
 
+static inline int SF_Any_Goto (int F)
+/* Check if there was any "goto" statement */
+{
+    return (F & SF_ANY_GOTO);
+}
+
 static inline int SF_Any (int F)
 /* Return just the "any" part of the given flags */
 {

src/cc65/swstmt.c

@@ -61,23 +61,35 @@
 
 
 
-/* Some bitmapped flags for use in SwitchCtrl */
+/* Flow control data for one switch label */
-#define SC_NONE         0x0000
+typedef struct LabelCtrl LabelCtrl;
-#define SC_CASE         0x0001  /* We had a case label */
+struct LabelCtrl {
-#define SC_DEFAULT      0x0002  /* We had a default label */
+    long        Value;          /* Numeric value if any */
-#define SC_MASK_LABEL   0x0003  /* Mask for the labels */
+    int         StmtFlags;      /* Collected statement flags for this label */
-#define SC_WEIRD        0x0004  /* Flag for a weird switch that contains code
+    uint8_t     Default;        /* Is this the default label? */
-                                ** outside of any label.
+    uint8_t     Unreachable;    /* Label is unreachable */
-                                */
+    uint8_t     Warning;        /* We've output a warning for this label */
+    uint8_t     Breaks;         /* Code after this label contains a "break" */
+};
 
+/* Switch control data */
 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 */
+
+    /* Data for flow control analysis. The Labels collection will contain
+    ** allocated but the ActiveLabels will contain just pointers to data
+    ** owned by the Labels collection.
+    */
+    int         Weird;          /* Flag for a weird switch that contains gotos
+                                ** or code outside of any label.
+                                */
     int         StmtFlags;      /* Collected statement flags */
+    Collection  Labels;         /* Collection with all labels */
+    Collection  ActiveLabels;   /* Collection with currently active labels */
 };
 
 /* Pointer to current switch control struct */
@@ -86,47 +98,46 @@ static SwitchCtrl* Switch = 0;
 
 
 /*****************************************************************************/
-/*                Functions that work with struct SwitchExpr                 */
+/*                             struct LabelCtrl                              */
 /*****************************************************************************/
 
 
 
-static int SC_Label (const SwitchCtrl* S)
+static void AddLabelCtrl (SwitchCtrl* Switch, uint8_t Default, long Value,
-/* Check if we had a switch label */
+                          uint8_t Unreachable)
+/* Create a new LabelCtrl structure and add it to the current switch */
 {
-    return (S->CtrlFlags & SC_MASK_LABEL) != SC_NONE;
+    /* Allocate */
+    LabelCtrl* LC = xmalloc (sizeof (*LC));
+
+    /* Initialize */
+    LC->Value       = Value;
+    LC->StmtFlags   = SF_NONE;
+    LC->Default     = Default;
+    LC->Unreachable = Unreachable;
+    LC->Warning     = 0;
+    LC->Breaks      = 0;
+
+    /* Add it to the labels. If the label isn't unreachable, do also add it
+    ** to the active labels.
+    */
+    CollAppend (&Switch->Labels, LC);
+    if (!LC->Unreachable) {
+        CollAppend (&Switch->ActiveLabels, LC);
+    }
 }
 
 
 
-static int SC_IsWeird (const SwitchCtrl* S)
+static void FreeLabels (SwitchCtrl* Switch)
-/* Check if this switch is weird */
+/* Delete all labels in the given switch control structure */
 {
-    return (S->CtrlFlags & SC_WEIRD) != SC_NONE;
+    unsigned I;
-}
+    for (I = 0; I < CollCount (&Switch->Labels); ++I) {
-
+        xfree (CollAtUnchecked (&Switch->Labels, I));
-
+    }
-
+    CollDeleteAll (&Switch->Labels);
-static void SC_SetCase (SwitchCtrl* S)
+    CollDeleteAll (&Switch->ActiveLabels);
-/* 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;
 }
 
 
@@ -190,8 +201,10 @@ int SwitchStatement (void)
     SwitchData.ExprType     = SwitchExpr.Type;
     SwitchData.Depth        = SizeOf (SwitchExpr.Type);
     SwitchData.DefaultLabel = 0;
-    SwitchData.CtrlFlags    = SC_NONE;
+    SwitchData.Weird        = 0;
     SwitchData.StmtFlags    = SF_NONE;
+    SwitchData.Labels       = EmptyCollection;
+    SwitchData.ActiveLabels = EmptyCollection;
     OldSwitch = Switch;
     Switch = &SwitchData;
 
@@ -251,17 +264,63 @@ int SwitchStatement (void)
     /* Free the case value tree */
     FreeCaseNodeColl (SwitchData.Nodes);
 
-    /* If the case statement was terminated by a closing curly
+    /* If the case statement was terminated by a closing curly brace, skip
-    ** brace, skip it now.
+    ** it now.
     */
     if (RCurlyBrace) {
         NextToken ();
     }
 
+    /* Remove "break" from the flags since it is handled completely inside the
+    ** switch statement.
+    */
+    StmtFlags = SwitchData.StmtFlags & ~(SF_ANY_BREAK | SF_BREAK);
+
+    /* If the switch was weird, we cannot really tell if the following code is
+    ** unreachable.
+    */
+    if (!SwitchData.Weird) {
+
+        /* Check the labels. If there is no default label, the code after the
+        ** switch is always reachable. If there is a default label, the code
+        ** after the switch is reachable if the default label code ends with a
+        ** break or no statement that makes following instructions unreachable.
+        ** Otherwise the code after the switch is reachable if any of the
+        ** labels contains a "break" statement.
+        */
+        StmtFlags = SF_NONE;
+        int Reachable = 0;
+        int Default = 0;
+        for (unsigned I = 0; I < CollCount (&SwitchData.Labels); ++I) {
+            const LabelCtrl* LC = CollAtUnchecked (&SwitchData.Labels, I);
+            if (LC->Default) {
+                Default = 1;
+                if (!SF_Unreach (LC->StmtFlags)) {
+                    Reachable = 1;
+                }
+            }
+            if (SF_Any_Break (LC->StmtFlags)) {
+                Reachable = 1;
+            }
+            StmtFlags |= (LC->StmtFlags & ~(SF_ANY_BREAK | SF_BREAK));
+        }
+        if (!Default) {
+            Reachable = 1;
+        }
+        if (Reachable) {
+            StmtFlags &= ~SF_MASK_UNREACH;
+        }
+    }
+
+    /* Now free the labels */
+    FreeLabels (&SwitchData);
+    DoneCollection (&SwitchData.Labels);
+    DoneCollection (&SwitchData.ActiveLabels);
+
     /* 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;
+    return StmtFlags;
 }
 
 
@@ -284,7 +343,6 @@ void CaseLabel (void)
         const Type* CaseT       = CaseExpr.Type;
         long        CaseVal     = CaseExpr.IVal;
         int         OutOfRange  = 0;
-        const char* DiagMsg     = 0;
 
         CaseExpr.Type = IntPromotion (Switch->ExprType);
         LimitExprValue (&CaseExpr, 1);
@@ -305,18 +363,21 @@ void CaseLabel (void)
         /* Check the range of the expression */
         if (IsSignSigned (CaseExpr.Type)) {
             if (CaseExpr.IVal < GetIntegerTypeMin (Switch->ExprType)) {
-                DiagMsg = "Case value (%ld) out of range for switch condition type";
                 OutOfRange = 1;
+                Warning ("Case value (%ld) out of range for switch condition type",
+                         CaseExpr.IVal);
             } else if (IsSignSigned (Switch->ExprType) ?
                        CaseExpr.IVal > (long)GetIntegerTypeMax (Switch->ExprType) :
                        SizeOf (CaseExpr.Type) > SizeOf (Switch->ExprType) &&
                        (unsigned long)CaseExpr.IVal > GetIntegerTypeMax (Switch->ExprType)) {
-                DiagMsg = "Case value (%ld) out of range for switch condition type";
                 OutOfRange = 1;
+                Warning ("Case value (%ld) out of range for switch condition type",
+                         CaseExpr.IVal);
             }
         } else if ((unsigned long)CaseExpr.IVal > GetIntegerTypeMax (Switch->ExprType)) {
-            DiagMsg = "Case value (%lu) out of range for switch condition type";
             OutOfRange = 1;
+            Warning ("Case value (%lu) out of range for switch condition type",
+                     (unsigned long) CaseExpr.IVal);
         }
 
         if (OutOfRange == 0) {
@@ -325,12 +386,10 @@ void CaseLabel (void)
 
             /* Define this label */
             g_defcodelabel (CodeLabel);
-        } else {
-            Warning (DiagMsg, CaseExpr.IVal);
         }
 
-        /* Remember that we're in a case label section now */
+        /* Add a label control structure for this label */
-        SC_SetCase (Switch);
+        AddLabelCtrl (Switch, 0, CaseExpr.IVal, OutOfRange);
 
     } else {
 
@@ -361,8 +420,8 @@ void DefaultLabel (void)
             Switch->DefaultLabel = GetLocalLabel ();
             g_defcodelabel (Switch->DefaultLabel);
 
-            /* Remember that we're in the default label section now */
+            /* Add a label control structure for this label */
-            SC_SetDefault (Switch);
+            AddLabelCtrl (Switch, 1, 0, 0);
 
         } else {
             /* We had the default label already */
@@ -387,20 +446,57 @@ void SwitchBodyStatement (struct SwitchCtrl* S, LineInfo* LI, int StmtFlags)
 ** a switch passing the flags for special statements.
 */
 {
+    unsigned I;
+
     /* 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) {
+    if (CollCount (&S->Labels) == 0) {
         /* This is a statement that preceedes any switch labels. If the
         ** switch is not already marked as weird and the current statement
         ** has no label, output a warning about unreachable code.
         */
-        if (!SC_IsWeird (S)) {
+        if (!S->Weird) {
             if (!SF_Label (StmtFlags)) {
                 LIUnreachableCodeWarning (LI);
             }
-            SC_MakeWeird (S);
+            S->Weird = 1;
+        }
+    }
+
+    /* If the new statement contains a "goto", mark the switch as "weird" */
+    if (SF_Any_Goto (StmtFlags)) {
+        S->Weird = 1;
+    }
+
+    /* If the switch is marked as weird, no further action */
+    if (S->Weird) {
+        return;
+    }
+
+    /* Handle all currently active labels. Walk from the end since we're
+    ** deleting stuff from the array.
+    */
+    I = CollCount (&S->ActiveLabels);
+    while (I > 0) {
+        LabelCtrl* LC = CollAtUnchecked (&S->ActiveLabels, --I);
+
+        /* Collect the flags for this label */
+        LC->StmtFlags = SF_Any (LC->StmtFlags) | (StmtFlags & ~SF_EMPTY);
+
+        /* If the new statement contains a "break", mark the label code as
+        ** "breaking".
+        */
+        if (SF_Any_Break (StmtFlags)) {
+            LC->Breaks = 1;
+        }
+
+        /* If the new statement makes the following ones unreachable, remove
+        ** the label from the active ones.
+        */
+        if (SF_Unreach (StmtFlags)) {
+            CollDelete (&S->ActiveLabels, I);
         }
     }
 

test/misc/flow-switch-01.c

@@ -44,7 +44,7 @@ static int f3(void)
         case 2:         return a+1;
         default:        return a+2;
     }
-    /* Unreachable but no warning */
+    /* Unreachable */
     return a;
 }
 
@@ -58,11 +58,71 @@ static int f4(void)
             default:        return a+2;
         } while (1);
     }
-    /* Unreachable but no warning */
+    /* Unreachable */
     return a;
 }
 
+static int f5(void)
+{
+    do {
+        switch (a) {
+            case 1:     return a;
+            case 2:     ++a; continue;
+            default:    return 1;
+        }
+    } while (0);
+    /* Unreachable */
+    return 2;
+}
+
+static int f6(void)
+{
+    do {
+L:      switch (a) {
+            case 1:     return a;
+            case 2:     ++a; goto L;
+            default:    return 1;
+        }
+    } while (0);
+    /* Unreachable but no warning because of "goto" */
+    return 2;
+}
+
+static int f7(void)
+{
+    do {
+        switch (a) {
+            /* Unreachable */
+            a = 3;
+            case 1:     return a;
+            case 2:     ++a; continue;
+            default:    return 1;
+        }
+    } while (0);
+    /* Unreachable but no warning because of weird switch */
+    return 2;
+}
+
+static void duff(char* to, char* from, unsigned count)
+{
+    unsigned n = (count+7)/8;
+    switch (count%8) {
+        case 0: do {    *to = *from++;
+        case 7:         *to = *from++;
+        case 6:         *to = *from++;
+        case 5:         *to = *from++;
+        case 4:         *to = *from++;
+        case 3:         *to = *from++;
+        case 2:         *to = *from++;
+        case 1:         *to = *from++;
+        } while (--n>0);
+    }
+}
+
 int main(void)
 {
-    return f1() + f2() + f3() + f4();
+    char x[11];
+    char y[11];
+    duff(x, y, 11);
+    return f1() + f2() + f3() + f4() + f5() + f6() + f7();
 }

test/misc/flow-switch-01.ref

@@ -1 +1,5 @@
 flow-switch-01.c:7: Warning: Unreachable code
+flow-switch-01.c:48: Warning: Unreachable code
+flow-switch-01.c:62: Warning: Unreachable code
+flow-switch-01.c:75: Warning: Unreachable code
+flow-switch-01.c:96: Warning: Unreachable code

test/misc/goto.ref

@@ -5,8 +5,11 @@ 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:100: Warning: Unreachable code
 goto.c:117: Warning: Variable 'a' is defined but never used
+goto.c:120: Warning: Unreachable code
 goto.c:137: Warning: Variable 'a' is defined but never used
+goto.c:140: Warning: Unreachable code
 goto.c:159: Warning: Goto at line 23 to label l8 jumps into a block with initialization of an object that has automatic storage duration
 goto.c:159: Warning: Goto at line 44 to label l8 jumps into a block with initialization of an object that has automatic storage duration
 goto.c:159: Warning: Goto at line 65 to label l8 jumps into a block with initialization of an object that has automatic storage duration
@@ -42,6 +45,7 @@ goto.c:221: Warning: Goto at line 109 to label lb jumps into a block with initia
 goto.c:221: Warning: Goto at line 129 to label lb jumps into a block with initialization of an object that has automatic storage duration
 goto.c:221: Warning: Goto at line 149 to label lb jumps into a block with initialization of an object that has automatic storage duration
 goto.c:221: Warning: Goto at line 170 to label lb jumps into a block with initialization of an object that has automatic storage duration
+goto.c:223: Warning: Unreachable code
 goto.c:231: Warning: Goto at line 231 to label la jumps into a block with initialization of an object that has automatic storage duration
 goto.c:241: Warning: Goto at line 27 to label lc jumps into a block with initialization of an object that has automatic storage duration
 goto.c:241: Warning: Goto at line 48 to label lc jumps into a block with initialization of an object that has automatic storage duration
@@ -50,6 +54,7 @@ goto.c:241: Warning: Goto at line 90 to label lc jumps into a block with initial
 goto.c:241: Warning: Goto at line 110 to label lc jumps into a block with initialization of an object that has automatic storage duration
 goto.c:241: Warning: Goto at line 130 to label lc jumps into a block with initialization of an object that has automatic storage duration
 goto.c:241: Warning: Goto at line 150 to label lc jumps into a block with initialization of an object that has automatic storage duration
+goto.c:243: Warning: Unreachable code
 goto.c:250: Warning: Goto at line 250 to label l9 jumps into a block with initialization of an object that has automatic storage duration
 goto.c:251: Warning: Goto at line 251 to label la jumps into a block with initialization of an object that has automatic storage duration
 goto.c:252: Warning: Goto at line 252 to label lb jumps into a block with initialization of an object that has automatic storage duration
@@ -123,6 +128,7 @@ goto.c:325: Warning: Goto at line 217 to label lg jumps into a block with initia
 goto.c:325: Warning: Goto at line 237 to label lg jumps into a block with initialization of an object that has automatic storage duration
 goto.c:325: Warning: Goto at line 257 to label lg jumps into a block with initialization of an object that has automatic storage duration
 goto.c:325: Warning: Goto at line 279 to label lg jumps into a block with initialization of an object that has automatic storage duration
+goto.c:327: Warning: Unreachable code
 goto.c:333: Warning: Goto at line 333 to label l8 jumps into a block with initialization of an object that has automatic storage duration
 goto.c:334: Warning: Goto at line 334 to label l9 jumps into a block with initialization of an object that has automatic storage duration
 goto.c:335: Warning: Goto at line 335 to label la jumps into a block with initialization of an object that has automatic storage duration
@@ -141,6 +147,7 @@ goto.c:345: Warning: Goto at line 197 to label lh jumps into a block with initia
 goto.c:345: Warning: Goto at line 218 to label lh jumps into a block with initialization of an object that has automatic storage duration
 goto.c:345: Warning: Goto at line 238 to label lh jumps into a block with initialization of an object that has automatic storage duration
 goto.c:345: Warning: Goto at line 258 to label lh jumps into a block with initialization of an object that has automatic storage duration
+goto.c:347: Warning: Unreachable code
 goto.c:353: Warning: Goto at line 353 to label l8 jumps into a block with initialization of an object that has automatic storage duration
 goto.c:354: Warning: Goto at line 354 to label l9 jumps into a block with initialization of an object that has automatic storage duration
 goto.c:355: Warning: Goto at line 355 to label la jumps into a block with initialization of an object that has automatic storage duration
@@ -149,6 +156,7 @@ goto.c:357: Warning: Goto at line 357 to label lc jumps into a block with initia
 goto.c:359: Warning: Goto at line 359 to label le jumps into a block with initialization of an object that has automatic storage duration
 goto.c:360: Warning: Goto at line 360 to label lf jumps into a block with initialization of an object that has automatic storage duration
 goto.c:361: Warning: Goto at line 361 to label lg jumps into a block with initialization of an object that has automatic storage duration
+goto.c:367: Warning: Unreachable code
 goto.c:373: Warning: Goto at line 373 to label l8 jumps into a block with initialization of an object that has automatic storage duration
 goto.c:374: Warning: Goto at line 374 to label l9 jumps into a block with initialization of an object that has automatic storage duration
 goto.c:375: Warning: Goto at line 375 to label la jumps into a block with initialization of an object that has automatic storage duration