Re: [Xen-devel] [PATCH] x86emul: correct stub invocation constraints again

[Ian Jackson]

Jan Beulich writes ("[PATCH] x86emul: correct stub invocation constraints 
again"):
> While the hypervisor side of commit cd91ab08ea ("x86emul: correct stub
> invocation constraints") was fine, the tools side triggered a bogus
> error with old gcc (4.3 and 4.4 at least). Use a slightly less
> appropriate variant instead, proven to be good enough to not
> re-introduce the original problem: Which of the addresses is actually
> used doesn't matter much as long as the compiler can't prove that the
> two pointers don't alias one another.
> 
> Reported-by: Boris Ostrovsky 
> Signed-off-by: Jan Beulich 

Release-acked-by: Ian Jackson 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86emul: correct stub invocation constraints again

[Boris Ostrovsky]

On 04/27/2017 04:41 AM, Jan Beulich wrote:
> While the hypervisor side of commit cd91ab08ea ("x86emul: correct stub
> invocation constraints") was fine, the tools side triggered a bogus
> error with old gcc (4.3 and 4.4 at least). Use a slightly less
> appropriate variant instead, proven to be good enough to not
> re-introduce the original problem: Which of the addresses is actually
> used doesn't matter much as long as the compiler can't prove that the
> two pointers don't alias one another.
>
> Reported-by: Boris Ostrovsky 
> Signed-off-by: Jan Beulich 
>
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -901,7 +901,7 @@ do{ asm volatile (
>  # define invoke_stub(pre, post, constraints...) \
>  asm volatile ( pre "\n\tcall *%[stub]\n\t" post \
> : constraints, [stub] "rm" (stub.func),  \
> - "m" (*(uint8_t(*)[MAX_INST_LEN + 1])stub.buf) )
> + "m" (*(typeof(stub.buf) *)stub.addr) )
>  #endif
>  
>  #define emulate_stub(dst, src...) do {  \

Tested-by: Boris Ostrovsky 


And to answer your earlier question --- this was tools-specific (but you
already know this now).

-boris

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86emul: correct stub invocation constraints again

[Andrew Cooper]

On 27/04/2017 09:41, Jan Beulich wrote:
> While the hypervisor side of commit cd91ab08ea ("x86emul: correct stub
> invocation constraints") was fine, the tools side triggered a bogus
> error with old gcc (4.3 and 4.4 at least). Use a slightly less
> appropriate variant instead, proven to be good enough to not
> re-introduce the original problem: Which of the addresses is actually
> used doesn't matter much as long as the compiler can't prove that the
> two pointers don't alias one another.
>
> Reported-by: Boris Ostrovsky 
> Signed-off-by: Jan Beulich 

Acked-by: Andrew Cooper 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86emul: correct stub invocation constraints

[Jan Beulich]

>>> On 27.04.17 at 10:07,  wrote:
 On 26.04.17 at 16:01,  wrote:
>> On 04/25/2017 05:04 AM, Jan Beulich wrote:
>>> Stub invocations need to have the space the stub occupies as an input,
>>> to prevent the compiler from re-ordering (or omitting) writes to it.
>>>
>>> Signed-off-by: Jan Beulich 
>>>
>>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
>>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
>>> @@ -837,7 +837,8 @@ do{ asm volatile (
>>> ".popsection\n\t"\
>>> _ASM_EXTABLE(.Lret%=, .Lfix%=)   \
>>> : [exn] "+g" (res_), constraints,\
>>> - [stub] "rm" (stub.func) ); \
>>> + [stub] "rm" (stub.func),   \
>>> + "m" (*(uint8_t(*)[MAX_INST_LEN + 1])stub.ptr) );   \
>>>  if ( unlikely(~res_.raw) )  \
>>>  {   \
>>>  gprintk(XENLOG_WARNING, \
>>> @@ -853,7 +854,8 @@ do{ asm volatile (
>>>  #else
>>>  # define invoke_stub(pre, post, constraints...) \
>>>  asm volatile ( pre "\n\tcall *%[stub]\n\t" post \
>>> -   : constraints, [stub] "rm" (stub.func) )
>>> +   : constraints, [stub] "rm" (stub.func),  \
>>> + "m" (*(uint8_t(*)[MAX_INST_LEN + 1])stub.buf) )
>>>  #endif
>>>  
>>>  #define emulate_stub(dst, src...) do {  \
>>>
>> 
>> 
>> This breaks on old compilers:
>> 
>> FC-64
>> > ulator>
>> gcc --version
>> gcc (GCC) 4.4.4 20100503 (Red Hat 4.4.4-2)
> 
> Btw., I've just realized that I did use an old gcc only on the
> hypervisor build. Do you see the same issue there, or is this tools
> side specific?

And now that I've extracted it into a smaller example and thus was
able to try, I can see the issue with 4.3.x. The problem is that with

struct stub {
unsigned long addr;
void *ptr;
unsigned char buf[16];
};

void test(const struct stub*ptr) {
asm("" :: "m" (*(unsigned char(*)[16])ptr->addr));
asm("" :: "m" (*(unsigned char(*)[16])ptr->ptr));
asm("" :: "m" (*(unsigned char(*)[16])ptr->buf));
asm("" :: "m" (*(unsigned char(*)[16])>buf));
asm("" :: "m" (*(unsigned char(*)[16])>buf[0]));
}

none of the last three work, so we'll have to resort to using the
first. I'll have to verify that this is good enough for the case
where I did actually observe things to break without the extra
constraint (with a not yet submitted patch).

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86emul: correct stub invocation constraints

[Jan Beulich]

>>> On 26.04.17 at 16:01,  wrote:
> On 04/25/2017 05:04 AM, Jan Beulich wrote:
>> Stub invocations need to have the space the stub occupies as an input,
>> to prevent the compiler from re-ordering (or omitting) writes to it.
>>
>> Signed-off-by: Jan Beulich 
>>
>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
>> @@ -837,7 +837,8 @@ do{ asm volatile (
>> ".popsection\n\t"\
>> _ASM_EXTABLE(.Lret%=, .Lfix%=)   \
>> : [exn] "+g" (res_), constraints,\
>> - [stub] "rm" (stub.func) ); \
>> + [stub] "rm" (stub.func),   \
>> + "m" (*(uint8_t(*)[MAX_INST_LEN + 1])stub.ptr) );   \
>>  if ( unlikely(~res_.raw) )  \
>>  {   \
>>  gprintk(XENLOG_WARNING, \
>> @@ -853,7 +854,8 @@ do{ asm volatile (
>>  #else
>>  # define invoke_stub(pre, post, constraints...) \
>>  asm volatile ( pre "\n\tcall *%[stub]\n\t" post \
>> -   : constraints, [stub] "rm" (stub.func) )
>> +   : constraints, [stub] "rm" (stub.func),  \
>> + "m" (*(uint8_t(*)[MAX_INST_LEN + 1])stub.buf) )
>>  #endif
>>  
>>  #define emulate_stub(dst, src...) do {  \
>>
> 
> 
> This breaks on old compilers:
> 
> FC-64
>  ulator>
> gcc --version
> gcc (GCC) 4.4.4 20100503 (Red Hat 4.4.4-2)

Btw., I've just realized that I did use an old gcc only on the
hypervisor build. Do you see the same issue there, or is this tools
side specific?

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86emul: correct stub invocation constraints

[Boris Ostrovsky]

>> This breaks on old compilers:
>>
>> FC-64
>> > ulator>
>> gcc --version
>> gcc (GCC) 4.4.4 20100503 (Red Hat 4.4.4-2)
> I did try with 4.3.x, fwiw (but I'm afraid I've lost that machine just
> now, and will hardly set it up again using an old distro). Also I can't
> immediately see what the compiler dislikes and hence how a fix may
> look like (short of adding memory clobbers instead).

This is Fedora 13, if that helps.

I can probably arrange your access to that environment (copying Konrad
who owns it).

-boris



___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86emul: correct stub invocation constraints

[Jan Beulich]

>>> On 26.04.17 at 16:01,  wrote:
> On 04/25/2017 05:04 AM, Jan Beulich wrote:
>> Stub invocations need to have the space the stub occupies as an input,
>> to prevent the compiler from re-ordering (or omitting) writes to it.
>>
>> Signed-off-by: Jan Beulich 
>>
>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
>> @@ -837,7 +837,8 @@ do{ asm volatile (
>> ".popsection\n\t"\
>> _ASM_EXTABLE(.Lret%=, .Lfix%=)   \
>> : [exn] "+g" (res_), constraints,\
>> - [stub] "rm" (stub.func) ); \
>> + [stub] "rm" (stub.func),   \
>> + "m" (*(uint8_t(*)[MAX_INST_LEN + 1])stub.ptr) );   \
>>  if ( unlikely(~res_.raw) )  \
>>  {   \
>>  gprintk(XENLOG_WARNING, \
>> @@ -853,7 +854,8 @@ do{ asm volatile (
>>  #else
>>  # define invoke_stub(pre, post, constraints...) \
>>  asm volatile ( pre "\n\tcall *%[stub]\n\t" post \
>> -   : constraints, [stub] "rm" (stub.func) )
>> +   : constraints, [stub] "rm" (stub.func),  \
>> + "m" (*(uint8_t(*)[MAX_INST_LEN + 1])stub.buf) )
>>  #endif
>>  
>>  #define emulate_stub(dst, src...) do {  \
>>
> 
> 
> This breaks on old compilers:
> 
> FC-64
>  ulator>
> gcc --version
> gcc (GCC) 4.4.4 20100503 (Red Hat 4.4.4-2)

I did try with 4.3.x, fwiw (but I'm afraid I've lost that machine just
now, and will hardly set it up again using an old distro). Also I can't
immediately see what the compiler dislikes and hence how a fix may
look like (short of adding memory clobbers instead).

Jan

> Copyright (C) 2010 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> 
> FC-64
>  ulator>
> pwd
> /home/build/xtt-x86_64/bootstrap/xen.git/tools/fuzz/x86_instruction_emulator
> FC-64
>  ulator>
> gcc  -m64 -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall
> -Wstrict-prototypes -Wdeclaration-after-statement
> -Wno-unused-but-set-variable   -g3 -O0 -fno-omit-frame-pointer
> -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -MMD -MF
> .x86_emulate.o.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE  
> -I/home/build/xtt-x86_64/bootstrap/xen.git/tools/fuzz/x86_instruction_emulat
> or/../../../tools/include
> -D__XEN_TOOLS__ -I.  -c -o x86_emulate.o x86_emulate.c
> In file included from x86_emulate.c:157:
> x86_emulate/x86_emulate.c: In function ‘x86_emulate’:
> x86_emulate/x86_emulate.c:4085: error: memory input 3 is not directly
> addressable
> In file included from x86_emulate.c:157:
> x86_emulate/x86_emulate.c:4161: error: memory input 3 is not directly
> addressable
> In file included from x86_emulate.c:157:
> x86_emulate/x86_emulate.c:4226: error: memory input 5 is not directly
> addressable
> In file included from x86_emulate.c:157:
> x86_emulate/x86_emulate.c:4229: error: memory input 3 is not directly
> addressable
> In file included from x86_emulate.c:157:
> x86_emulate/x86_emulate.c:4279: error: memory input 5 is not directly
> addressable
> In file included from x86_emulate.c:157:
> x86_emulate/x86_emulate.c:4288: error: memory input 3 is not directly
> addressable
> In file included from x86_emulate.c:157:
> x86_emulate/x86_emulate.c:4353: error: memory input 3 is not directly
> addressable
> In file included from x86_emulate.c:157:
> x86_emulate/x86_emulate.c:4402: error: memory input 3 is not directly
> addressable
> In file included from x86_emulate.c:157:
> x86_emulate/x86_emulate.c:4465: error: memory input 3 is not directly
> addressable
> In file included from x86_emulate.c:157:
> x86_emulate/x86_emulate.c:4516: error: memory input 5 is not directly
> addressable
> In file included from x86_emulate.c:157:
> x86_emulate/x86_emulate.c:4522: error: memory input 3 is not directly
> addressable
> In file included from x86_emulate.c:157:
> x86_emulate/x86_emulate.c:5632: error: memory input 5 is not directly
> addressable
> In file included from x86_emulate.c:157:
> x86_emulate/x86_emulate.c:5679: error: memory input 8 is not directly
> addressable
> In file included from x86_emulate.c:157:
> x86_emulate/x86_emulate.c:5863: error: memory input 3 is not directly
> addressable
> 

Re: [Xen-devel] [PATCH] x86emul: correct stub invocation constraints

[Boris Ostrovsky]

On 04/25/2017 05:04 AM, Jan Beulich wrote:
> Stub invocations need to have the space the stub occupies as an input,
> to prevent the compiler from re-ordering (or omitting) writes to it.
>
> Signed-off-by: Jan Beulich 
>
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -837,7 +837,8 @@ do{ asm volatile (
> ".popsection\n\t"\
> _ASM_EXTABLE(.Lret%=, .Lfix%=)   \
> : [exn] "+g" (res_), constraints,\
> - [stub] "rm" (stub.func) ); \
> + [stub] "rm" (stub.func),   \
> + "m" (*(uint8_t(*)[MAX_INST_LEN + 1])stub.ptr) );   \
>  if ( unlikely(~res_.raw) )  \
>  {   \
>  gprintk(XENLOG_WARNING, \
> @@ -853,7 +854,8 @@ do{ asm volatile (
>  #else
>  # define invoke_stub(pre, post, constraints...) \
>  asm volatile ( pre "\n\tcall *%[stub]\n\t" post \
> -   : constraints, [stub] "rm" (stub.func) )
> +   : constraints, [stub] "rm" (stub.func),  \
> + "m" (*(uint8_t(*)[MAX_INST_LEN + 1])stub.buf) )
>  #endif
>  
>  #define emulate_stub(dst, src...) do {  \
>


This breaks on old compilers:

FC-64

gcc --version
gcc (GCC) 4.4.4 20100503 (Red Hat 4.4.4-2)
Copyright (C) 2010 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

FC-64

pwd
/home/build/xtt-x86_64/bootstrap/xen.git/tools/fuzz/x86_instruction_emulator
FC-64

gcc  -m64 -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall
-Wstrict-prototypes -Wdeclaration-after-statement
-Wno-unused-but-set-variable   -g3 -O0 -fno-omit-frame-pointer
-D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -MMD -MF
.x86_emulate.o.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE  
-I/home/build/xtt-x86_64/bootstrap/xen.git/tools/fuzz/x86_instruction_emulator/../../../tools/include
-D__XEN_TOOLS__ -I.  -c -o x86_emulate.o x86_emulate.c
In file included from x86_emulate.c:157:
x86_emulate/x86_emulate.c: In function ‘x86_emulate’:
x86_emulate/x86_emulate.c:4085: error: memory input 3 is not directly
addressable
In file included from x86_emulate.c:157:
x86_emulate/x86_emulate.c:4161: error: memory input 3 is not directly
addressable
In file included from x86_emulate.c:157:
x86_emulate/x86_emulate.c:4226: error: memory input 5 is not directly
addressable
In file included from x86_emulate.c:157:
x86_emulate/x86_emulate.c:4229: error: memory input 3 is not directly
addressable
In file included from x86_emulate.c:157:
x86_emulate/x86_emulate.c:4279: error: memory input 5 is not directly
addressable
In file included from x86_emulate.c:157:
x86_emulate/x86_emulate.c:4288: error: memory input 3 is not directly
addressable
In file included from x86_emulate.c:157:
x86_emulate/x86_emulate.c:4353: error: memory input 3 is not directly
addressable
In file included from x86_emulate.c:157:
x86_emulate/x86_emulate.c:4402: error: memory input 3 is not directly
addressable
In file included from x86_emulate.c:157:
x86_emulate/x86_emulate.c:4465: error: memory input 3 is not directly
addressable
In file included from x86_emulate.c:157:
x86_emulate/x86_emulate.c:4516: error: memory input 5 is not directly
addressable
In file included from x86_emulate.c:157:
x86_emulate/x86_emulate.c:4522: error: memory input 3 is not directly
addressable
In file included from x86_emulate.c:157:
x86_emulate/x86_emulate.c:5632: error: memory input 5 is not directly
addressable
In file included from x86_emulate.c:157:
x86_emulate/x86_emulate.c:5679: error: memory input 8 is not directly
addressable
In file included from x86_emulate.c:157:
x86_emulate/x86_emulate.c:5863: error: memory input 3 is not directly
addressable
x86_emulate/x86_emulate.c:6069: error: memory input 4 is not directly
addressable
x86_emulate/x86_emulate.c:6213: error: memory input 3 is not directly
addressable
In file included from x86_emulate.c:157:
x86_emulate/x86_emulate.c:7029: error: memory input 3 is not directly
addressable
In file included from x86_emulate.c:157:
x86_emulate/x86_emulate.c:7178: error: memory input 6 is not directly
addressable
In file included from x86_emulate.c:157:
x86_emulate/x86_emulate.c:7328: error: memory input 7 is not directly
addressable
In 

Re: [Xen-devel] [PATCH] x86emul: correct stub invocation constraints

[Julien Grall]

On 25/04/17 16:00, Andrew Cooper wrote:

On 25/04/17 10:04, Jan Beulich wrote:

Stub invocations need to have the space the stub occupies as an input,
to prevent the compiler from re-ordering (or omitting) writes to it.

Signed-off-by: Jan Beulich 


Reviewed-by: Andrew Cooper 


Release-acked-by: Julien Grall 





--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86emul: correct stub invocation constraints

[Andrew Cooper]

On 25/04/17 10:04, Jan Beulich wrote:
> Stub invocations need to have the space the stub occupies as an input,
> to prevent the compiler from re-ordering (or omitting) writes to it.
>
> Signed-off-by: Jan Beulich 

Reviewed-by: Andrew Cooper 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel