[Bug objc/103639] New: [REGRESSION] GCC 11.2 (or even earlier) breaks switch case with break in fast enumeration loop

[Bug objc/103639] New: [REGRESSION] GCC 11.2 (or even earlier) breaks switch case with break in fast enumeration loop

[js-gcc at webkeks dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103639

            Bug ID: 103639
           Summary: [REGRESSION] GCC 11.2 (or even earlier) breaks switch
                    case with break in fast enumeration loop
           Product: gcc
           Version: 11.2.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: objc
          Assignee: unassigned at gcc dot gnu.org
          Reporter: js-gcc at webkeks dot org
  Target Milestone: ---

The following code is miscompiled by GCC 11.2.1. I don't know when this
started, but this is a regression and used to work just fine in older version:

for (id object in someCollection) {
  switch (someVar) {
  case 0:
     puts("0");
     break;
  }
  puts("this should print but doesn't");
}

The break breaks out of the for loop instead of the switch case.

Changing the code to this makes it work:

for (id object in someCollection) {
  if (someVar == 0) {
     puts("0");
     break;
  }
  puts("this should print and does");
}

This worked just fine with older GCC versions and also works fine with all
versions of Clang.

[Bug objc/103639] [REGRESSION] GCC 11.2 (or even earlier) breaks switch case with break in fast enumeration loop

[js-gcc at webkeks dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103639

js-gcc at webkeks dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |js-gcc at webkeks dot org

--- Comment #1 from js-gcc at webkeks dot org ---
Can confirm the same happens with 11.1.0. 10.2 seems to be fine.

[Bug objc/103639] [REGRESSION] GCC 11.2 (or even earlier) breaks switch case with break in fast enumeration loop

[js-gcc at webkeks dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103639

--- Comment #2 from js-gcc at webkeks dot org ---
Oh, forgot to clarify in the example:

Assume more than 1 object in the collection and someVar to be 0.

[Bug objc/103639] [REGRESSION] GCC 11.2 (or even earlier) breaks switch case with break in fast enumeration loop

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103639

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2021-12-09
             Status|UNCONFIRMED                 |WAITING

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Do you have a full testcase?

[Bug objc/103639] [REGRESSION] GCC 11.2 (or even earlier) breaks switch case with break in fast enumeration loop

[js-gcc at webkeks dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103639

js-gcc at webkeks dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|wrong-code                  |

--- Comment #4 from js-gcc at webkeks dot org ---
Sure:

#import <ObjFW/ObjFW.h>

int
main()
{
        OFArray *array = [OFArray arrayWithObjects: @"a", @"b", nil];
        int someVar = 0;
        for (id object in array) {
                switch (someVar) {
                case 0:
                        OFLog(@"%@", object);
                        break;
                }
                OFLog(@"foo");
        }

        return 0;
}

Sorry, but something free-standing isn't possible as you need a collection
object. The same code should work with GNUstep by just replacing OF with NS.

[Bug objc/103639] [REGRESSION] GCC 11.2 (or even earlier) breaks switch case with break in fast enumeration loop

[egallager at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103639

Eric Gallager <egallager at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |egallager at gcc dot gnu.org,
                   |                            |iains at gcc dot gnu.org,
                   |                            |mikestump at comcast dot net

--- Comment #5 from Eric Gallager <egallager at gcc dot gnu.org> ---
cc-ing objc maintainers

[Bug objc/103639] [REGRESSION] GCC 11.2 (or even earlier) breaks switch case with break in fast enumeration loop

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103639

Iain Sandoe <iains at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |NEW

--- Comment #6 from Iain Sandoe <iains at gcc dot gnu.org> ---
confirmed on x86-64-darwin18 - the gimple is wrong out of the FE, so it seems a
FE or gimplification bug.

[Bug objc/103639] [11/12 Regression] switch case with break in fast enumeration loop generates wrong code

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103639

--- Comment #7 from Iain Sandoe <iains at gcc dot gnu.org> ---
Created attachment 51965
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51965&action=edit
preprocessed (using testsuite sub-set of GNUStep headers)


This reproduces the issue when compiled with GCC-10 (OK) and GCC-11.x/master
(bad) with the Foundation framework on Darwin.

[Bug objc/103639] [11/12 Regression] switch case with break in fast enumeration loop generates wrong code

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103639

--- Comment #8 from Iain Sandoe <iains at gcc dot gnu.org> ---
Here's a version using the testsuite object etc. it does not need Foundation
(it needs CoreFoundation on Darwin for NSString, but only libobjc on Linux)

the issue repeats equally with gnu-runtime on Linux.

gcc pr103639.m -I path/to/gcc-sources/gcc/testsuite/objc-obj-c++-shared -o t
-lobjc

=====

#import "TestsuiteObject.m"
#ifndef __NEXT_RUNTIME__
#include <objc/NXConstStr.h>
#else
#include "nsconstantstring-class.h"
#endif

extern int printf (const char *, ...);
#include <stdlib.h>

/* A mini-array implementation that can be used to test fast
    enumeration.  You create the array with some objects; you can
    mutate the array, and you can fast-enumerate it.
 */
@interface MyArray : TestsuiteObject
{
  unsigned int length;
  id *objects;
  unsigned long mutated;
}
- (id) initWithLength: (unsigned int)l  objects: (id *)o;
- (void) mutate;
- (unsigned long)countByEnumeratingWithState: (struct
__objcFastEnumerationState *)state
                                     objects:(id *)stackbuf 
                                       count:(unsigned long)len;
@end

@implementation MyArray : TestsuiteObject
- (id) initWithLength: (unsigned int)l
              objects: (id *)o
{
  length = l;
  objects = o;
  mutated = 0;
  return self;
}
- (void) mutate
{
  mutated = 1;
}
- (unsigned long)countByEnumeratingWithState: (struct
__objcFastEnumerationState*)state 
                                     objects: (id*)stackbuf
                                       count: (unsigned long)len
{
  unsigned long i, batch_size;

  /* We keep how many objects we served in the state->state counter.  So the
next batch
     will contain up to length - state->state objects.  */
  batch_size = length - state->state;

  /* Make obvious adjustments.  */
  if (batch_size < 0)
    batch_size = 0;

  if (batch_size > len)
    batch_size = len;

  /* Copy the objects.  */
  for (i = 0; i < batch_size; i++)
    stackbuf[i] = objects[i];

  state->state += batch_size;
  state->itemsPtr = stackbuf;
  state->mutationsPtr = &mutated;

  return batch_size;
}
@end

int
main()
{
  id *objects = malloc (sizeof (id) * 2);
  objects[0] = @"a";
  objects[1] = @"b";

  MyArray *array = [[MyArray alloc] initWithLength: 2 objects: objects];

// NSArray *array = [NSArray arrayWithObjects: @"a", @"b", nil];
  int someVar = 0;
  for (id object in array) {
    switch (someVar) {
      case 0:
        printf ("case 0: %s\n", [object cString]);
        break;
    }
    printf ("foo\n");
  }

  return 0;
}

[Bug objc/103639] [11/12 Regression] switch case with break in fast enumeration loop generates wrong code

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103639

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |11.3
                 CC|                            |pinskia at gcc dot gnu.org

[Bug objc/103639] [11/12 Regression] switch case with break in fast enumeration loop generates wrong code

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103639

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org,
                   |                            |sandra at gcc dot gnu.org

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Most likely caused by r11-3302-g3696a50beeb73f4ded8a584e76ee16f0bde109b9

[Bug objc/103639] [11/12 Regression] switch case with break in fast enumeration loop generates wrong code

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103639

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 52083
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52083&action=edit
gcc12-pr103639.patch

Untested fix.  I'll defer the testcase for the testsuite to somebody who speaks
ObjC.

[Bug objc/103639] [11/12 Regression] switch case with break in fast enumeration loop generates wrong code

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103639

--- Comment #11 from Iain Sandoe <iains at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #10)
> Created attachment 52083 [details]
> gcc12-pr103639.patch
> 
> Untested fix.  I'll defer the testcase for the testsuite to somebody who
> speaks ObjC.

thanks. I'll try this on my next build cycle (the code in comment #8 should be
usable as a basis for a suitable test - but I'll check that too).

[Bug objc/103639] [11/12 Regression] switch case with break in fast enumeration loop generates wrong code

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103639

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Likely, but with something that will easily detect miscompilation at runtime
(checking dg-output is too ugly) and I don't have much experience with objc.dg/
dg- directives etc.

[Bug objc/103639] [11/12 Regression] switch case with break in fast enumeration loop generates wrong code

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103639

--- Comment #13 from Iain Sandoe <iains at gcc dot gnu.org> ---
Created attachment 52090
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52090&action=edit
patch with test case

this is what I'm going to test (it passes for NeXT and GNU runtimes on
x86_64-darwin18, but needs wider testing) -- it should work also on Linux.

[Bug objc/103639] [11/12 Regression] switch case with break in fast enumeration loop generates wrong code

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103639

--- Comment #14 from Iain Sandoe <iains at gcc dot gnu.org> ---
I suppose

+    check += 1;
+    printf ("foo\n");
+  }
+
+  if (check != 2)


would be a more picky test.

[Bug objc/103639] [11/12 Regression] switch case with break in fast enumeration loop generates wrong code

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103639

--- Comment #15 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Yeah, and probably no need for the printf calls...

[Bug objc/103639] [11/12 Regression] switch case with break in fast enumeration loop generates wrong code

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103639

--- Comment #16 from Iain Sandoe <iains at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #15)
> Yeah, and probably no need for the printf calls...

indeed.

works for me on x86_64-darwin18 and powerpc-darwin9 (m32, m64, NeXT and GNU
runtimes)
I can cover more cases later in the week - but if it DTRT on Linux too I'd say
LGTM.

[Bug objc/103639] [11/12 Regression] switch case with break in fast enumeration loop generates wrong code

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103639

--- Comment #17 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:222dbebefbbc07f78e51d82ba605988ef57e5fc9

commit r12-6162-g222dbebefbbc07f78e51d82ba605988ef57e5fc9
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Sat Jan 1 06:29:36 2022 +0100

    objc: Fix handling of break stmt inside of switch inside of ObjC foreach
[PR103639]

    The r11-3302-g3696a50beeb73f changes broke the following ObjC testcase.
    in_statement is either 0 (not in a looping statement), various IN_* flags
    for various kinds of looping statements (or OpenMP structured blocks) or
    those flags ored with IN_SWITCH_STMT when a switch appears inside of those
    contexts.  This is because break binds to switch in that last case, but
    continue binds to the looping construct in that case.
    The c_finish_bc_stmt function performs diagnostics on incorrect
    break/continue uses and then checks if in_statement & IN_OBJC_FOREACH
    and in that case jumps to the label provided by the caller, otherwise
    emits a BREAK_STMT or CONTINUE_STMT.  This is incorrect if we have
    ObjC foreach with switch nested in it and break inside of that,
    in_statement in that case is IN_OBJC_FOREACH | IN_SWITCH_STMT and
    is_break is true.  We want to handle it like other breaks inside of
    switch, i.e. emit a BREAK_STMT.

    The following patch fixes that.

    2022-01-01  Jakub Jelinek  <jakub@redhat.com>

            PR objc/103639
            * c-typeck.c (c_finish_bc_stmt): For break inside of switch inside
of
            ObjC foreach, emit normal BREAK_STMT rather than goto to label.

    2022-01-01  Iain Sandoe  <iain@sandoe.co.uk>

            PR objc/103639
            * objc.dg/pr103639.m: New test.

[Bug objc/103639] [11 Regression] switch case with break in fast enumeration loop generates wrong code

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103639

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2
            Summary|[11/12 Regression] switch   |[11 Regression] switch case
                   |case with break in fast     |with break in fast
                   |enumeration loop generates  |enumeration loop generates
                   |wrong code                  |wrong code
      Known to work|                            |12.0

[Bug objc/103639] [11 Regression] switch case with break in fast enumeration loop generates wrong code

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103639

--- Comment #18 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:27cfe1068239cdafb727de36ec7d9ae19ff5c141

commit r11-9497-g27cfe1068239cdafb727de36ec7d9ae19ff5c141
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Sat Jan 1 06:29:36 2022 +0100

    objc: Fix handling of break stmt inside of switch inside of ObjC foreach
[PR103639]

    The r11-3302-g3696a50beeb73f changes broke the following ObjC testcase.
    in_statement is either 0 (not in a looping statement), various IN_* flags
    for various kinds of looping statements (or OpenMP structured blocks) or
    those flags ored with IN_SWITCH_STMT when a switch appears inside of those
    contexts.  This is because break binds to switch in that last case, but
    continue binds to the looping construct in that case.
    The c_finish_bc_stmt function performs diagnostics on incorrect
    break/continue uses and then checks if in_statement & IN_OBJC_FOREACH
    and in that case jumps to the label provided by the caller, otherwise
    emits a BREAK_STMT or CONTINUE_STMT.  This is incorrect if we have
    ObjC foreach with switch nested in it and break inside of that,
    in_statement in that case is IN_OBJC_FOREACH | IN_SWITCH_STMT and
    is_break is true.  We want to handle it like other breaks inside of
    switch, i.e. emit a BREAK_STMT.

    The following patch fixes that.

    2022-01-01  Jakub Jelinek  <jakub@redhat.com>

            PR objc/103639
            * c-typeck.c (c_finish_bc_stmt): For break inside of switch inside
of
            ObjC foreach, emit normal BREAK_STMT rather than goto to label.

    2022-01-01  Iain Sandoe  <iain@sandoe.co.uk>

            PR objc/103639
            * objc.dg/pr103639.m: New test.

    (cherry picked from commit 222dbebefbbc07f78e51d82ba605988ef57e5fc9)

[Bug objc/103639] [11 Regression] switch case with break in fast enumeration loop generates wrong code

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103639

--- Comment #19 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for 11.3 too.

[Bug objc/103639] [11 Regression] switch case with break in fast enumeration loop generates wrong code

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103639

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #20 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
.