Skip to content

9fdd7fc479 breaks build using g++ as compiler #20108

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jkeenan opened this issue Aug 17, 2022 · 4 comments · Fixed by #20109
Closed

9fdd7fc479 breaks build using g++ as compiler #20108

jkeenan opened this issue Aug 17, 2022 · 4 comments · Fixed by #20109

Comments

@jkeenan
Copy link
Contributor

jkeenan commented Aug 17, 2022

A recent commit to blead has broken the build when using g++ as the C-compiler.

I first noticed this breakage in recent smoke-test reports such as this one.

Bisecting with this invocation:

perl Porting/bisect.pl \
-Dcc=g++ \
--test-build \
--start cf1717027b95225cb27f33b15b84ff798c8bacaf \
--end 40666a761f261de4ec02a7fd4e770a48957df7c7

... points to this commit:

9fdd7fc4796d89d16dceea42f2af91e4fde296ed is the first bad commit
commit 9fdd7fc4796d89d16dceea42f2af91e4fde296ed
Author: Richard Leach <[email protected]>
Date:   Fri Jul 8 14:52:19 2022 +0000
Commit:     Richard Leach <[email protected]>
CommitDate: Wed Aug 17 11:19:10 2022 +0100

 
    Implement OP_PADSV_STORE - combined sassign/padsv OP

    This commit introduces a new OP to replace simple cases
    of OP_SASSIGN and OP_PADSV.

@richardleach, can you take a look?

@neilb, can you keep an eye on this, as it will affect the monthly dev release scheduled for Saturday Aug 20? Thanks.

@jkeenan
Copy link
Contributor Author

jkeenan commented Aug 17, 2022

Compilation on FreeBSD-12 using g++ version 10.3.0 ends here:

