Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2123,22 +2123,25 @@ 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")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to update E here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be case-insensitive?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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;
} else {
Lex();
}

SMLoc E = SMLoc::getFromPointer(S.getPointer() + Identifier.size());
RISCVMCExpr::Specifier Kind = RISCVMCExpr::VK_CALL_PLT;

MCSymbol *Sym = getContext().getOrCreateSymbol(Identifier);
Expand Down
1 change: 1 addition & 0 deletions llvm/test/MC/RISCV/function-call-invalid.s
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions llvm/test/MC/RISCV/tail-call-invalid.s
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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