[RISCV] Error out on incorrectly spelt @plt on call symbols

[svs-quic]

The asm parser currently accepts anything after the @ eg. call symbol@rlt on call symbols and produces an empty object file. It would be better if we error out.

Fixes #135323

[llvmbot]

@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-risc-v

Author: Sudharsan Veeravalli (svs-quic)

Changes

The asm parser currently accepts anything after the @ eg. call symbol@<!-- -->rlt on call symbols and produces an empty object file. It would be better if we error out.

Fixes #135323

---

Full diff: https://github.com/llvm/llvm-project/pull/135324.diff

3 Files Affected:

• (modified) llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp (+5-2)
• (modified) llvm/test/MC/RISCV/function-call-invalid.s (+1)
• (modified) llvm/test/MC/RISCV/tail-call-invalid.s (+1)

diff --git a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
index 55f1a90b2a01a..5355d38fc046d 100644
--- a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
+++ b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
@@ -2123,14 +2123,18 @@ ParseStatus RISCVAsmParser::parseCallSymbol(OperandVector &Operands) {
 
   if (getLexer().getKind() != AsmToken::Identifier)
     return ParseStatus::NoMatch;
+
   std::string Identifier(getTok().getIdentifier());
+  SMLoc E = getTok().getEndLoc();
 
   if (getLexer().peekTok().is(AsmToken::At)) {
     Lex();
     Lex();
     StringRef PLT;
+    SMLoc PLTLoc = getLoc();
     if (getParser().parseIdentifier(PLT) || PLT != "plt")
-      return ParseStatus::Failure;
+      return Error(PLTLoc,
+                   "'@plt' is the only valid operand for this instruction");
   } else if (!getLexer().peekTok().is(AsmToken::EndOfStatement)) {
     // Avoid parsing the register in `call rd, foo` as a call symbol.
     return ParseStatus::NoMatch;
@@ -2138,7 +2142,6 @@ ParseStatus RISCVAsmParser::parseCallSymbol(OperandVector &Operands) {
     Lex();
   }
 
-  SMLoc E = SMLoc::getFromPointer(S.getPointer() + Identifier.size());
   RISCVMCExpr::Specifier Kind = RISCVMCExpr::VK_CALL_PLT;
 
   MCSymbol *Sym = getContext().getOrCreateSymbol(Identifier);
diff --git a/llvm/test/MC/RISCV/function-call-invalid.s b/llvm/test/MC/RISCV/function-call-invalid.s
index 2b7a85245880d..358d7342c6d1f 100644
--- a/llvm/test/MC/RISCV/function-call-invalid.s
+++ b/llvm/test/MC/RISCV/function-call-invalid.s
@@ -10,3 +10,4 @@ call %lo(1234) # CHECK: :[[@LINE]]:6: error: operand must be a bare symbol name
 call %hi(foo) # CHECK: :[[@LINE]]:6: error: operand must be a bare symbol name
 call %lo(foo) # CHECK: :[[@LINE]]:6: error: operand must be a bare symbol name
 call foo, bar # CHECK: :[[@LINE]]:6: error: operand must be a bare symbol name
+call foo@rlt # CHECK: :[[@LINE]]:10: error: '@plt' is the only valid operand for this instruction
diff --git a/llvm/test/MC/RISCV/tail-call-invalid.s b/llvm/test/MC/RISCV/tail-call-invalid.s
index 270d84df58ac4..6d31354ffce1f 100644
--- a/llvm/test/MC/RISCV/tail-call-invalid.s
+++ b/llvm/test/MC/RISCV/tail-call-invalid.s
@@ -10,3 +10,4 @@ tail %hi(1234) # CHECK: :[[@LINE]]:6: error: operand must be a bare symbol name
 tail %lo(1234) # CHECK: :[[@LINE]]:6: error: operand must be a bare symbol name
 tail %hi(foo) # CHECK: :[[@LINE]]:6: error: operand must be a bare symbol name
 tail %lo(foo) # CHECK: :[[@LINE]]:6: error: operand must be a bare symbol name
+tail foo@mlt # CHECK: :[[@LINE]]:10: error: '@plt' is the only valid operand for this instruction

[topperc]

Need to update E here

[svs-quic]

Copying my comment from here #135044 (comment)

Given that we parse and drop the @plt in the assembly that is being printed, shouldn't the end location be the end location of the Identifier (eg foo)?

[topperc]

As far as I know it is supposed to be the end location of the last token parsed to create the operand. If it ever gets used, it would be for diagnostic printing with an SMRange.

[jrtc27]

Should this be case-insensitive?

[svs-quic]

I'm not really sure. All instances in the code and tests have 'plt' in lower case only. I can maybe do PLT.lower() ?

[MaskRay]

I think it's fine to only accept @plt and not @PLT. This is a workaround for GCC output and we don't encourage that users write this...

After I replaced x@plt - . with %pltpcrel, this @plt is the only @ specifier workaround.

[jrtc27]

I went looking in binutils. It's only ever done strcmp("@plt") since the first commit of RISC-V support, and has ignored it since Sep 2022 / 2.40. So case-sensitive is fine.

[svs-quic]

Closing this since it has been fixed by #135509