g++ -c -DPERL_CORE -DHAS_FPSETMASK -DHAS_FLOATINGPOINT_H -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_FORTIFY_SOURCE=2 -O2 -Wall -Werror=pointer-arith -Werror=vla -Wextra -Wno-long-long -Wwrite-strings peep.c
peep.c: In function 'void Perl_rpeep(OP*)':
peep.c:3897:14: error: jump to case label
 3897 |         case OP_AASSIGN: {
      |              ^~~~~~~~~~
peep.c:3844:17: note:   crosses initialization of 'OP* lval'
 3844 |             OP* lval = cBINOPx(o)->op_last;
      |                 ^~~~
peep.c:3843:17: note:   crosses initialization of 'OP* rhs'
 3843 |             OP* rhs = cBINOPx(o)->op_first;
      |                 ^~~
peep.c:4004:14: error: jump to case label
 4004 |         case OP_REF:
      |              ^~~~~~
peep.c:3844:17: note:   crosses initialization of 'OP* lval'
 3844 |             OP* lval = cBINOPx(o)->op_last;
      |                 ^~~~
peep.c:3843:17: note:   crosses initialization of 'OP* rhs'
 3843 |             OP* rhs = cBINOPx(o)->op_first;
      |                 ^~~
peep.c:4005:14: error: jump to case label
 4005 |         case OP_BLESSED:
      |              ^~~~~~~~~~
peep.c:3844:17: note:   crosses initialization of 'OP* lval'
 3844 |             OP* lval = cBINOPx(o)->op_last;
      |                 ^~~~
peep.c:3843:17: note:   crosses initialization of 'OP* rhs'
 3843 |             OP* rhs = cBINOPx(o)->op_first;
      |                 ^~~
peep.c:4013:14: error: jump to case label
 4013 |         case OP_LENGTH:
      |              ^~~~~~~~~
peep.c:3844:17: note:   crosses initialization of 'OP* lval'
 3844 |             OP* lval = cBINOPx(o)->op_last;
      |                 ^~~~
peep.c:3843:17: note:   crosses initialization of 'OP* rhs'
 3843 |             OP* rhs = cBINOPx(o)->op_first;
      |                 ^~~
peep.c:4022:14: error: jump to case label
 4022 |         case OP_POS:
      |              ^~~~~~
peep.c:3844:17: note:   crosses initialization of 'OP* lval'
 3844 |             OP* lval = cBINOPx(o)->op_last;
      |                 ^~~~
peep.c:3843:17: note:   crosses initialization of 'OP* rhs'
 3843 |             OP* rhs = cBINOPx(o)->op_first;
      |                 ^~~
peep.c:4028:14: error: jump to case label
 4028 |         case OP_CUSTOM: {
      |              ^~~~~~~~~
peep.c:3844:17: note:   crosses initialization of 'OP* lval'
 3844 |             OP* lval = cBINOPx(o)->op_last;
      |                 ^~~~
peep.c:3843:17: note:   crosses initialization of 'OP* rhs'
 3843 |             OP* rhs = cBINOPx(o)->op_first;
      |                 ^~~
*** Error code 1

Stop.
make: stopped in /usr/home/jkeenan/gitwork/perl

@jkeenan
Copy link
Contributor Author

jkeenan commented Aug 17, 2022

And, no surprise, clang++ builds are breaking as well. Here's one on OpenBSD.

@jkeenan
Copy link
Contributor Author

jkeenan commented Aug 17, 2022

A recent commit to blead has broken the build when using g++ as the C-compiler.

I first noticed this breakage in recent smoke-test reports such as this one.

Correction for copy/paste error noted by Bram:
http://perl.develop-help.com/raw/?id=284149

bram-perl pushed a commit to bram-perl/perl5 that referenced this issue Aug 17, 2022
Code in 9fdd7fc broke the g++ builds,

Basically what the code looked like:

         switch (o->op_type) {
         ...
         case OP_SASSIGN:
             ...
             OP* rhs = cBINOPx(o)->op_first;
             OP* lval = cBINOPx(o)->op_last;
             ...
             break;

         case OP_AASSIGN: {

g++ does not allow this and errors with:
	peep.c:3897:14: error: jump to case label
	 3897 |         case OP_AASSIGN: {
	      |              ^~~~~~~~~~
	peep.c:3844:17: note:   crosses initialization of 'OP* lval'
	 3844 |             OP* lval = cBINOPx(o)->op_last;
	      |                 ^~~~
	peep.c:3843:17: note:   crosses initialization of 'OP* rhs'
	 3843 |             OP* rhs = cBINOPx(o)->op_first;
	      |                 ^~~

This happens because `rhs` and `lval` are not scoped in the case statement
so it could fall through to the next case.

The solution is to scope them which this commit now does by adding a
separate scope for `OP_SASSIGN` (similar to `OP_AASSIGN).

Fixes Perl#20108
bram-perl pushed a commit to bram-perl/perl5 that referenced this issue Aug 17, 2022
Code in 9fdd7fc broke the g++ builds,

Basically after the commit the code looked like:

         switch (o->op_type) {
         ...
         case OP_SASSIGN:
             ...
             OP* rhs = cBINOPx(o)->op_first;
             OP* lval = cBINOPx(o)->op_last;
             ...
             break;

         case OP_AASSIGN: {

g++ does not allow this and errors with:

	peep.c:3897:14: error: jump to case label
	 3897 |         case OP_AASSIGN: {
	      |              ^~~~~~~~~~
	peep.c:3844:17: note:   crosses initialization of 'OP* lval'
	 3844 |             OP* lval = cBINOPx(o)->op_last;
	      |                 ^~~~
	peep.c:3843:17: note:   crosses initialization of 'OP* rhs'
	 3843 |             OP* rhs = cBINOPx(o)->op_first;
	      |                 ^~~

This happens because `rhs` and `lval` are not scoped in the case statement
so it could fall through to the next case.

The solution is to scope them which this commit now does by adding a
separate scope for `OP_SASSIGN` (similar to `OP_AASSIGN).

Fixes Perl#20108
bram-perl pushed a commit to bram-perl/perl5 that referenced this issue Aug 17, 2022
Code in 9fdd7fc broke the g++ builds,

Basically after the commit the code looked like:

         switch (o->op_type) {
         ...
         case OP_SASSIGN:
             ...
             OP* rhs = cBINOPx(o)->op_first;
             OP* lval = cBINOPx(o)->op_last;
             ...
             break;

         case OP_AASSIGN: {

g++ does not allow this and errors with:

	peep.c:3897:14: error: jump to case label
	 3897 |         case OP_AASSIGN: {
	      |              ^~~~~~~~~~
	peep.c:3844:17: note:   crosses initialization of 'OP* lval'
	 3844 |             OP* lval = cBINOPx(o)->op_last;
	      |                 ^~~~
	peep.c:3843:17: note:   crosses initialization of 'OP* rhs'
	 3843 |             OP* rhs = cBINOPx(o)->op_first;
	      |                 ^~~

This happens because `rhs` and `lval` are not scoped in the case statement
so it could fall through to the next case.

The solution is to scope them which this commit now does by adding a
separate scope for `OP_SASSIGN` (similar to `OP_AASSIGN`).

Fixes Perl#20108
khwilliamson pushed a commit that referenced this issue Aug 17, 2022
Code in 9fdd7fc broke the g++ builds,

Basically after the commit the code looked like:

         switch (o->op_type) {
         ...
         case OP_SASSIGN:
             ...
             OP* rhs = cBINOPx(o)->op_first;
             OP* lval = cBINOPx(o)->op_last;
             ...
             break;

         case OP_AASSIGN: {

g++ does not allow this and errors with:

	peep.c:3897:14: error: jump to case label
	 3897 |         case OP_AASSIGN: {
	      |              ^~~~~~~~~~
	peep.c:3844:17: note:   crosses initialization of 'OP* lval'
	 3844 |             OP* lval = cBINOPx(o)->op_last;
	      |                 ^~~~
	peep.c:3843:17: note:   crosses initialization of 'OP* rhs'
	 3843 |             OP* rhs = cBINOPx(o)->op_first;
	      |                 ^~~

This happens because `rhs` and `lval` are not scoped in the case statement
so it could fall through to the next case.

The solution is to scope them which this commit now does by adding a
separate scope for `OP_SASSIGN` (similar to `OP_AASSIGN`).

Fixes #20108
@jkeenan
Copy link
Contributor Author

jkeenan commented Aug 18, 2022

Repair confirmed with a build on FreeBSD with this invocation:

$ sh ./Configure -des -Dusedevel -Duseithreads -Dcc=g++ && make test_harness

scottchiefbaker pushed a commit to scottchiefbaker/perl5 that referenced this issue Nov 3, 2022
Code in 9fdd7fc broke the g++ builds,

Basically after the commit the code looked like:

         switch (o->op_type) {
         ...
         case OP_SASSIGN:
             ...
             OP* rhs = cBINOPx(o)->op_first;
             OP* lval = cBINOPx(o)->op_last;
             ...
             break;

         case OP_AASSIGN: {

g++ does not allow this and errors with:

	peep.c:3897:14: error: jump to case label
	 3897 |         case OP_AASSIGN: {
	      |              ^~~~~~~~~~
	peep.c:3844:17: note:   crosses initialization of 'OP* lval'
	 3844 |             OP* lval = cBINOPx(o)->op_last;
	      |                 ^~~~
	peep.c:3843:17: note:   crosses initialization of 'OP* rhs'
	 3843 |             OP* rhs = cBINOPx(o)->op_first;
	      |                 ^~~

This happens because `rhs` and `lval` are not scoped in the case statement
so it could fall through to the next case.

The solution is to scope them which this commit now does by adding a
separate scope for `OP_SASSIGN` (similar to `OP_AASSIGN`).

Fixes Perl#20108
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant