Bug 15069 - [3.4 regression] a bit test on a variable of enum type is miscompiled
|
Bug#:
15069
|
Product: gcc
|
Version: 3.4.0
|
|
Host: i686-pc-linux-gnu
|
Target: i686-pc-linux-gnu
|
Build: i686-pc-linux-gnu
|
|
Status: RESOLVED
|
Severity: critical
|
Priority: P3
|
|
Resolution: FIXED
|
Assigned To: unassigned@gcc.gnu.org
|
Reported By: bruno@clisp.org
|
|
Component: c++
|
Target Milestone: 3.4.1
|
|
Summary: [3.4 regression] a bit test on a variable of enum type is miscompiled
|
|
Keywords: wrong-code
|
|
Opened: 2004-04-22 12:52
|
|
Description:
|
Last confirmed: 2004-04-22 16:08
|
Opened: 2004-04-22 12:52
|
When testing a bit of a variable with enum type, the test for the
highest bit (bit 2 in my case) is compiled to a bit test for bit 31.
But bit 31 is always zero for all possible values of the enum type.
Environment:
System: Linux honolulu.ilog.fr 2.4.21-0.13mdk #1 Fri Mar 14 15:08:06 EST 2003 i686 unknown unknown GNU/Linux
Architecture: i686
host: i686-pc-linux-gnu
build: i686-pc-linux-gnu
target: i686-pc-linux-gnu
configured with: ../gcc-3.4.0/configure --prefix=/home/haible/gnu/arch/linuxgcc34 --enable-shared --enable-threads --enable-__cxa_atexit --enable-languages=c,c++,f77,java,objc --enable-nls
How-To-Repeat:
============================== bug.cc ===================================
extern "C" void abort (void);
void* allocate_stream (unsigned char strmflags)
{
if (strmflags == 0)
abort ();
return 0;
}
typedef enum {
DIRECTION_PROBE = 0,
DIRECTION_INPUT = 1,
DIRECTION_INPUT_IMMUTABLE = 3,
DIRECTION_OUTPUT = 4,
DIRECTION_IO = 5
} direction_t;
void* make_unbuffered_stream (direction_t direction, int* eltype)
{
unsigned char flags =
(((direction) & (1L<<(0))) ? ((1L<<(4)) | (1L<<(6))) : 0)
| (((direction) & (1L<<(2))) ? ((1L<<(5)) | (1L<<(7))) : 0)
| (((direction) & (1L<<(1))) ? (1L<<(1)) : 0);
if (*eltype == 0)
flags &= ((1L<<(6)) | (1L<<(7))) | (1L<<(1));
else
flags &= ((1L<<(4)) | (1L<<(5))) | (1L<<(1));
return allocate_stream (flags);
}
int main ()
{
int x = 1;
make_unbuffered_stream (DIRECTION_OUTPUT, &x);
return 0;
}
=========================================================================
$ g++ bug.cc
$ ./a.out
Aborted
Here is the assembler code:
======================== g++ -O -S bug.cc ===============================
.file "bug.cc"
.text
.align 2
.globl _Z15allocate_streamh
.type _Z15allocate_streamh, @function
_Z15allocate_streamh:
.LFB2:
pushl %ebp
.LCFI0:
movl %esp, %ebp
.LCFI1:
subl $8, %esp
.LCFI2:
cmpb $0, 8(%ebp)
jne .L2
call abort
.L2:
movl $0, %eax
leave
ret
.LFE2:
.size _Z15allocate_streamh, .-_Z15allocate_streamh
.align 2
.globl _Z22make_unbuffered_stream11direction_tPi
.type _Z22make_unbuffered_stream11direction_tPi, @function
_Z22make_unbuffered_stream11direction_tPi:
.LFB3:
pushl %ebp
.LCFI3:
movl %esp, %ebp
.LCFI4:
subl $8, %esp
.LCFI5:
movl 8(%ebp), %edx
movb %dl, %al
andb $1, %al ; test bit 0 - correct
cmpb $1, %al
sbbl %eax, %eax
notl %eax
andl $80, %eax
testl %edx, %edx ; test bit 31 instead of bit 2 - wrong
jns .L6
orl $160, %eax
.L6:
testb $2, %dl ; test bit 1 - correct
je .L7
orl $2, %eax
.L7:
movb %al, %dl
movl 12(%ebp), %eax
cmpl $0, (%eax)
jne .L8
andb $-62, %dl
jmp .L9
.L8:
andb $50, %dl
.L9:
movzbl %dl, %eax
movl %eax, (%esp)
call _Z15allocate_streamh
leave
ret
.LFE3:
.size _Z22make_unbuffered_stream11direction_tPi, .-_Z22make_unbuffered_stream11direction_tPi
.align 2
.globl main
.type main, @function
main:
.LFB4:
pushl %ebp
.LCFI6:
movl %esp, %ebp
.LCFI7:
subl $24, %esp
.LCFI8:
andl $-16, %esp
subl $16, %esp
movl $1, -4(%ebp)
leal -4(%ebp), %eax
movl %eax, 4(%esp)
movl $4, (%esp)
call _Z22make_unbuffered_stream11direction_tPi
movl $0, %eax
leave
ret
.LFE4:
.size main, .-main
.section .eh_frame,"a",@progbits
.Lframe1:
.long .LECIE1-.LSCIE1
.LSCIE1:
.long 0x0
.byte 0x1
.string "zP"
.uleb128 0x1
.sleb128 -4
.byte 0x8
.uleb128 0x5
.byte 0x0
.long __gxx_personality_v0
.byte 0xc
.uleb128 0x4
.uleb128 0x4
.byte 0x88
.uleb128 0x1
.align 4
.LECIE1:
.LSFDE1:
.long .LEFDE1-.LASFDE1
.LASFDE1:
.long .LASFDE1-.Lframe1
.long .LFB2
.long .LFE2-.LFB2
.uleb128 0x0
.byte 0x4
.long .LCFI0-.LFB2
.byte 0xe
.uleb128 0x8
.byte 0x85
.uleb128 0x2
.byte 0x4
.long .LCFI1-.LCFI0
.byte 0xd
.uleb128 0x5
.align 4
.LEFDE1:
.LSFDE3:
.long .LEFDE3-.LASFDE3
.LASFDE3:
.long .LASFDE3-.Lframe1
.long .LFB3
.long .LFE3-.LFB3
.uleb128 0x0
.byte 0x4
.long .LCFI3-.LFB3
.byte 0xe
.uleb128 0x8
.byte 0x85
.uleb128 0x2
.byte 0x4
.long .LCFI4-.LCFI3
.byte 0xd
.uleb128 0x5
.align 4
.LEFDE3:
.LSFDE5:
.long .LEFDE5-.LASFDE5
.LASFDE5:
.long .LASFDE5-.Lframe1
.long .LFB4
.long .LFE4-.LFB4
.uleb128 0x0
.byte 0x4
.long .LCFI6-.LFB4
.byte 0xe
.uleb128 0x8
.byte 0x85
.uleb128 0x2
.byte 0x4
.long .LCFI7-.LCFI6
.byte 0xd
.uleb128 0x5
.align 4
.LEFDE5:
.section .note.GNU-stack,"",@progbits
.ident "GCC: (GNU) 3.4.0"
=========================================================================
Fix:
My workaround is to use the C compiler instead of the C++ compiler.
IIRC the rules for enums in C++ is different than C. Also sizes outside of the
enums are undefined for
C++ which makes this invalid and correct behavior, IIRC.
To be precise, values outside the bit-range of the enum are undefined.
I.e. in this case, values of type direction_t must be in the range
0...7 including end points.
Yes I am correct in that you are invoking undefined behavior as direction_t can
only have values
between 0 and 5 because C++ rules are different than C. In C enum types have
the same size of the
underlining type but C++ is different in that respect.
> IIRC the rules for enums in C++ is different than C.
> Also sizes outside of the enums are undefined for C++ which makes this invalid and correct
behavior, IIRC.
Look at the code. Nowhere is a value outside the range 0..5 used. The argument that is being
passed is not only one in the range bmin..bmax (speak of [dcl.enum] paragraph 6). It is also
one of the values in the enumator-list.
The expression being miscompiled is: (((direction) & (1L<<(2))). [expr.bit.and] paragraph 1
says that enumeration operands are allowed for binary '&', and that the usual arithmetic
conversions are performed. The value (long)(direction) is 4, as expected, as I can see by
adding a printf statement. Therefore the value of (((direction) & (1L<<(2))) should be nonzero
in the case where direction = DIRECTION_OUTPUT.
Btw, why did you change this as reported against g++ 2.95? I reported it against g++ 3.4.0
after verifying that it behaves differently than g++ 3.2.2. It is a regression.
This is clearly a serious bug. Here's something smaller:
-----------
extern "C" void abort (void);
typedef enum {
FOUR = 4,
FIVE = 5
} direction_t;
int main ()
{
direction_t four = FOUR;
int flags = (four & 4L) ? (32L | 128L) : 0;
flags &= 32L;
if (flags == 0)
abort ();
}
-----------------
It is obvious that flag should be 32 at the end, but gcc3.4/3.5 aborts:
g/x> /home/bangerth/bin/gcc-3.5-pre/bin/c++ x.cc -W -Wall ; ./a.out
Aborted
g/x> /home/bangerth/bin/gcc-3.4-pre/bin/c++ x.cc -W -Wall ; ./a.out
Aborted
g/x> /home/bangerth/bin/gcc-3.3.4-pre/bin/c++ x.cc -W -Wall ; ./a.out
g/x> /home/bangerth/bin/gcc-3.2.3/bin/c++ x.cc -W -Wall ; ./a.out
g/x> /home/bangerth/bin/gcc-2.95.3/bin/c++ x.cc -W -Wall ; ./a.out
This should absolutely be fixed for 3.4.1 if possible!
As a sidenote: the bug goes away when one uses
const direction_t four = FOUR;
instead of the non-const version. Maybe that gives a hint as to what
is going wrong...
W.
Yes this is a bug in fold.
with:
int flags = (four) < 0 ? 160 : 0
with:
int flags = ((four) >> 2u & 1) ? 160 : 0
The issue is that fold believes that the enumerated type, which is a signed
quantity that's three bits wide, is sign extended to fill the 32 bit integer.
Testing the most significant bit of the unextended type, direction_t, which is
DIRECTION_OUTPUT, should then be equivalent to testing whether the sign-extended
value is negative.
Clearly the C++ front-end and the middle-end are miscomunicating about how these
enums are to be stored, but it isn't immediately clear which one is at fault.
Should this enum type be unsigned, or have a type that's one bit wider in order
to potentially represent negative values. Clearly, a signed type thats only
three bits wide can't represent the value 4.
*** Bug 15277 has been marked as a duplicate of this bug. ***
*** Bug 15354 has been marked as a duplicate of this bug. ***
Roger --
The front end is marking the enumeration type as unsigned.
I think that the problem is that fold is simplifying
(NOP_EXPR long (CONVERT_EXPR int enum_expr))
to:
(NOP_EXPR long enum_expr)
which is incorrect; that CONVERT_EXPR needs to be preserved, given that the enum
type is unsigned.
Would you please look into that?
-- Mark
I think this is because STRIP_NOP looks into CONVERT_EXPR which it has done for
a long time now.
Why are we removing CONVERT_EXPR anyways, the definition of CONVERT_EXPR is
that it can generate
code.
The definition of CONVERT_EXPR is that it *may* generate code to perform
the conversion. It is ultimately the middle-end that decides whether
code needs to be generated or not. However, this is a red-herring, if
you look at my patch to fold_single_bit_test, you'll notice that the
cause of the failure is unrelated to conversion operations and affects
code without NOP_EXPRs or CONVERT_EXPRs.
It's actually in the code that optimizes "((unsigned char)C & 128) != 0"
into "(char)C < 0"! When C is a bit-field enumeration, creating a signed
type, returns a type of a different width. Doh!
Roger --
I implemented Richard's suggestion and it doesn't seem to help.
Patch attached.
-- Mark
*** Bug 15776 has been marked as a duplicate of this bug. ***