From: Richard.Earnshaw@arm.com (Richard Earnshaw)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH-WIP 01/13] xen/arm: use r12 to pass the hypercall number to the hypervisor
Date: Thu, 8 Mar 2012 18:47:28 +0000 [thread overview]
Message-ID: <4F58FEC0.8080706@arm.com> (raw)
In-Reply-To: <alpine.LFD.2.02.1203081041340.24151@xanadu.home>
On 08/03/12 17:21, Nicolas Pitre wrote:
> On Thu, 8 Mar 2012, Richard Earnshaw wrote:
>
>> On 02/03/12 21:15, Nicolas Pitre wrote:
>>> So, to me, the gcc documentation is perfectly clear on this topic.
>>> there really _is_ a guarantee that those asm marked variables will be in
>>> the expected registers on entry to the inline asm, given that the
>>> variable is _also_ listed as an operand to the asm statement. But only
>>> in that case.
>>>
>>> It is true that gcc may reorder other function calls or other code
>>> around the inline asm and then intervening code can clobber any
>>> registers. Then it is up to gcc to preserve the variable's content
>>> elsewhere when its register is used for other purposes, and restore it
>>> when some inline asm statement is referring to it.
>>>
>>> And if gcc does not do this then it is buggy. Version 3.4.0 of gcc was
>>> buggy. No other gcc versions in the last 7 years had such a problem or
>>> the __asmeq macro in the kernel would have told us.
>>>
>>>> Or, to summarise another way, there is no way to control which register
>>>> is used to pass something to an inline asm in general (often we get away
>>>> with this, and there are a lot of inline asms in the kernel that assume
>>>> it works, but the more you inline the more likely you are to get nasty
>>>> surprises).
>>>
>>> This statement is therefore unfounded and wrong. Please direct the
>>> tools guy who mislead you to the above gcc documentation.
>>>
>>
>> The problem is not really about re-ordering functions but about implicit
>> functions that come from the source code; for example
>>
>> int foo (int a, int b)
>> {
>> register int x __asm__("r0") = 33;
>>
>> register int c __asm__("r1") = a / b; /* Ooops, clobbers r0 with
>> division function call. */
>>
>> asm ("svc 0" : : "r" (x));
>> }
>>
>> There's nothing in the specification to say what happens if there's a
>> statement in the code that causes an implicit clobber of your assembly
>> register.
>
> I'm sure gcc is full of implicit behaviors that are not mentioned in
> the specification. But as long as the specification is respected, then
> there is no need to mention any unobservable side effects from a program
> flow point of view, right?
>
> Why wouldn't gcc be able to respect the documented feature by
> preventing live variable from being clobbered and reloading them in
> the specified register at the inline asm entry point, just like it does
> for function calls?
>
> Here's an example code that shows that, unfortunately, gcc is still
> broken with regards to the documented behavior:
>
> extern int bar(int);
> int foo(int y)
> {
> register int x __asm__("r1") = 33;
> y += bar(x);
> asm ("@ x should be live in %0 here" : "+r" (x) : "r" (y));
> y += bar(x);
> asm ("@ x should be live in %0 here" : "+r" (x) : "r" (y));
> return x;
> }
>
> Result is:
>
> foo:
> stmfd sp!, {r4, lr}
> mov r4, r0
> mov r0, #33
> bl bar
> add r4, r0, r4
> @ x should be live in r1 here
> mov r0, r1
> bl bar
> add r0, r0, r4
> @ x should be live in r1 here
> mov r0, r1
> ldmfd sp!, {r4, lr}
> bx lr
>
> To me this is clearly a bug if gcc is not able to meet the documented
> expectation. And the documented expectation is not at all unreasonable.
>
No, in this case it is presumed that /you/ know that calling bar() will
modify x. Thus the code is either well defined (if you know what is in
r1 after each call to bar), or undefined (if you can't say anything
about r1 after each call).
As I said, the problem really comes from compiler generated calls which
are not mentioned explicitly in the source code.
R.
--
Richard Earnshaw Email: Richard.Earnshaw at arm.com
Engineering Manager Phone: +44 1223 400569 (Direct + VoiceMail)
OpenSource Tools Switchboard: +44 1223 400400
ARM Ltd Fax: +44 1223 400410
110 Fulbourn Rd Web: http://www.arm.com/
Cambridge, UK. CB1 9NJ
-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
next prev parent reply other threads:[~2012-03-08 18:47 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-02-23 17:47 [PATCH-WIP 00/13] xen/arm: receive Xen events and initialize xenbus Stefano Stabellini
2012-02-23 17:48 ` [PATCH-WIP 01/13] xen/arm: use r12 to pass the hypercall number to the hypervisor Stefano Stabellini
2012-02-27 16:27 ` Ian Campbell
2012-02-27 18:03 ` Dave Martin
2012-02-27 19:33 ` Ian Campbell
2012-02-28 10:20 ` Dave Martin
2012-02-28 10:48 ` Ian Campbell
2012-02-28 12:28 ` Stefano Stabellini
2012-02-29 9:34 ` Dave Martin
2012-02-29 9:56 ` Ian Campbell
2012-02-29 11:47 ` Dave Martin
2012-02-29 12:58 ` Dave Martin
2012-02-29 14:44 ` Ian Campbell
2012-03-01 9:35 ` Dave Martin
2012-03-01 10:12 ` Russell King - ARM Linux
2012-03-02 21:19 ` Nicolas Pitre
2012-02-29 14:52 ` Stefano Stabellini
2012-03-01 9:51 ` Dave Martin
2012-03-01 10:10 ` Russell King - ARM Linux
2012-03-01 10:27 ` Dave Martin
2012-03-01 10:35 ` Russell King - ARM Linux
2012-03-01 12:12 ` Stefano Stabellini
2012-03-02 21:15 ` Nicolas Pitre
2012-03-08 9:58 ` Richard Earnshaw
2012-03-08 12:17 ` Dave Martin
2012-03-08 17:21 ` Nicolas Pitre
2012-03-08 18:47 ` Richard Earnshaw [this message]
2012-03-09 15:58 ` Dave Martin
2012-03-09 16:20 ` Nicolas Pitre
2012-03-09 17:38 ` Richard Earnshaw
2012-02-27 21:05 ` Peter Maydell
2012-02-28 10:12 ` Ian Campbell
2012-02-27 17:53 ` Dave Martin
2012-02-27 19:48 ` Ian Campbell
2012-02-28 9:46 ` Dave Martin
2012-02-28 10:07 ` Ian Campbell
2012-02-28 12:21 ` Stefano Stabellini
2012-02-23 17:48 ` [PATCH-WIP 02/13] xen/arm: introduce privcmp, physdev_op and memory_op hypercalls Stefano Stabellini
2012-02-23 17:48 ` [PATCH-WIP 03/13] xen/arm: mmu.h and page.h related definitions Stefano Stabellini
2012-02-23 17:48 ` [PATCH-WIP 04/13] xen/arm: sync_bitops Stefano Stabellini
2012-02-23 17:48 ` [PATCH-WIP 05/13] xen/arm: empty implementation of grant_table arch specific functions Stefano Stabellini
2012-02-23 17:48 ` [PATCH-WIP 06/13] xen/arm: missing includes Stefano Stabellini
2012-02-23 17:48 ` [PATCH-WIP 07/13] xen/arm: receive xen events on arm Stefano Stabellini
2012-02-24 11:12 ` David Vrabel
2012-02-24 12:23 ` Stefano Stabellini
2012-02-23 17:48 ` [PATCH-WIP 08/13] xen/arm: fix arm xen guest handle definitions Stefano Stabellini
2012-02-23 17:48 ` [PATCH-WIP 09/13] xen/arm: shared_info and start_info Stefano Stabellini
2012-02-23 17:48 ` [PATCH-WIP 10/13] xen/arm: empty implementation of xen_remap_domain_mfn_range Stefano Stabellini
2012-02-23 17:48 ` [PATCH-WIP 11/13] xen/arm: Introduce xen_pfn_t for pfn and mfn types Stefano Stabellini
2012-02-23 17:48 ` [PATCH-WIP 12/13] xen/arm: compile and run xenbus Stefano Stabellini
2012-02-23 17:48 ` [PATCH-WIP 13/13] xen/arm: compile grant-table features events and xenbus, do not compile pci Stefano Stabellini
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=4F58FEC0.8080706@arm.com \
--to=richard.earnshaw@arm.com \
--cc=linux-arm-kernel@lists.infradead.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;
as well as URLs for NNTP newsgroup(s).