Skip to content
This repository was archived by the owner on Feb 5, 2019. It is now read-only.

Commit 2235c76

Browse files
committed
[CodeGen] emit inline asm clobber list warnings for reserved
Summary: Currently, in line with GCC, when specifying reserved registers like sp or pc on an inline asm() clobber list, we don't always preserve the original value across the statement. And in general, overwriting reserved registers can have surprising results. For example: ``` extern int bar(int[]); int foo(int i) { int a[i]; // VLA asm volatile( "mov r7, #1" : : : "r7" ); return 1 + bar(a); } ``` Compiled for thumb, this gives: ``` $ clang --target=arm-arm-none-eabi -march=armv7a -c test.c -o - -S -O1 -mthumb ... foo: .fnstart @ %bb.0: @ %entry .save {r4, r5, r6, r7, lr} push {r4, r5, r6, r7, lr} .setfp r7, sp, #12 add r7, sp, #12 .pad #4 sub sp, #4 movs r1, #7 add.w r0, r1, r0, lsl #2 bic r0, r0, #7 sub.w r0, sp, r0 mov sp, r0 @app mov.w r7, #1 @NO_APP bl bar adds r0, #1 sub.w r4, r7, #12 mov sp, r4 pop {r4, r5, r6, r7, pc} ... ``` r7 is used as the frame pointer for thumb targets, and this function needs to restore the SP from the FP because of the variable-length stack allocation a. r7 is clobbered by the inline assembly (and r7 is included in the clobber list), but LLVM does not preserve the value of the frame pointer across the assembly block. This type of behavior is similar to GCC's and has been discussed on the bugtracker: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=11807 . No consensus seemed to have been reached on the way forward. Clang behavior has briefly been discussed on the CFE mailing (starting here: http://lists.llvm.org/pipermail/cfe-dev/2018-July/058392.html). I've opted for following Eli Friedman's advice to print warnings when there are reserved registers on the clobber list so as not to diverge from GCC behavior for now. The patch uses MachineRegisterInfo's target-specific knowledge of reserved registers, just before we convert the inline asm string in the AsmPrinter. If we find a reserved register, we print a warning: ``` repro.c:6:7: warning: inline asm clobber list contains reserved registers: R7 [-Winline-asm] "mov r7, #1" ^ ``` Reviewers: eli.friedman, olista01, javed.absar, efriedma Reviewed By: efriedma Subscribers: efriedma, eraman, kristof.beyls, llvm-commits Differential Revision: https://reviews.llvm.org/D49727 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@339257 91177308-0d34-0410-b5e6-96231b3b80d8
1 parent 5e1abaf commit 2235c76

File tree

5 files changed

+126
-32
lines changed

5 files changed

+126
-32
lines changed

include/llvm/CodeGen/AsmPrinter.h

+5
Original file line numberDiff line numberDiff line change
@@ -631,6 +631,11 @@ class AsmPrinter : public MachineFunctionPass {
631631
/// inline asm.
632632
void EmitInlineAsm(const MachineInstr *MI) const;
633633

634+
/// Add inline assembly info to the diagnostics machinery, so we can
635+
/// emit file and position info. Returns SrcMgr memory buffer position.
636+
unsigned addInlineAsmDiagBuffer(StringRef AsmStr,
637+
const MDNode *LocMDNode) const;
638+
634639
//===------------------------------------------------------------------===//
635640
// Internal Implementation Details
636641
//===------------------------------------------------------------------===//

lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp

+78-32
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,42 @@ static void srcMgrDiagHandler(const SMDiagnostic &Diag, void *diagInfo) {
7171
DiagInfo->DiagHandler(Diag, DiagInfo->DiagContext, LocCookie);
7272
}
7373

74+
unsigned AsmPrinter::addInlineAsmDiagBuffer(StringRef AsmStr,
75+
const MDNode *LocMDNode) const {
76+
if (!DiagInfo) {
77+
DiagInfo = make_unique<SrcMgrDiagInfo>();
78+
79+
MCContext &Context = MMI->getContext();
80+
Context.setInlineSourceManager(&DiagInfo->SrcMgr);
81+
82+
LLVMContext &LLVMCtx = MMI->getModule()->getContext();
83+
if (LLVMCtx.getInlineAsmDiagnosticHandler()) {
84+
DiagInfo->DiagHandler = LLVMCtx.getInlineAsmDiagnosticHandler();
85+
DiagInfo->DiagContext = LLVMCtx.getInlineAsmDiagnosticContext();
86+
DiagInfo->SrcMgr.setDiagHandler(srcMgrDiagHandler, DiagInfo.get());
87+
}
88+
}
89+
90+
SourceMgr &SrcMgr = DiagInfo->SrcMgr;
91+
92+
std::unique_ptr<MemoryBuffer> Buffer;
93+
// The inline asm source manager will outlive AsmStr, so make a copy of the
94+
// string for SourceMgr to own.
95+
Buffer = MemoryBuffer::getMemBufferCopy(AsmStr, "<inline asm>");
96+
97+
// Tell SrcMgr about this buffer, it takes ownership of the buffer.
98+
unsigned BufNum = SrcMgr.AddNewSourceBuffer(std::move(Buffer), SMLoc());
99+
100+
// Store LocMDNode in DiagInfo, using BufNum as an identifier.
101+
if (LocMDNode) {
102+
DiagInfo->LocInfos.resize(BufNum);
103+
DiagInfo->LocInfos[BufNum - 1] = LocMDNode;
104+
}
105+
106+
return BufNum;
107+
}
108+
109+
74110
/// EmitInlineAsm - Emit a blob of inline asm to the output streamer.
75111
void AsmPrinter::EmitInlineAsm(StringRef Str, const MCSubtargetInfo &STI,
76112
const MCTargetOptions &MCOptions,
@@ -98,39 +134,11 @@ void AsmPrinter::EmitInlineAsm(StringRef Str, const MCSubtargetInfo &STI,
98134
return;
99135
}
100136

101-
if (!DiagInfo) {
102-
DiagInfo = make_unique<SrcMgrDiagInfo>();
137+
unsigned BufNum = addInlineAsmDiagBuffer(Str, LocMDNode);
138+
DiagInfo->SrcMgr.setIncludeDirs(MCOptions.IASSearchPaths);
103139

104-
MCContext &Context = MMI->getContext();
105-
Context.setInlineSourceManager(&DiagInfo->SrcMgr);
106-
107-
LLVMContext &LLVMCtx = MMI->getModule()->getContext();
108-
if (LLVMCtx.getInlineAsmDiagnosticHandler()) {
109-
DiagInfo->DiagHandler = LLVMCtx.getInlineAsmDiagnosticHandler();
110-
DiagInfo->DiagContext = LLVMCtx.getInlineAsmDiagnosticContext();
111-
DiagInfo->SrcMgr.setDiagHandler(srcMgrDiagHandler, DiagInfo.get());
112-
}
113-
}
114-
115-
SourceMgr &SrcMgr = DiagInfo->SrcMgr;
116-
SrcMgr.setIncludeDirs(MCOptions.IASSearchPaths);
117-
118-
std::unique_ptr<MemoryBuffer> Buffer;
119-
// The inline asm source manager will outlive Str, so make a copy of the
120-
// string for SourceMgr to own.
121-
Buffer = MemoryBuffer::getMemBufferCopy(Str, "<inline asm>");
122-
123-
// Tell SrcMgr about this buffer, it takes ownership of the buffer.
124-
unsigned BufNum = SrcMgr.AddNewSourceBuffer(std::move(Buffer), SMLoc());
125-
126-
// Store LocMDNode in DiagInfo, using BufNum as an identifier.
127-
if (LocMDNode) {
128-
DiagInfo->LocInfos.resize(BufNum);
129-
DiagInfo->LocInfos[BufNum-1] = LocMDNode;
130-
}
131-
132-
std::unique_ptr<MCAsmParser> Parser(
133-
createMCAsmParser(SrcMgr, OutContext, *OutStreamer, *MAI, BufNum));
140+
std::unique_ptr<MCAsmParser> Parser(createMCAsmParser(
141+
DiagInfo->SrcMgr, OutContext, *OutStreamer, *MAI, BufNum));
134142

135143
// Do not use assembler-level information for parsing inline assembly.
136144
OutStreamer->setUseAssemblerInfoForParsing(false);
@@ -519,6 +527,44 @@ void AsmPrinter::EmitInlineAsm(const MachineInstr *MI) const {
519527
MCOptions.SanitizeAddress =
520528
MF->getFunction().hasFnAttribute(Attribute::SanitizeAddress);
521529

530+
// Emit warnings if we use reserved registers on the clobber list, as
531+
// that might give surprising results.
532+
std::vector<std::string> RestrRegs;
533+
// Start with the first operand descriptor, and iterate over them.
534+
for (unsigned I = InlineAsm::MIOp_FirstOperand, NumOps = MI->getNumOperands();
535+
I < NumOps; ++I) {
536+
const MachineOperand &MO = MI->getOperand(I);
537+
if (MO.isImm()) {
538+
unsigned Flags = MO.getImm();
539+
if (InlineAsm::getKind(Flags) == InlineAsm::Kind_Clobber &&
540+
MF->getRegInfo().isReserved(MI->getOperand(I + 1).getReg())) {
541+
const TargetRegisterInfo *TRI = MF->getSubtarget().getRegisterInfo();
542+
RestrRegs.push_back(TRI->getName(MI->getOperand(I + 1).getReg()));
543+
}
544+
// Skip to one before the next operand descriptor, if it exists.
545+
I += InlineAsm::getNumOperandRegisters(Flags);
546+
}
547+
}
548+
549+
if (!RestrRegs.empty()) {
550+
unsigned BufNum = addInlineAsmDiagBuffer(OS.str(), LocMD);
551+
auto &SrcMgr = DiagInfo->SrcMgr;
552+
SMLoc Loc = SMLoc::getFromPointer(
553+
SrcMgr.getMemoryBuffer(BufNum)->getBuffer().begin());
554+
555+
std::string Msg = "inline asm clobber list contains reserved registers: ";
556+
for (auto I = RestrRegs.begin(), E = RestrRegs.end(); I != E; I++) {
557+
if(I != RestrRegs.begin())
558+
Msg += ", ";
559+
Msg += *I;
560+
}
561+
std::string Note = "Reserved registers on the clobber list may not be "
562+
"preserved across the asm statement, and clobbering them may "
563+
"lead to undefined behaviour.";
564+
SrcMgr.PrintMessage(Loc, SourceMgr::DK_Warning, Msg);
565+
SrcMgr.PrintMessage(Loc, SourceMgr::DK_Note, Note);
566+
}
567+
522568
EmitInlineAsm(OS.str(), getSubtargetInfo(), MCOptions, LocMD,
523569
MI->getInlineAsmDialect());
524570

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
; RUN: llc <%s -mtriple=aarch64-none-eabi 2>&1 | FileCheck %s
2+
3+
; CHECK: warning: inline asm clobber list contains reserved registers: SP
4+
5+
define void @foo() nounwind {
6+
call void asm sideeffect "mov x7, #1", "~{x7},~{sp}"()
7+
ret void
8+
}
+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
; RUN: llc <%s -mtriple=arm-none-eabi 2>&1 | FileCheck %s -check-prefix=CHECK
2+
3+
; RUN: llc <%s -mtriple=arm-none-eabi -relocation-model=rwpi 2>&1 \
4+
; RUN: | FileCheck %s -check-prefix=RWPI
5+
6+
; RUN: llc <%s -mtriple=arm-none-eabi --disable-fp-elim 2>&1 \
7+
; RUN: | FileCheck %s -check-prefix=NO_FP_ELIM
8+
9+
; CHECK: warning: inline asm clobber list contains reserved registers: SP, PC
10+
; CHECK: warning: inline asm clobber list contains reserved registers: R11
11+
; RWPI: warning: inline asm clobber list contains reserved registers: R9, SP, PC
12+
; RWPI: warning: inline asm clobber list contains reserved registers: R11
13+
; NO_FP_ELIM: warning: inline asm clobber list contains reserved registers: R11, SP, PC
14+
; NO_FP_ELIM: warning: inline asm clobber list contains reserved registers: R11
15+
16+
define void @foo() nounwind {
17+
call void asm sideeffect "mov r7, #1",
18+
"~{r9},~{r11},~{r12},~{lr},~{sp},~{pc},~{r10}"()
19+
ret void
20+
}
21+
22+
define i32 @bar(i32 %i) {
23+
%vla = alloca i32, i32 %i, align 4
24+
tail call void asm sideeffect "mov r7, #1", "~{r11}"()
25+
%1 = load volatile i32, i32* %vla, align 4
26+
ret i32 %1
27+
}
+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
; RUN: llc <%s -mtriple=x86_64-unknown-unknown -- 2>&1 | FileCheck %s
2+
3+
; CHECK: warning: inline asm clobber list contains reserved registers: RSP, EBP
4+
5+
define void @foo() nounwind {
6+
call void asm sideeffect "mov $$0x12, %eax", "~{eax},~{rsp},~{esi},~{ebp}"()
7+
ret void
8+
}

0 commit comments

Comments
 (0)