public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Jan Beulich" <jbeulich@novell.com>
To: "Jakub Jelinek" <jakub@redhat.com>
Cc: "Andi Kleen" <ak@suse.de>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] work around gcc4 issue with -Os in Dwarf2 stack unwind code
Date: Tue, 28 Nov 2006 14:48:15 +0000	[thread overview]
Message-ID: <456C5A3F.76E4.0078.0@novell.com> (raw)
In-Reply-To: <20061128143214.GD6570@devserv.devel.redhat.com>

>"mis-compiling" and "work around" are wrong words, the code had undefined
>behavior (there is no sequence point between evaluation of ptr and
>get_uleb128(&ptr, end) and ptr is modified twice, so the compiler can
>evaluate it e.g. as:
>temp = ptr;
>temp = temp + get_uleb128(&ptr, end);
>ptr = temp;
>or
>temp = get_uleb128(&ptr, end);
>ptr += temp;
>While gcc has some warnings for sequence point semantics violations
>(-Wsequence-point), this can't be one of the cases at least until IPA moves
>much further, because get_uleb128 might very well not modify the variable
>and at that point the code would be ok).

I disagree - the standard says there's a sequence point at a function
call after evaluating all function arguments. To me this means that any
(parts of an) expression the function call is contained in must be
evaluated after the function call. Otherwise it would be illegal to e.g.
modify a variable in both operands of && or ||.
I consider my opinion supported by the fact that the problem doesn't
happen with any non-Os optimization, where it is obvious that it would
in all cases be beneficial to load the variable's value into a register early.
(And, btw., after the change the code generated is significantly smaller -
hinting at questionable effects of -Os.)

Jan

  reply	other threads:[~2006-11-28 14:46 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-11-28 14:12 [PATCH] work around gcc4 issue with -Os in Dwarf2 stack unwind code Jan Beulich
2006-11-28 14:32 ` Jakub Jelinek
2006-11-28 14:48   ` Jan Beulich [this message]
2006-11-28 14:59     ` Jakub Jelinek
2006-11-28 16:23 ` Andi Kleen

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=456C5A3F.76E4.0078.0@novell.com \
    --to=jbeulich@novell.com \
    --cc=ak@suse.de \
    --cc=jakub@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    /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