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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jan 6, 2022 at 4:32 PM mihaidavid--- via Gcc-help
<gcc-help@xxxxxxxxxxx> 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



[Index of Archives]     [Linux C Programming]     [Linux Kernel]     [eCos]     [Fedora Development]     [Fedora Announce]     [Autoconf]     [The DWARVES Debugging Tools]     [Yosemite Campsites]     [Yosemite News]     [Linux GCC]

  Powered by Linux