Question regarding C code generation for a compound condition

[David Brown]

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
>