This is the mail archive of the gcc-help@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Question regarding C code generation for a compound condition


On 07/06/2011 15:38, Albert ARIBAUD wrote:
Hi all,

First of all, apologies if this is not the right list, or the correct
way, to ask -- just point me in the right direction.

We have encountered a weird case of gcc C code generation when
compiling an if statement with a compound condition under -Os -O2; we
suspect that the generated code is wrong, and we would like to make
sure before we report a compiler bug...


The options "-Os -O2" are contradictory. "-Os" enables most of the same optimisations as "-O2" except those that are expected to increase code size, and it also makes the compiler pick smaller code on some occasions.



The key issue is documented here :


<http://gcc.gnu.org/onlinedocs/gcc-4.3.5/gcc/Optimize-Options.html#index-fdelete_002dnull_002dpointer_002dchecks-658>

Disable this optimisation and you'll get the "correct" result for your "incorrect" C code.

mvh.,

David



The code is as follows:

-------------------------------------------
#include<stdio.h>

struct fields {
   char        a;
   char        b;
};


struct fields *g;


void test_function(void)
{

   if ( (g->a==0) || (g==NULL) )
   {
     g->b = 0;
     return;
   }

g->b = 10;

return;

}
-------------------------------------------

Granted, the way the condition is written is not valid, as the
evaluation will be done left to right, causing a dereference of g
before testing it against NULL -- the right order should be "(g==NULL)
|| (g->a==0)", with short-circuit ensuring g->a is only evaluated if g
!= NULL.

However, even with this admittedly bad order, we would expect both
sides of the '||' expression to be generated; however, only the left
side is, both with an ARM or and x86 backend as an objdump extract
shows:

ARM, with gcc version 4.4.1 (Sourcery G++ Lite 2010q1-188)

void test_function(void)
{

   if ( (g->a==0) || (g==NULL) )
    0:	e59f0014 	ldr	r0, [pc, #20]	; 1c<test_function+0x1c>
    4:	e5903000 	ldr	r3, [r0]
    8:	e5d32000 	ldrb	r2, [r3]
    c:	e3520000 	cmp	r2, #0
   {
     g->b = 0;
     return;
   }

   g->b = 10;
   10:	13a0200a 	movne	r2, #10
   14:	e5c32001 	strb	r2, [r3, #1]
   18:	e12fff1e 	bx	lr
   1c:	00000000 	.word	0x00000000

(r3 holds the value of g, and is never tested against 0)

x86 with gcc version 4.1.2 20080704 (Red Hat 4.1.2-46)

Disassembly of section .text:

00000000<test_function>:

void test_function(void)
{

   if ( (g->a==0) || (g==NULL) )
    0:	8b 15 00 00 00 00    	mov    0x0,%edx
    6:	55                   	push   %ebp
    7:	89 e5                	mov    %esp,%ebp
//  if ( (g==NULL) || (g->a==0) )
   {
     g->b = 0;
     return;
   }

   g->b = 10;
    9:	80 3a 01             	cmpb   $0x1,(%edx)
    c:	19 c0                	sbb    %eax,%eax
    e:	f7 d0                	not    %eax
   10:	83 e0 0a             	and    $0xa,%eax
   13:	88 42 01             	mov    %al,0x1(%edx)

return;

}
   16:	5d                   	pop    %ebp
   17:	c3                   	ret

edx holds the value of g, and is never tested against 0. Also, the
same code is generated, albeit with different register naming
conventions, using gcc version 4.5.2 (Ubuntu/Linaro 4.5.2-8ubuntu4)

Note: with the code in the 'right' order, code generation is indeed
performed for both branches, with obvious short-circuit for both ARM
and x86:

    0:	e59f3028 	ldr	r3, [pc, #40]	; 30<test_function+0x30>
    4:	e5933000 	ldr	r3, [r3]
    8:	e3530000 	cmp	r3, #0
    c:	0a000004 	beq	24<test_function+0x24>
   10:	e5d32000 	ldrb	r2, [r3]
   14:	e3520000 	cmp	r2, #0


0: a1 00 00 00 00 mov 0x0,%eax ... 8: 85 c0 test %eax,%eax a: 74 0b je 17<test_function+0x17> c: 80 38 00 cmpb $0x0,(%eax) f: 74 06 je 17<test_function+0x17>

So my question (finally) is: even though the order of the '||'
branches in the C code above is truly bad, shouldn't the compiler have
generated code for both branches of the '||', thus should we raise a
PR for this?

Thanks in advance for your advice!

Regards,
--
Albert ARIBAUD - Motorola Mobility France SAS




Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]