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
