Seemingly wrong generated code for a function with inline assembly

Seemingly wrong generated code for a function with inline assembly

[mihaidavid]

Hello,

For context, I'm writing a function as part of a Linux kernel module 
(x86_64),
its aim is to wrap a cpu instruction related to virtualization 
technology,
namely, VMREAD (part of Intel VMX / Intel VT-x).

The generated assembly code for the function seems to lack the 
initialization
of a local variable to 0 (`int _retval`).




Here's the C code (you can ignore the lines with the `/* IGNORE */` 
comment at
the end, the same behavior is observed with or without them).

My intention is for _retval to remain 0 after the asm block iff vmread 
is
successful (i.e. `ja 3f` is taken, jumping out of the asm block with
_retval remaining to its initial value of 0).

``` c
static noinline unsigned long __cpu_vmread(unsigned long field, int 
*retval) {
     unsigned long val;
     int _retval = 0;

     // IMPORTANT TODO: TEST FAULTING!!! MAYBE LABELS ARE WRONG!
     asm volatile(
         "1:    vmread %[field], %[val]\n\t"
         "      ja 3f\n\t" /* success! jump out of asm */
         /* VMfail, set ret to -2 and jump out of asm  */
         "      mov $-2, %[_retval]\n\t"
         "      jmp 3f\n\t"
         /* fault, set ret to -1 and fall out of asm */    /* IGNORE */
         "2:    mov $-1, %[_retval]\n\t"                   /* IGNORE */
         "3:\n\t"

         _ASM_EXTABLE(1b, 2b) /* IGNORE */
         : [val] "=rm" (val), [_retval] "=rm" (_retval)
         : [field] "r" (field)
         : "cc"
     );

     *retval = _retval;
     return val;
}
```

Here's the generated assembly for the function (objdump):
``` assembly
   7 0000000000000000 <__cpu_vmread>:
   8    0:   0f 78 f8                vmread %rdi,%rax
   9    3:   77 0c                   ja     11 <__cpu_vmread+0x11>
  10    5:   bf fe ff ff ff          mov    $0xfffffffe,%edi
  11    a:   eb 05                   jmp    11 <__cpu_vmread+0x11>
  12    c:   bf ff ff ff ff          mov    $0xffffffff,%edi
  13   11:   89 3e                   mov    %edi,(%rsi)     // *retval = 
_retval
  14   13:   c3                      retq

```

As you can see, the compiler decided that _retval should be allocated in
register %edi (see the `mov $-1/$-2, $[_retval]` instructions) but 
nowhere is it
initialized to 0 as the declaration says, so if `ja` is taken, _retval 
is
_undefined_ with a garbage value (I checked with the debugger), but it 
should
actually be 0.

To solve this I added an inline instruction before vmread to initialize 
_retval
to 0 and it works, but it still baffles me why the compiler doesn't make 
sure
_retval is initialized to 0 since it says so in its declaration.

Maybe (likely) I'm just missing something and this is actually expected 
behavior
but it could also be a bug in gcc, I'd appreciate it if someone could 
shed
some light. Could it have something to do with the optimization of the
function's epilogue?

The -O is -O2, the buildsystem is the standard one for linux modules
(kernel 5.10.7) and the only added cc options are `-g -DDEBUG`.
gcc version is:
`gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0`

Here are some things I tried, with no result:
* initializing _retval to another value instead of 0
* making the function inline, same happens at the inline sites
* removing `static noinline` altogether
* changing the optimization level to -O1


Thanks,
- Horia Mihai David (hMihaiDavid)

Re: Seemingly wrong generated code for a function with inline assembly

[Tom Kacvinsky]

