From e503aa9de4d542743435e2516b8239bf665edf1e Mon Sep 17 00:00:00 2001 From: Luke Cheeseman Date: Fri, 21 Dec 2018 10:45:08 +0000 Subject: [PATCH] [Dwarf/AArch64] Return address signing B key dwarf support - When signing return addresses with -msign-return-address={+}, either the A key instructions or the B key instructions can be used. To correctly authenticate the return address, the unwinder/debugger must know which key was used to sign the return address. - When and exception is thrown or a break point reached, it may be necessary to unwind the stack. To accomplish this, the unwinder/debugger must be able to first authenticate an the return address if it has been signed. - To enable this, the augmentation string of CIEs has been extended to allow inclusion of a 'B' character. Functions that are signed using the B key variant of the instructions should have and FDE whose associated CIE has a 'B' in the augmentation string. - One must also be able to preserve these semantics when first stepping from a high level language into assembly and then, as a second step, into an object file. To achieve this, I have introduced a new assembly directive '.cfi_b_key_frame ', that tells the assembler the current frame uses return address signing with the B key. - This ensures that the FDE is associated with a CIE that has 'B' in the augmentation string. Differential Revision: https://reviews.llvm.org/D51798 llvm-svn: 349895 --- include/llvm/MC/MCDwarf.h | 1 + include/llvm/MC/MCStreamer.h | 2 + lib/DebugInfo/DWARF/DWARFDebugFrame.cpp | 5 ++ lib/MC/MCAsmStreamer.cpp | 7 +++ lib/MC/MCDwarf.cpp | 29 ++++++----- lib/MC/MCParser/AsmParser.cpp | 2 + lib/MC/MCStreamer.cpp | 7 +++ lib/Target/AArch64/AArch64AsmPrinter.cpp | 13 +++++ lib/Target/AArch64/AArch64FrameLowering.cpp | 13 +++-- lib/Target/AArch64/AArch64InstrInfo.td | 4 ++ .../AArch64/AsmParser/AArch64AsmParser.cpp | 13 +++++ .../MIR/AArch64/return-address-signing.mir | 48 +++++++++++++++++++ .../AArch64/return-address-signing.ll | 27 +++++++++++ test/MC/ELF/cfi-b-key-frame.s | 6 +++ 14 files changed, 161 insertions(+), 16 deletions(-) create mode 100644 test/CodeGen/MIR/AArch64/return-address-signing.mir create mode 100644 test/DebugInfo/AArch64/return-address-signing.ll create mode 100644 test/MC/ELF/cfi-b-key-frame.s diff --git a/include/llvm/MC/MCDwarf.h b/include/llvm/MC/MCDwarf.h index aeaa4e55879..7b96e9aaca8 100644 --- a/include/llvm/MC/MCDwarf.h +++ b/include/llvm/MC/MCDwarf.h @@ -599,6 +599,7 @@ struct MCDwarfFrameInfo { bool IsSignalFrame = false; bool IsSimple = false; unsigned RAReg = static_cast(INT_MAX); + bool IsBKeyFrame = false; }; class MCDwarfFrameEmitter { diff --git a/include/llvm/MC/MCStreamer.h b/include/llvm/MC/MCStreamer.h index 849ea9a624e..f613d3a1943 100644 --- a/include/llvm/MC/MCStreamer.h +++ b/include/llvm/MC/MCStreamer.h @@ -806,6 +806,8 @@ public: Optional Source, unsigned CUID = 0); + virtual void EmitCFIBKeyFrame(); + /// This implements the DWARF2 '.loc fileno lineno ...' assembler /// directive. virtual void EmitDwarfLocDirective(unsigned FileNo, unsigned Line, diff --git a/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp b/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp index 6349a6efb8c..ba55ffc2817 100644 --- a/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp +++ b/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp @@ -446,6 +446,11 @@ void DWARFDebugFrame::parse(DWARFDataExtractor Data) { StartAugmentationOffset = Offset; EndAugmentationOffset = Offset + static_cast(*AugmentationLength); + break; + case 'B': + // B-Key is used for signing functions associated with this + // augmentation string + break; } } diff --git a/lib/MC/MCAsmStreamer.cpp b/lib/MC/MCAsmStreamer.cpp index 4f0efab73e8..e017103070b 100644 --- a/lib/MC/MCAsmStreamer.cpp +++ b/lib/MC/MCAsmStreamer.cpp @@ -266,6 +266,7 @@ public: void EmitCVFPOData(const MCSymbol *ProcSym, SMLoc L) override; void EmitIdent(StringRef IdentString) override; + void EmitCFIBKeyFrame() override; void EmitCFISections(bool EH, bool Debug) override; void EmitCFIDefCfa(int64_t Register, int64_t Offset) override; void EmitCFIDefCfaOffset(int64_t Offset) override; @@ -1602,6 +1603,12 @@ void MCAsmStreamer::EmitCFIReturnColumn(int64_t Register) { EmitEOL(); } +void MCAsmStreamer::EmitCFIBKeyFrame() { + MCStreamer::EmitCFIBKeyFrame(); + OS << "\t.cfi_b_key_frame"; + EmitEOL(); +} + void MCAsmStreamer::EmitWinCFIStartProc(const MCSymbol *Symbol, SMLoc Loc) { MCStreamer::EmitWinCFIStartProc(Symbol, Loc); diff --git a/lib/MC/MCDwarf.cpp b/lib/MC/MCDwarf.cpp index 9791fb5724b..38b02694d81 100644 --- a/lib/MC/MCDwarf.cpp +++ b/lib/MC/MCDwarf.cpp @@ -1565,9 +1565,8 @@ const MCSymbol &FrameEmitterImpl::EmitCIE(const MCDwarfFrameInfo &Frame) { uint8_t CIEVersion = getCIEVersion(IsEH, context.getDwarfVersion()); Streamer.EmitIntValue(CIEVersion, 1); - // Augmentation String - SmallString<8> Augmentation; if (IsEH) { + SmallString<8> Augmentation; Augmentation += "z"; if (Frame.Personality) Augmentation += "P"; @@ -1576,6 +1575,8 @@ const MCSymbol &FrameEmitterImpl::EmitCIE(const MCDwarfFrameInfo &Frame) { Augmentation += "R"; if (Frame.IsSignalFrame) Augmentation += "S"; + if (Frame.IsBKeyFrame) + Augmentation += "B"; Streamer.EmitBytes(Augmentation); } Streamer.EmitIntValue(0, 1); @@ -1730,25 +1731,28 @@ namespace { struct CIEKey { static const CIEKey getEmptyKey() { - return CIEKey(nullptr, 0, -1, false, false, static_cast(INT_MAX)); + return CIEKey(nullptr, 0, -1, false, false, static_cast(INT_MAX), + false); } static const CIEKey getTombstoneKey() { - return CIEKey(nullptr, -1, 0, false, false, static_cast(INT_MAX)); + return CIEKey(nullptr, -1, 0, false, false, static_cast(INT_MAX), + false); } CIEKey(const MCSymbol *Personality, unsigned PersonalityEncoding, unsigned LSDAEncoding, bool IsSignalFrame, bool IsSimple, - unsigned RAReg) + unsigned RAReg, bool IsBKeyFrame) : Personality(Personality), PersonalityEncoding(PersonalityEncoding), LsdaEncoding(LSDAEncoding), IsSignalFrame(IsSignalFrame), - IsSimple(IsSimple), RAReg(RAReg) {} + IsSimple(IsSimple), RAReg(RAReg), IsBKeyFrame(IsBKeyFrame) {} explicit CIEKey(const MCDwarfFrameInfo &Frame) : Personality(Frame.Personality), PersonalityEncoding(Frame.PersonalityEncoding), LsdaEncoding(Frame.LsdaEncoding), IsSignalFrame(Frame.IsSignalFrame), - IsSimple(Frame.IsSimple), RAReg(Frame.RAReg) {} + IsSimple(Frame.IsSimple), RAReg(Frame.RAReg), + IsBKeyFrame(Frame.IsBKeyFrame) {} const MCSymbol *Personality; unsigned PersonalityEncoding; @@ -1756,6 +1760,7 @@ struct CIEKey { bool IsSignalFrame; bool IsSimple; unsigned RAReg; + bool IsBKeyFrame; }; } // end anonymous namespace @@ -1767,9 +1772,9 @@ template <> struct DenseMapInfo { static CIEKey getTombstoneKey() { return CIEKey::getTombstoneKey(); } static unsigned getHashValue(const CIEKey &Key) { - return static_cast( - hash_combine(Key.Personality, Key.PersonalityEncoding, Key.LsdaEncoding, - Key.IsSignalFrame, Key.IsSimple, Key.RAReg)); + return static_cast(hash_combine( + Key.Personality, Key.PersonalityEncoding, Key.LsdaEncoding, + Key.IsSignalFrame, Key.IsSimple, Key.RAReg, Key.IsBKeyFrame)); } static bool isEqual(const CIEKey &LHS, const CIEKey &RHS) { @@ -1777,8 +1782,8 @@ template <> struct DenseMapInfo { LHS.PersonalityEncoding == RHS.PersonalityEncoding && LHS.LsdaEncoding == RHS.LsdaEncoding && LHS.IsSignalFrame == RHS.IsSignalFrame && - LHS.IsSimple == RHS.IsSimple && - LHS.RAReg == RHS.RAReg; + LHS.IsSimple == RHS.IsSimple && LHS.RAReg == RHS.RAReg && + LHS.IsBKeyFrame == RHS.IsBKeyFrame; } }; diff --git a/lib/MC/MCParser/AsmParser.cpp b/lib/MC/MCParser/AsmParser.cpp index aa07eee4491..5c76c900c74 100644 --- a/lib/MC/MCParser/AsmParser.cpp +++ b/lib/MC/MCParser/AsmParser.cpp @@ -495,6 +495,7 @@ private: DK_CFI_UNDEFINED, DK_CFI_REGISTER, DK_CFI_WINDOW_SAVE, + DK_CFI_B_KEY_FRAME, DK_MACROS_ON, DK_MACROS_OFF, DK_ALTMACRO, @@ -5293,6 +5294,7 @@ void AsmParser::initializeDirectiveKindMap() { DirectiveKindMap[".cfi_undefined"] = DK_CFI_UNDEFINED; DirectiveKindMap[".cfi_register"] = DK_CFI_REGISTER; DirectiveKindMap[".cfi_window_save"] = DK_CFI_WINDOW_SAVE; + DirectiveKindMap[".cfi_b_key_frame"] = DK_CFI_B_KEY_FRAME; DirectiveKindMap[".macros_on"] = DK_MACROS_ON; DirectiveKindMap[".macros_off"] = DK_MACROS_OFF; DirectiveKindMap[".macro"] = DK_MACRO; diff --git a/lib/MC/MCStreamer.cpp b/lib/MC/MCStreamer.cpp index 85e69f6f663..18fbea4afa0 100644 --- a/lib/MC/MCStreamer.cpp +++ b/lib/MC/MCStreamer.cpp @@ -221,6 +221,13 @@ void MCStreamer::emitDwarfFile0Directive(StringRef Directory, Source); } +void MCStreamer::EmitCFIBKeyFrame() { + MCDwarfFrameInfo *CurFrame = getCurrentDwarfFrameInfo(); + if (!CurFrame) + return; + CurFrame->IsBKeyFrame = true; +} + void MCStreamer::EmitDwarfLocDirective(unsigned FileNo, unsigned Line, unsigned Column, unsigned Flags, unsigned Isa, diff --git a/lib/Target/AArch64/AArch64AsmPrinter.cpp b/lib/Target/AArch64/AArch64AsmPrinter.cpp index 828fbb2de7e..0442076992e 100644 --- a/lib/Target/AArch64/AArch64AsmPrinter.cpp +++ b/lib/Target/AArch64/AArch64AsmPrinter.cpp @@ -716,6 +716,19 @@ void AArch64AsmPrinter::EmitInstruction(const MachineInstr *MI) { OutStreamer->EmitRawText(StringRef(OS.str())); } return; + + case AArch64::EMITBKEY: { + ExceptionHandling ExceptionHandlingType = MAI->getExceptionHandlingType(); + if (ExceptionHandlingType != ExceptionHandling::DwarfCFI && + ExceptionHandlingType != ExceptionHandling::ARM) + return; + + if (needsCFIMoves() == CFI_M_None) + return; + + OutStreamer->EmitCFIBKeyFrame(); + return; + } } // Tail calls use pseudo instructions so they have the proper code-gen diff --git a/lib/Target/AArch64/AArch64FrameLowering.cpp b/lib/Target/AArch64/AArch64FrameLowering.cpp index 800dd449b49..538a8d7e8fb 100644 --- a/lib/Target/AArch64/AArch64FrameLowering.cpp +++ b/lib/Target/AArch64/AArch64FrameLowering.cpp @@ -816,10 +816,15 @@ void AArch64FrameLowering::emitPrologue(MachineFunction &MF, DebugLoc DL; if (ShouldSignReturnAddress(MF)) { - BuildMI( - MBB, MBBI, DL, - TII->get(ShouldSignWithAKey(MF) ? AArch64::PACIASP : AArch64::PACIBSP)) - .setMIFlag(MachineInstr::FrameSetup); + if (ShouldSignWithAKey(MF)) + BuildMI(MBB, MBBI, DL, TII->get(AArch64::PACIASP)) + .setMIFlag(MachineInstr::FrameSetup); + else { + BuildMI(MBB, MBBI, DL, TII->get(AArch64::EMITBKEY)) + .setMIFlag(MachineInstr::FrameSetup); + BuildMI(MBB, MBBI, DL, TII->get(AArch64::PACIBSP)) + .setMIFlag(MachineInstr::FrameSetup); + } unsigned CFIIndex = MF.addFrameInst(MCCFIInstruction::createNegateRAState(nullptr)); diff --git a/lib/Target/AArch64/AArch64InstrInfo.td b/lib/Target/AArch64/AArch64InstrInfo.td index ee43b55aed9..eb972b51134 100644 --- a/lib/Target/AArch64/AArch64InstrInfo.td +++ b/lib/Target/AArch64/AArch64InstrInfo.td @@ -1619,6 +1619,10 @@ def TLSDESCCALL : Pseudo<(outs), (ins i64imm:$sym), []>, Sched<[]> { let AsmString = ".tlsdesccall $sym"; } +// Pseudo instruction to tell the streamer to emit a 'B' character into the +// augmentation string. +def EMITBKEY : Pseudo<(outs), (ins), []>, Sched<[]> {} + // FIXME: maybe the scratch register used shouldn't be fixed to X1? // FIXME: can "hasSideEffects be dropped? let isCall = 1, Defs = [LR, X0, X1], hasSideEffects = 1, diff --git a/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp b/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp index a65ddd478f4..384b474abe8 100644 --- a/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp +++ b/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp @@ -176,6 +176,7 @@ private: bool parseDirectiveReq(StringRef Name, SMLoc L); bool parseDirectiveUnreq(SMLoc L); bool parseDirectiveCFINegateRAState(); + bool parseDirectiveCFIBKeyFrame(); bool validateInstruction(MCInst &Inst, SMLoc &IDLoc, SmallVectorImpl &Loc); @@ -5030,6 +5031,8 @@ bool AArch64AsmParser::ParseDirective(AsmToken DirectiveID) { parseDirectiveInst(Loc); else if (IDVal == ".cfi_negate_ra_state") parseDirectiveCFINegateRAState(); + else if (IDVal == ".cfi_b_key_frame") + parseDirectiveCFIBKeyFrame(); else if (IsMachO) { if (IDVal == MCLOHDirectiveName()) parseDirectiveLOH(IDVal, Loc); @@ -5410,6 +5413,16 @@ bool AArch64AsmParser::parseDirectiveCFINegateRAState() { return false; } +/// parseDirectiveCFIBKeyFrame +/// ::= .cfi_b_key +bool AArch64AsmParser::parseDirectiveCFIBKeyFrame() { + if (parseToken(AsmToken::EndOfStatement, + "unexpected token in '.cfi_b_key_frame'")) + return true; + getStreamer().EmitCFIBKeyFrame(); + return false; +} + bool AArch64AsmParser::classifySymbolRef(const MCExpr *Expr, AArch64MCExpr::VariantKind &ELFRefKind, diff --git a/test/CodeGen/MIR/AArch64/return-address-signing.mir b/test/CodeGen/MIR/AArch64/return-address-signing.mir new file mode 100644 index 00000000000..1489c677567 --- /dev/null +++ b/test/CodeGen/MIR/AArch64/return-address-signing.mir @@ -0,0 +1,48 @@ +# RUN: llc -mtriple=aarch64-arm-none-eabi -run-pass=prologepilog -o - %s 2>&1 | FileCheck %s +--- | + target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128" + target triple = "aarch64-arm-none-eabi" + + define dso_local i32 @foo() "sign-return-address"="all" "sign-return-address-key"="a_key" { + entry: + ret i32 2 + } + + define dso_local i32 @bar() "sign-return-address"="all" "sign-return-address-key"="b_key" { + entry: + ret i32 2 + } +... +--- +#CHECK: foo +name: foo +alignment: 2 +tracksRegLiveness: true +frameInfo: + maxCallFrameSize: 0 +#CHECK: frame-setup PACIASP implicit-def $lr, implicit $lr, implicit $sp +#CHECK: frame-setup CFI_INSTRUCTION negate_ra_sign_state +#CHECK: frame-destroy AUTIASP implicit-def $lr, implicit $lr, implicit $sp +body: | + bb.0.entry: + $w0 = MOVi32imm 2 + RET_ReallyLR implicit killed $w0 + +... +--- +#CHECK: bar +name: bar +alignment: 2 +tracksRegLiveness: true +frameInfo: + maxCallFrameSize: 0 +#CHECK: frame-setup EMITBKEY +#CHECK: frame-setup PACIBSP implicit-def $lr, implicit $lr, implicit $sp +#CHECK: frame-setup CFI_INSTRUCTION negate_ra_sign_state +#CHECK: frame-destroy AUTIBSP implicit-def $lr, implicit $lr, implicit $sp +body: | + bb.0.entry: + $w0 = MOVi32imm 2 + RET_ReallyLR implicit killed $w0 + +... diff --git a/test/DebugInfo/AArch64/return-address-signing.ll b/test/DebugInfo/AArch64/return-address-signing.ll new file mode 100644 index 00000000000..c679a9da65d --- /dev/null +++ b/test/DebugInfo/AArch64/return-address-signing.ll @@ -0,0 +1,27 @@ +; RUN: llc -mtriple=aarch64-arm-none-eabi < %s -filetype=obj -o - \ +; RUN: | llvm-dwarfdump -v - | FileCheck -check-prefix=CHECK %s + +;CHECK: CIE +;CHECK: Augmentation: "zR" +define i32 @foo() "sign-return-address"="all" { + ret i32 0 +} + +;CHECK: CIE +;CHECK: Augmentation: "zRB" + +define i32 @bar() "sign-return-address"="all" "sign-return-address-key"="b_key" { + ret i32 0 +} + +;CHECK-NOT: CIE + +define i32 @baz() "sign-return-address"="all" nounwind { + ret i32 0 +} + +;CHECK-NOT: CIE + +define i32 @qux() "sign-return-address"="all" "sign-return-address-key"="b_key" nounwind { + ret i32 0 +} diff --git a/test/MC/ELF/cfi-b-key-frame.s b/test/MC/ELF/cfi-b-key-frame.s new file mode 100644 index 00000000000..3093bee7c59 --- /dev/null +++ b/test/MC/ELF/cfi-b-key-frame.s @@ -0,0 +1,6 @@ +// RUN: llvm-mc -filetype=obj -triple aarch64-arm-none-eabi %s -o - | llvm-dwarfdump - -v | FileCheck %s +#CHECK: Augmentation: "zRB" +f1: + .cfi_startproc + .cfi_b_key_frame + .cfi_endproc