ICEs in print_operand() of the rs6000 backend with invalid input assembly

[Arseny Solokha]

Hi,

I've found recently that rs6000 and powerpcspe backends can easily trip over
various gcc_unreachable()'s and gcc_assert()'s in their respective copies of
print_operand() when provided with some invalid assembly (i.e. assembly written
for other architectures). For example, when feeding
gcc/testsuite/gcc.target/i386/indirect-thunk-register-4.c to the
powerpc-targeted compiler:

% powerpc-e300c3-linux-gnu-gcc-9.0.0-alpha20181118 -c
gcc/testsuite/gcc.target/i386/indirect-thunk-register-4.c
during RTL pass: final
gcc/testsuite/gcc.target/i386/indirect-thunk-register-4.c: In function 'foo':
gcc/testsuite/gcc.target/i386/indirect-thunk-register-4.c:10:1: internal
compiler error: in print_operand, at config/rs6000/rs6000.c:20992
10 | }
| ^
0x6da7e3 print_operand(_IO_FILE*, rtx_def*, int)
<…>/gcc-9-20181118/gcc/config/rs6000/rs6000.c:20992
0x9eb84f output_operand(rtx_def*, int)
<…>/gcc-9-20181118/gcc/final.c:4042
0x9ec4d6 output_asm_insn(char const*, rtx_def**)
<…>/gcc-9-20181118/gcc/final.c:3935
0x9ee6a6 final_scan_insn_1
<…>/gcc-9-20181118/gcc/final.c:2712
0x9ee998 final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*)
<…>/gcc-9-20181118/gcc/final.c:3149
0x9eecae final_1
<…>/gcc-9-20181118/gcc/final.c:2019
0x9ef838 rest_of_handle_final
<…>/gcc-9-20181118/gcc/final.c:4649
0x9ef838 execute
<…>/gcc-9-20181118/gcc/final.c:4723

I have a hunch that this kind of invalid input should have been rejected way
earlier, before getting to the printing out the resulting assembly, and that
i386 backend actually does exactly that. Still, I'm not convinced that the
current behavior is really unintended. Is it worthwhile at all to report ICEs of
this kind?

Thanks.

[Segher Boessenkool]

Hi Arseny,

Post by Arseny Solokha
I've found recently that rs6000 and powerpcspe backends can easily trip over
various gcc_unreachable()'s and gcc_assert()'s in their respective copies of
print_operand() when provided with some invalid assembly (i.e. assembly written
for other architectures).

Yup. They should use output_operand_lossage instead.

Post by Arseny Solokha
For example, when feeding
gcc/testsuite/gcc.target/i386/indirect-thunk-register-4.c to the
% powerpc-e300c3-linux-gnu-gcc-9.0.0-alpha20181118 -c
gcc/testsuite/gcc.target/i386/indirect-thunk-register-4.c

(This is a V output modifier, which expects a trap code, which you cannot
give in inline asm at all).

Post by Arseny Solokha
I have a hunch that this kind of invalid input should have been rejected way
earlier, before getting to the printing out the resulting assembly, and that
i386 backend actually does exactly that. Still, I'm not convinced that the
current behavior is really unintended. Is it worthwhile at all to report ICEs of
this kind?

Please report them, yes! Bonus points if you can find some way to test
this in the testsuite (preferably for all targets), testing many kinds
of input args (reg, imm, memory) and all output modifiers.

Thanks,


Segher


p.s. Patch for the %V thing:

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index ac0e210..9b29f0b 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -20971,7 +20971,7 @@ print_operand (FILE *file, rtx x, int code)
fputs ("lge", file); /* 5 */
break;
default:
- gcc_unreachable ();
+ output_operand_lossage ("invalid %%V value");
}
break;

[Arseny Solokha]

Post by Segher Boessenkool
Hi Arseny,

Post by Arseny Solokha
I've found recently that rs6000 and powerpcspe backends can easily trip over
various gcc_unreachable()'s and gcc_assert()'s in their respective copies of
print_operand() when provided with some invalid assembly (i.e. assembly written
for other architectures).

Yup. They should use output_operand_lossage instead.

Isn't it something that should be mentioned in the Internals manual?

Post by Segher Boessenkool

Post by Arseny Solokha
I have a hunch that this kind of invalid input should have been rejected way
earlier, before getting to the printing out the resulting assembly, and that
i386 backend actually does exactly that. Still, I'm not convinced that the
current behavior is really unintended. Is it worthwhile at all to report ICEs of
this kind?

Please report them, yes!

So I've filed PR88188 with three issues I've found so far. Though they might be
of different nature.

Post by Segher Boessenkool
Bonus points if you can find some way to test
this in the testsuite (preferably for all targets), testing many kinds
of input args (reg, imm, memory) and all output modifiers.

For now I use a trivial script that simply tries to compile everything it finds,
each file with a new set of -mcpu value, optimization options and --param
options. Maybe enhancing the testsuite to facilitate some kind of fuzzing out of
the box could be a proper task for the GSoC?

[Segher Boessenkool]

Hi!

Post by Arseny Solokha

Post by Segher Boessenkool

Post by Arseny Solokha
I've found recently that rs6000 and powerpcspe backends can easily trip over
various gcc_unreachable()'s and gcc_assert()'s in their respective copies of
print_operand() when provided with some invalid assembly (i.e. assembly written
for other architectures).

Yup. They should use output_operand_lossage instead.

Isn't it something that should be mentioned in the Internals manual?

It is a general principle that the compiler should only ICE for an internal
error, so never for unexpected use input.

Most existing backends use output_operand_lossage, so everyone writing
a new backend should see it, anyway.

Post by Arseny Solokha

Post by Segher Boessenkool
Bonus points if you can find some way to test
this in the testsuite (preferably for all targets), testing many kinds
of input args (reg, imm, memory) and all output modifiers.

For now I use a trivial script that simply tries to compile everything it finds,
each file with a new set of -mcpu value, optimization options and --param
options. Maybe enhancing the testsuite to facilitate some kind of fuzzing out of
the box could be a proper task for the GSoC?

See what Jakub committed in r266515, maybe you can do something like that
for all targets and all modifier letters? Any error is correct, just not
an ICE?

https://gcc.gnu.org/viewcvs/gcc/trunk/gcc/testsuite/gcc.target/powerpc/pr88188.c?view=markup&pathrev=266515


Segher