On Thu, Jan 6, 2022 at 4:32 PM mihaidavid--- via Gcc-help
<gcc-help@gcc.gnu.org> wrote:
>
> Hello,
>
> For context, I'm writing a function as part of a Linux kernel module
> (x86_64),
> its aim is to wrap a cpu instruction related to virtualization
> technology,
> namely, VMREAD (part of Intel VMX / Intel VT-x).
>
> The generated assembly code for the function seems to lack the
> initialization
> of a local variable to 0 (`int _retval`).
>
>
>
>
> Here's the C code (you can ignore the lines with the `/* IGNORE */`
> comment at
> the end, the same behavior is observed with or without them).
>
> My intention is for _retval to remain 0 after the asm block iff vmread
> is
> successful (i.e. `ja 3f` is taken, jumping out of the asm block with
> _retval remaining to its initial value of 0).
>
> ``` c
> static noinline unsigned long __cpu_vmread(unsigned long field, int
> *retval) {
>      unsigned long val;
>      int _retval = 0;
>
>      // IMPORTANT TODO: TEST FAULTING!!! MAYBE LABELS ARE WRONG!
>      asm volatile(
>          "1:    vmread %[field], %[val]\n\t"
>          "      ja 3f\n\t" /* success! jump out of asm */
>          /* VMfail, set ret to -2 and jump out of asm  */
>          "      mov $-2, %[_retval]\n\t"
>          "      jmp 3f\n\t"
>          /* fault, set ret to -1 and fall out of asm */    /* IGNORE */
>          "2:    mov $-1, %[_retval]\n\t"                   /* IGNORE */
>          "3:\n\t"
>
>          _ASM_EXTABLE(1b, 2b) /* IGNORE */
>          : [val] "=rm" (val), [_retval] "=rm" (_retval)
>          : [field] "r" (field)
>          : "cc"
>      );
>
>      *retval = _retval;
>      return val;
> }
> ```
>
> Here's the generated assembly for the function (objdump):
> ``` assembly
>    7 0000000000000000 <__cpu_vmread>:
>    8    0:   0f 78 f8                vmread %rdi,%rax
>    9    3:   77 0c                   ja     11 <__cpu_vmread+0x11>
>   10    5:   bf fe ff ff ff          mov    $0xfffffffe,%edi
>   11    a:   eb 05                   jmp    11 <__cpu_vmread+0x11>
>   12    c:   bf ff ff ff ff          mov    $0xffffffff,%edi
>   13   11:   89 3e                   mov    %edi,(%rsi)     // *retval =
> _retval
>   14   13:   c3                      retq
>
> ```
>
> As you can see, the compiler decided that _retval should be allocated in
> register %edi (see the `mov $-1/$-2, $[_retval]` instructions) but
> nowhere is it
> initialized to 0 as the declaration says, so if `ja` is taken, _retval
> is
> _undefined_ with a garbage value (I checked with the debugger), but it
> should
> actually be 0.
>
> To solve this I added an inline instruction before vmread to initialize
> _retval
> to 0 and it works, but it still baffles me why the compiler doesn't make
> sure
> _retval is initialized to 0 since it says so in its declaration.
>
> Maybe (likely) I'm just missing something and this is actually expected
> behavior
> but it could also be a bug in gcc, I'd appreciate it if someone could
> shed
> some light. Could it have something to do with the optimization of the
> function's epilogue?
>
> The -O is -O2, the buildsystem is the standard one for linux modules
> (kernel 5.10.7) and the only added cc options are `-g -DDEBUG`.
> gcc version is:
> `gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0`
>
> Here are some things I tried, with no result:
> * initializing _retval to another value instead of 0
> * making the function inline, same happens at the inline sites
> * removing `static noinline` altogether
> * changing the optimization level to -O1
>

I see you ran objdump on the resulting object code.  Did you try
running gcc with -save-temps and look at the gcc produced .s
file?  Could be that gcc is doing the right thing, and gas is doing
something with the gcc generated assembly that's not right.

I think it's worth looking into seeing what -save-temps tells you.

Tom

Re: Seemingly wrong generated code for a function with inline assembly

[Andrew Haley]

On 1/6/22 21:31, mihaidavid--- via Gcc-help wrote:
> As you can see, the compiler decided that _retval should be allocated in
> register %edi (see the `mov $-1/$-2, $[_retval]` instructions) but
> nowhere is it
> initialized to 0 as the declaration says, so if `ja` is taken, _retval
> is
> _undefined_  with a garbage value (I checked with the debugger), but it
> should
> actually be 0.

_retval is marked in your asm as an output, therefore you have to set it in
the assembly code. gcc knows the assignment to _retval is unused. If you
want to modify _retval, use "+rm", not "=rm".

-- 
Andrew Haley  (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

Re: Seemingly wrong generated code for a function with inline assembly

[mihaidavid]

> _retval is marked in your asm as an output, therefore you have to set 
> it in
> the assembly code. gcc knows the assignment to _retval is unused. If 
> you
> want to modify _retval, use "+rm", not "=rm".

Thanks! It works now, I knew I must have done something wrong.

- Mihai