First Last Prev Next    No search results available      Search page      Enter new bug
Bug#: 15069
Product:  
Component:  
Status: RESOLVED
Resolution: FIXED
Assigned To: Not yet assigned to anyone <unassigned@gcc.gnu.org>
Host:
Reported against  
Priority:  
Severity:  
Target Milestone:  
 
 
Target:
Reporter: bruno@clisp.org
Add CC:
CC:
Remove selected CCs
Build:
URL:
Summary:
Keywords:
Known to work:
Known to fail:

Attachment Description Type Created Size Actions
diffs Draft patch to provide C++-specific signed_type routine patch 2004-05-29 00:04 1.24 KB Edit | Diff
Create a New Attachment (proposed patch, testcase, etc.) View All

Bug 15069 depends on: Show dependency tree
Show dependency graph
Bug 15069 blocks:

Additional Comments:






View Bug Activity   |   Format For Printing   |   Clone This Bug


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"
=========================================================================

------- Comment #1 From bruno@clisp.org 2004-04-22 12:52 -------
Fix:

My workaround is to use the C compiler instead of the C++ compiler.

------- Comment #2 From Andrew Pinski 2004-04-22 14:44 -------
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.

------- Comment #3 From Wolfgang Bangerth 2004-04-22 15:53 -------
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. 

------- Comment #4 From Andrew Pinski 2004-04-22 15:58 -------
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.

------- Comment #5 From bruno@clisp.org 2004-04-22 16:03 -------
> 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.  
  

------- Comment #6 From Wolfgang Bangerth 2004-04-22 16:08 -------
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. 

------- Comment #7 From Andrew Pinski 2004-04-22 16:24 -------
Yes this is a bug in fold.
with:
       int flags = (four) < 0 ? 160 : 0
with:
       int flags = ((four) >> 2u & 1) ? 160 : 0

------- Comment #8 From roger@eyesopen.com 2004-04-24 03:27 -------
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.

------- Comment #9 From Andrew Pinski 2004-05-04 05:16 -------
*** Bug 15277 has been marked as a duplicate of this bug. ***

------- Comment #10 From Andrew Pinski 2004-05-09 20:46 -------
*** Bug 15354 has been marked as a duplicate of this bug. ***

------- Comment #11 From Mark Mitchell 2004-05-28 22:01 -------
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

------- Comment #12 From Andrew Pinski 2004-05-28 22:05 -------
I think this is because STRIP_NOP looks into CONVERT_EXPR which it has done for
a long time now.

------- Comment #13 From roger@eyesopen.com 2004-05-28 22:17 -------
A middle-end fix for this was proposed back on 24th April in
http://gcc.gnu.org/ml/gcc-patches/2004-04/msg01657.html
However, RTH agreed with my analysis that this is actually a C++ issue
http://gcc.gnu.org/ml/gcc-patches/2004-04/msg01950.html

This is also intertwined with Jospeph Myers' proposals to overhaul
bit-field enumerations in the C family front-ends.

------- Comment #14 From Andrew Pinski 2004-05-28 22:19 -------
Why are we removing CONVERT_EXPR anyways, the definition of CONVERT_EXPR is
that it can generate 
code.

------- Comment #15 From roger@eyesopen.com 2004-05-28 22:33 -------
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!

------- Comment #16 From Mark Mitchell 2004-05-29 00:03 -------
Roger --

I implemented Richard's suggestion and it doesn't seem to help.

Patch attached.

-- Mark

------- Comment #17 From Mark Mitchell 2004-05-29 00:04 -------
Created an attachment (id=6423) [edit]
Draft patch to provide C++-specific signed_type routine

------- Comment #18 From CVS Commits 2004-05-31 17:01 -------
Subject: Bug 15069

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	sayle@gcc.gnu.org	2004-05-31 17:01:17

Modified files:
	gcc            : ChangeLog fold-const.c 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/g++.dg/opt: fold3.C 

Log message:
	PR middle-end/15069
	* fold-const.c (fold_single_bit_test): Only perform "(X & C) != 0"
	into "X < 0" (where C is the signbit) if X's type is a full mode.
	
	* g++.dg/opt/fold3.C: New test case.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.3804&r2=2.3805
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/fold-const.c.diff?cvsroot=gcc&r1=1.386&r2=1.387
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&r1=1.3796&r2=1.3797
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/opt/fold3.C.diff?cvsroot=gcc&r1=NONE&r2=1.1


------- Comment #19 From CVS Commits 2004-05-31 23:15 -------
Subject: Bug 15069

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-3_4-branch
Changes by:	sayle@gcc.gnu.org	2004-05-31 23:15:22

Modified files:
	gcc            : ChangeLog fold-const.c 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/g++.dg/opt: fold3.C 

Log message:
	PR middle-end/15069
	* fold-const.c (fold_single_bit_test): Only perform "(X & C) != 0"
	into "X < 0" (where C is the signbit) if X's type is a full mode.
	
	* g++.dg/opt/fold3.C: New test case.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=2.2326.2.459&r2=2.2326.2.460
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/fold-const.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.322.2.10&r2=1.322.2.11
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.3389.2.191&r2=1.3389.2.192
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/opt/fold3.C.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=NONE&r2=1.1.2.1


------- Comment #20 From Andrew Pinski 2004-05-31 23:21 -------
Fixed.

------- Comment #21 From Andrew Pinski 2004-06-02 13:18 -------
*** Bug 15776 has been marked as a duplicate of this bug. ***

First Last Prev Next    No search results available      Search page      Enter new bug