Skip to content

Commit

Permalink
[AsmParser] Add source location to all errors related to .cfi directives
Browse files Browse the repository at this point in the history
I was trying to add .cfi_ annotations to assembly code in the FreeBSD
kernel and changed a macro that then resulted in incorrectly nested
directives. However, clang's diagnostics said the error was happening at
<unknown>:0. This addresses one of the TODOs added in D51695.

Reviewed By: MaskRay

Differential Revision: https://reviews.llvm.org/D89787
  • Loading branch information
arichardson committed Nov 11, 2020
1 parent 3df3b62 commit fb9942f
Show file tree
Hide file tree
Showing 7 changed files with 20 additions and 30 deletions.
2 changes: 1 addition & 1 deletion llvm/include/llvm/MC/MCStreamer.h
Expand Up @@ -1064,7 +1064,7 @@ class MCStreamer {
/// Streamer specific finalization.
virtual void finishImpl();
/// Finish emission of machine code.
void Finish();
void Finish(SMLoc EndLoc = SMLoc());

virtual bool mayHaveInstructions(MCSection &Sec) const { return true; }
};
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/MC/MCParser/AsmParser.cpp
Expand Up @@ -994,7 +994,7 @@ bool AsmParser::Run(bool NoInitialTextSection, bool NoFinalize) {
// Finalize the output stream if there are no errors and if the client wants
// us to.
if (!HadError && !NoFinalize)
Out.Finish();
Out.Finish(Lexer.getLoc());

return HadError || getContext().hadError();
}
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/MC/MCParser/MasmParser.cpp
Expand Up @@ -1280,7 +1280,7 @@ bool MasmParser::Run(bool NoInitialTextSection, bool NoFinalize) {
// Finalize the output stream if there are no errors and if the client wants
// us to.
if (!HadError && !NoFinalize)
Out.Finish();
Out.Finish(Lexer.getLoc());

return HadError || getContext().hadError();
}
Expand Down
10 changes: 5 additions & 5 deletions llvm/lib/MC/MCStreamer.cpp
Expand Up @@ -273,9 +273,9 @@ bool MCStreamer::hasUnfinishedDwarfFrameInfo() {

MCDwarfFrameInfo *MCStreamer::getCurrentDwarfFrameInfo() {
if (!hasUnfinishedDwarfFrameInfo()) {
getContext().reportError(SMLoc(), "this directive must appear between "
".cfi_startproc and .cfi_endproc "
"directives");
getContext().reportError(getStartTokLoc(),
"this directive must appear between "
".cfi_startproc and .cfi_endproc directives");
return nullptr;
}
return &DwarfFrameInfos.back();
Expand Down Expand Up @@ -967,10 +967,10 @@ void MCStreamer::emitRawText(const Twine &T) {
void MCStreamer::EmitWindowsUnwindTables() {
}

void MCStreamer::Finish() {
void MCStreamer::Finish(SMLoc EndLoc) {
if ((!DwarfFrameInfos.empty() && !DwarfFrameInfos.back().End) ||
(!WinFrameInfos.empty() && !WinFrameInfos.back()->End)) {
getContext().reportError(SMLoc(), "Unfinished frame!");
getContext().reportError(EndLoc, "Unfinished frame!");
return;
}

Expand Down
4 changes: 2 additions & 2 deletions llvm/test/MC/X86/cfi-open-within-another-crash.s
Expand Up @@ -12,7 +12,7 @@ proc_one:
.globl proc_two
proc_two:
.cfi_startproc

# CHECK: [[#@LINE]]:1: error: starting new .cfi frame before finishing the previous one

.cfi_endproc

# CHECK: error: starting new .cfi frame before finishing the previous one
20 changes: 10 additions & 10 deletions llvm/test/MC/X86/cfi-scope-errors.s
Expand Up @@ -3,23 +3,23 @@

.text
.cfi_def_cfa rsp, 8
# CHECK: error: this directive must appear between .cfi_startproc and .cfi_endproc directives
# CHECK: [[#@LINE-1]]:1: error: this directive must appear between .cfi_startproc and .cfi_endproc directives

.cfi_startproc
nop

# TODO(kristina): As Reid suggested, this now supports source locations as a side effect
# of another patch aimed at fixing the crash that would occur here, however the other
# ones do not unfortunately. Will address it in a further patch propogating SMLoc down to
# other CFI directives at which point more LINE checks can be added to ensure proper source
# location reporting.

# This tests source location correctness as well as the error and it not crashing.
# CHECK: [[@LINE+2]]:1: error: starting new .cfi frame before finishing the previous one
## This tests source location correctness as well as the error and it not crashing.
# CHECK: [[#@LINE+2]]:1: error: starting new .cfi frame before finishing the previous one
.cfi_startproc

nop
.cfi_endproc

.cfi_def_cfa rsp, 8
# CHECK: error: this directive must appear between .cfi_startproc and .cfi_endproc directives
# CHECK: [[#@LINE-1]]:1: error: this directive must appear between .cfi_startproc and .cfi_endproc directives

## Check we don't crash on unclosed frame scope.
.globl foo
foo:
.cfi_startproc
# CHECK: [[#@LINE+1]]:1: error: Unfinished frame!
10 changes: 0 additions & 10 deletions llvm/test/MC/X86/cfi-scope-unclosed.s

This file was deleted.

0 comments on commit fb9942f

Please sign in to comment.