Question regarding C code generation for a compound condition

[Albert ARIBAUD <albert dot aribaud at motorola dot com>]

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