llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.53k stars 11.79k forks source link

[Clang] Change register types. #95241

Open witbring opened 4 months ago

witbring commented 4 months ago

I discovered that Clang changes register types without any warning when processing invalid assembly code. I believe Clang should more accurately verify assembly syntax.

.intel_syntax noprefix
Bug:
    lar R11, R8W
    movmskpd R9, XMM0       
    pmovmskb R10, MM2
    tpause RDX
    umwait RBP    
Bug:
 lar    r11,r8
 movmskpd r9d,xmm0
 pmovmskb r10d,mm2
 tpause edx
 umwait ebp

You can reproduce the bug through godbolt. https://godbolt.org/z/WofxMjWqG

topperc commented 4 months ago

The movmskpd and movmskb is for compatibility with binutils assembler. In 64-bit mode, the instructions do write all 64 bits of the register since that's how 64-bit mode works.

lar also looks like it is compatible with binutils.

tpause and umwait are not compatible with binutils. https://godbolt.org/z/3YKo6bPeP

llvmbot commented 4 months ago

@llvm/issue-subscribers-backend-x86

Author: Hyungseok Kim (witbring)

I discovered that Clang changes register types without any warning when processing invalid assembly code. I believe Clang should more accurately verify assembly syntax. ``` .intel_syntax noprefix Bug: lar R11, R8W movmskpd R9, XMM0 pmovmskb R10, MM2 tpause RDX umwait RBP ``` ``` Bug: lar r11,r8 movmskpd r9d,xmm0 pmovmskb r10d,mm2 tpause edx umwait ebp ``` You can reproduce the bug through godbolt. https://godbolt.org/z/WofxMjWqG
witbring commented 4 months ago

Thank you for your response. As you pointed out, there are compatibility issues with the tpause and umwait instructions. However, I believe Clang should also address the syntax checker errors on lar, movmskpd, and pmovmskb instructions. This is because GAS might update their syntax checker, which could lead to compatibility issues.

topperc commented 4 months ago

However, I believe Clang should also address the syntax checker errors on lar, movmskpd, and pmovmskb instructions. This is because GAS might update their syntax checker, which could lead to compatibility issues.

But we don't want to reject code that gas accepts. That hurts people migrating from gas to clang.

witbring commented 4 months ago

I respect your opinion. So, are you planning to fix those two?

topperc commented 4 months ago

I respect your opinion. So, are you planning to fix those two?

I defer to @phoebewang. I'm not involved in X86 anymore.

witbring commented 4 months ago

Okay. Thanks.