mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: Ian Lance Taylor <iant@google.com>
Cc: Arnaud Lacombe <lacombar@gmail.com>,
	gcc-help@gcc.gnu.org, stufever@gmail.com,
	linux-kernel@vger.kernel.org,
	Wang Shaoyan <wangshaoyan.pt@taobao.com>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Ingo Molnar <mingo@redhat.com>
Subject: Re: [PATCH] TRACING: Fix a copmile warning
Date: Mon, 25 Jul 2011 21:10:28 -0400	[thread overview]
Message-ID: <1311642628.3526.58.camel@gandalf.stny.rr.com> (raw)
In-Reply-To: <mcrfwltoov5.fsf@coign.corp.google.com>

On Mon, 2011-07-25 at 17:41 -0700, Ian Lance Taylor wrote:
> Arnaud Lacombe <lacombar@gmail.com> writes:

> > It seems gcc transforms the conditional from:
> >
> > if (a != NULL && b != NULL) ...
> >
> > to
> >
> > if (b != NULL && a != NULL) ...
> >
> > In which case the warning is fully valid. I'm not sure what's the C
> > standard guarantee in term of conditional test order. gcc 4.7.0 has
> > the same behavior.
> 
> Not quite.  C guarantees that && is executed in order.  In this case gcc
> is generating
> 
>   a = e();
>   if (a != NULL)
>     b = f();
>   if (a != NULL & b != NULL)
>     g();
> 
> Note the change from && to & in the last conditional.  This
> transformation is safe, in that it does not change the meaning of the
> program.  However, it does cause a read of an uninitialized memory
> location, and this is causing a later gcc pass to generate a false
> positive warning.
> 

Looking at the assembly again, and not knowing what gcc is doing
internally, it does seem to be:

	if (a != NULL)
		b = f();
	if (b != NULL && a != NULL)
		g();

But if the first conditional fails, then the second will never pass
regardless of what b is. In which case, it is the same as:

	if (a != NULL)
		b = f();
	if (a != NULL && b != NULL)
		g();

And it doesn't change the meaning of the code.

> Please consider filing a bug report about this false positive.  Thanks.

I agree that this is just a warning bug.


On a tangent:

Compiling with -O2 (which gives no warning) (x86_64) produces:

0000000000000000 <fn>:
   0:	48 83 ec 08          	sub    $0x8,%rsp
   4:	e8 00 00 00 00       	callq  9 <fn+0x9>
			5: R_X86_64_PC32	e-0x4
   9:	48 85 c0             	test   %rax,%rax
   c:	74 1a                	je     28 <fn+0x28>
   e:	e8 00 00 00 00       	callq  13 <fn+0x13>
			f: R_X86_64_PC32	f-0x4
  13:	48 85 c0             	test   %rax,%rax
  16:	74 10                	je     28 <fn+0x28>
  18:	48 83 c4 08          	add    $0x8,%rsp
  1c:	e9 00 00 00 00       	jmpq   21 <fn+0x21>
			1d: R_X86_64_PC32	g-0x4
  21:	0f 1f 80 00 00 00 00 	nopl   0x0(%rax)
  28:	48 83 c4 08          	add    $0x8,%rsp
  2c:	c3                   	retq   

and compiling with -Os:

0000000000000000 <fn>:
   0:	55                   	push   %rbp
   1:	53                   	push   %rbx
   2:	51                   	push   %rcx
   3:	e8 00 00 00 00       	callq  8 <fn+0x8>
			4: R_X86_64_PC32	e-0x4
   8:	48 85 c0             	test   %rax,%rax
   b:	48 89 c3             	mov    %rax,%rbx
   e:	74 08                	je     18 <fn+0x18>
  10:	e8 00 00 00 00       	callq  15 <fn+0x15>
			11: R_X86_64_PC32	f-0x4
  15:	48 89 c5             	mov    %rax,%rbp
  18:	48 85 ed             	test   %rbp,%rbp
  1b:	74 0d                	je     2a <fn+0x2a>
  1d:	48 85 db             	test   %rbx,%rbx
  20:	74 08                	je     2a <fn+0x2a>
  22:	5a                   	pop    %rdx
  23:	5b                   	pop    %rbx
  24:	5d                   	pop    %rbp
  25:	e9 00 00 00 00       	jmpq   2a <fn+0x2a>
			26: R_X86_64_PC32	g-0x4
  2a:	58                   	pop    %rax
  2b:	5b                   	pop    %rbx
  2c:	5d                   	pop    %rbp
  2d:	c3                   	retq   

Which is 1 byte more than -O2. I would think that -Os would be smaller.

-- Steve



  parent reply	other threads:[~2011-07-26  1:10 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-18  9:40 stufever
2011-07-25 18:32 ` Steven Rostedt
2011-07-25 19:43   ` Arnaud Lacombe
2011-07-25 20:19     ` Steven Rostedt
2011-07-25 20:28       ` Arnaud Lacombe
2011-07-25 22:38       ` Arnaud Lacombe
2011-07-25 23:49         ` Steven Rostedt
2011-07-25 23:52           ` Arnaud Lacombe
2011-07-26  0:14             ` Steven Rostedt
2011-07-25 23:50         ` Arnaud Lacombe
2011-07-25 23:58           ` Arnaud Lacombe
2011-07-26  0:08             ` Steven Rostedt
2011-07-26  0:35           ` Steven Rostedt
2011-07-26  0:44             ` Ian Lance Taylor
2011-07-26  0:41           ` Ian Lance Taylor
2011-07-26  1:08             ` Arnaud Lacombe
2011-07-26  1:12               ` Steven Rostedt
2011-07-26  1:19                 ` Arnaud Lacombe
2011-07-26 20:43                   ` Jeff Law
2011-07-26  1:10             ` Steven Rostedt [this message]
2011-07-26  5:55               ` Ian Lance Taylor
2011-07-26 12:00   ` Paulo Marques
2011-07-26 13:18     ` Jesper Juhl
2011-07-26 13:32       ` Paulo Marques
2011-07-26 13:55         ` Steven Rostedt
  -- strict thread matches above, loose matches on Subject: below --
2011-07-18  9:35 stufever

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1311642628.3526.58.camel@gandalf.stny.rr.com \
    --to=rostedt@goodmis.org \
    --cc=fweisbec@gmail.com \
    --cc=gcc-help@gcc.gnu.org \
    --cc=iant@google.com \
    --cc=lacombar@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=stufever@gmail.com \
    --cc=wangshaoyan.pt@taobao.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®