[llvm] [clang] ms inline asm: Fix {call,jmp} fptr (PR #73207)
Fangrui Song via cfe-commits
cfe-commits at lists.llvm.org
Wed Nov 22 20:06:46 PST 2023
https://github.com/MaskRay created https://github.com/llvm/llvm-project/pull/73207
https://reviews.llvm.org/D151863 (2023-05) removed
`BaseReg = BaseReg ? BaseReg : 1` (introduced in commit
175d0aeef3725ce17032e9ef76e018139f2f52f0 (2013)) and caused a
regression: ensuring a non-zero `BaseReg` was to treat
`static void (*fptr)(); __asm { call var }` as non-`AbsMem`
`AsmOperandClass` and generate `call $0`/`callq *fptr(%rip)` instead of
`call ${0:P}`/`callq *fptr`
(The asm template argument modifier `P` is for call targets, while
no modifier is used by other instructions like `mov rax, fptr`)
This patch reinstates the BaseReg-setting statement but places it inside
`if (IsGlobalLV)` for clarify.
The special case is unfortunate, but we also have special case in
similar places (https://reviews.llvm.org/D149920).
>From a2b0d74d35ea21612e96e322aed336cdc0b282ae Mon Sep 17 00:00:00 2001
From: Fangrui Song <i at maskray.me>
Date: Wed, 22 Nov 2023 16:14:14 -0800
Subject: [PATCH] ms inline asm: Fix {call,jmp} fptr
https://reviews.llvm.org/D151863 (2023-05) removed
`BaseReg = BaseReg ? BaseReg : 1` (introduced in commit
175d0aeef3725ce17032e9ef76e018139f2f52f0 (2013)) and caused a
regression: ensuring a non-zero `BaseReg` was to treat
`static void (*fptr)(); __asm { call var }` as non-`AbsMem`
`AsmOperandClass` and generate `call $0`/`callq *fptr(%rip)` instead of
`call ${0:P}`/`callq *fptr`
(The asm template argument modifier `P` is for call targets, while
no modifier is used by other instructions like `mov rax, fptr`)
This patch reinstates the BaseReg-setting statement but places it inside
`if (IsGlobalLV)` for clarify.
The special case is unfortunate, but we also have special case in
similar places (https://reviews.llvm.org/D149920).
---
clang/test/CodeGen/ms-inline-asm-64.c | 7 +++-
.../lib/Target/X86/AsmParser/X86AsmParser.cpp | 38 +++++++++++--------
2 files changed, 29 insertions(+), 16 deletions(-)
diff --git a/clang/test/CodeGen/ms-inline-asm-64.c b/clang/test/CodeGen/ms-inline-asm-64.c
index 313d380e121bce0..c7e4c1b603bd76c 100644
--- a/clang/test/CodeGen/ms-inline-asm-64.c
+++ b/clang/test/CodeGen/ms-inline-asm-64.c
@@ -60,17 +60,22 @@ int t4(void) {
}
void bar() {}
+static void (*fptr)();
void t5(void) {
__asm {
call bar
jmp bar
+ call fptr
+ jmp fptr
}
// CHECK: t5
// CHECK: call void asm sideeffect inteldialect
// CHECK-SAME: call ${0:P}
// CHECK-SAME: jmp ${1:P}
- // CHECK-SAME: "*m,*m,~{dirflag},~{fpsr},~{flags}"(ptr elementtype(void (...)) @bar, ptr elementtype(void (...)) @bar)
+ // CHECK-SAME: call $2
+ // CHECK-SAME: jmp $3
+ // CHECK-SAME: "*m,*m,*m,*m,~{dirflag},~{fpsr},~{flags}"(ptr elementtype(void (...)) @bar, ptr elementtype(void (...)) @bar, ptr elementtype(ptr) @fptr, ptr elementtype(ptr) @fptr)
}
void t47(void) {
diff --git a/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp b/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
index 008075163b90a8d..a02978c64412cf7 100644
--- a/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ b/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -1144,8 +1144,8 @@ class X86AsmParser : public MCTargetAsmParser {
bool ParseIntelMemoryOperandSize(unsigned &Size);
bool CreateMemForMSInlineAsm(unsigned SegReg, const MCExpr *Disp,
unsigned BaseReg, unsigned IndexReg,
- unsigned Scale, SMLoc Start, SMLoc End,
- unsigned Size, StringRef Identifier,
+ unsigned Scale, bool NonAbsMem, SMLoc Start,
+ SMLoc End, unsigned Size, StringRef Identifier,
const InlineAsmIdentifierInfo &Info,
OperandVector &Operands);
@@ -1745,10 +1745,13 @@ bool X86AsmParser::parseOperand(OperandVector &Operands, StringRef Name) {
return parseATTOperand(Operands);
}
-bool X86AsmParser::CreateMemForMSInlineAsm(
- unsigned SegReg, const MCExpr *Disp, unsigned BaseReg, unsigned IndexReg,
- unsigned Scale, SMLoc Start, SMLoc End, unsigned Size, StringRef Identifier,
- const InlineAsmIdentifierInfo &Info, OperandVector &Operands) {
+bool X86AsmParser::CreateMemForMSInlineAsm(unsigned SegReg, const MCExpr *Disp,
+ unsigned BaseReg, unsigned IndexReg,
+ unsigned Scale, bool NonAbsMem,
+ SMLoc Start, SMLoc End,
+ unsigned Size, StringRef Identifier,
+ const InlineAsmIdentifierInfo &Info,
+ OperandVector &Operands) {
// If we found a decl other than a VarDecl, then assume it is a FuncDecl or
// some other label reference.
if (Info.isKind(InlineAsmIdentifierInfo::IK_Label)) {
@@ -1773,11 +1776,15 @@ bool X86AsmParser::CreateMemForMSInlineAsm(
}
// It is widely common for MS InlineAsm to use a global variable and one/two
// registers in a mmory expression, and though unaccessible via rip/eip.
- if (IsGlobalLV && (BaseReg || IndexReg)) {
- Operands.push_back(X86Operand::CreateMem(getPointerWidth(), Disp, Start,
- End, Size, Identifier, Decl, 0,
- BaseReg && IndexReg));
- return false;
+ if (IsGlobalLV) {
+ if (BaseReg || IndexReg) {
+ Operands.push_back(X86Operand::CreateMem(getPointerWidth(), Disp, Start,
+ End, Size, Identifier, Decl, 0,
+ BaseReg && IndexReg));
+ return false;
+ }
+ if (NonAbsMem)
+ BaseReg = 1; // Make isAbsMem() false
}
Operands.push_back(X86Operand::CreateMem(
getPointerWidth(), SegReg, Disp, BaseReg, IndexReg, Scale, Start, End,
@@ -2621,9 +2628,12 @@ bool X86AsmParser::parseIntelOperand(OperandVector &Operands, StringRef Name) {
CheckBaseRegAndIndexRegAndScale(BaseReg, IndexReg, Scale, is64BitMode(),
ErrMsg))
return Error(Start, ErrMsg);
+ bool IsUnconditionalBranch =
+ Name.equals_insensitive("jmp") || Name.equals_insensitive("call");
if (isParsingMSInlineAsm())
- return CreateMemForMSInlineAsm(RegNo, Disp, BaseReg, IndexReg, Scale, Start,
- End, Size, SM.getSymName(),
+ return CreateMemForMSInlineAsm(RegNo, Disp, BaseReg, IndexReg, Scale,
+ IsUnconditionalBranch && is64BitMode(),
+ Start, End, Size, SM.getSymName(),
SM.getIdentifierInfo(), Operands);
// When parsing x64 MS-style assembly, all non-absolute references to a named
@@ -2631,8 +2641,6 @@ bool X86AsmParser::parseIntelOperand(OperandVector &Operands, StringRef Name) {
unsigned DefaultBaseReg = X86::NoRegister;
bool MaybeDirectBranchDest = true;
- bool IsUnconditionalBranch =
- Name.equals_insensitive("jmp") || Name.equals_insensitive("call");
if (Parser.isParsingMasm()) {
if (is64BitMode() && SM.getElementSize() > 0) {
DefaultBaseReg = X86::RIP;
More information about the cfe-commits
mailing list