https://github.com/androm3da updated https://github.com/llvm/llvm-project/pull/213820
>From a0e53fd0b677137066da12befcf72e5837701caa Mon Sep 17 00:00:00 2001 From: Brian Cain <[email protected]> Date: Mon, 3 Aug 2026 18:47:27 -0500 Subject: [PATCH] [Hexagon] Fix unusable SCS reg, make it selectable SCS hardcoded r19 as the shadow call stack pointer and required -ffixed-r19. That was the wrong register to pick: r19 is precisely the one the intended consumers cannot give up, so the feature was unusable in practice. * The Hexagon Linux kernel already reserves r19 for its thread-info pointer (arch/hexagon/Makefile: "TIR_NAME := r19", documented there as not configurable because it is hard-coded in several files). * hexagon-hypervisor reserves r20-r28 (kernel/CMakeLists.txt), with r28 bound to a register global (H2K_gp). That leaves h2 only r16-r19, so no single hardcoded choice can serve both consumers. Intersecting that with the callee-saved regs leaves r1{6,7,8}. So the new default is r18. * Add scs-reg-r{16..27} subtarget features * Add a -mscs-reg=<reg> flag. It's in m_Group instead of m_hexagon_Features_Group, since the latter is consumed by handleTargetFeaturesGroup() and would synthesize a bogus "+scs-reg=" feature. Assisted-by: Claude --- clang/docs/ReleaseNotes.md | 5 + clang/docs/ShadowCallStack.md | 16 +- clang/include/clang/Options/Options.td | 8 + clang/lib/Driver/SanitizerArgs.cpp | 57 +++++- clang/lib/Driver/ToolChains/Hexagon.cpp | 17 ++ .../fsanitize-shadow-call-stack-hexagon.c | 57 +++++- clang/test/Driver/hexagon-toolchain-linux.c | 12 +- llvm/lib/Target/Hexagon/Hexagon.td | 8 + .../Target/Hexagon/HexagonFrameLowering.cpp | 76 +++++--- llvm/lib/Target/Hexagon/HexagonSubtarget.cpp | 19 ++ llvm/lib/Target/Hexagon/HexagonSubtarget.h | 10 + .../test/CodeGen/Hexagon/shadow-call-stack.ll | 171 ++++++++++++------ 12 files changed, 362 insertions(+), 94 deletions(-) diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index a38b99ff8e075..d0db573e8b5bc 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -167,6 +167,11 @@ features cannot lower the translation-unit ABI level; the C++ standard library selected by the driver and the include directories added for it. +- Added `-mscs-reg=<reg>` on Hexagon to select which callee-saved register + (`r16`-`r27`, default `r18`) holds the shadow call stack pointer under + `-fsanitize=shadow-call-stack`. The selected register must also be reserved + with the matching `-ffixed-<reg>`. + ### Deprecated Compiler Flags ### Modified Compiler Flags diff --git a/clang/docs/ShadowCallStack.md b/clang/docs/ShadowCallStack.md index 62bfaa83040ea..9505fbe28be04 100644 --- a/clang/docs/ShadowCallStack.md +++ b/clang/docs/ShadowCallStack.md @@ -7,7 +7,7 @@ ## Introduction ShadowCallStack is an instrumentation pass, currently only implemented for -aarch64 and RISC-V, that protects programs against return address overwrites +aarch64, RISC-V and Hexagon, that protects programs against return address overwrites (e.g. stack buffer overflows.) It works by saving a function's return address to a separately allocated 'shadow call stack' in the function prolog in non-leaf functions and loading the return address from the shadow call stack @@ -71,6 +71,16 @@ principle, a platform could choose to reserve and use another register for ShadowCallStack, but this would be incompatible with the ABI standards published in AAPCS64 and the RISC-V psABI. +On Hexagon, `SCSReg` defaults to `r18` and is not fixed by the ABI, because +the Hexagon ABI does not designate a platform register. Any of the +callee-saved registers `r16`-`r27` may be used instead, selected with +`-mscs-reg=<reg>`; caller-saved registers cannot hold the pointer across a +call. The selected register must always be reserved with the matching +`-ffixed-<reg>`, and clang errors out if it is not. This flexibility exists +because different Hexagon environments have already claimed different +registers -- for example the Hexagon Linux kernel reserves `r19` for its +thread-info pointer, which is why `r18` rather than `r19` is the default. + Special unwind information is required on functions that are compiled with ShadowCallStack and that may be unwound, i.e. functions compiled with `-fexceptions` (which is the default in C++). Some unwinders (such as the @@ -141,6 +151,10 @@ However, it is important to disable GP relaxation in the linker when using the software based shadow call stack on RISC-V. This can be done with the `--no-relax-gp` flag in GNU ld, and is off by default in LLD. +On Hexagon you also need to reserve the shadow call stack pointer register, +i.e. `-ffixed-r18` for the default, or `-mscs-reg=<reg> -ffixed-<reg>` to use +a different one. + ### Low-level API #### `__has_feature(shadow_call_stack)` diff --git a/clang/include/clang/Options/Options.td b/clang/include/clang/Options/Options.td index 2467ebd0abe19..6ba4644f2b701 100644 --- a/clang/include/clang/Options/Options.td +++ b/clang/include/clang/Options/Options.td @@ -7172,6 +7172,14 @@ foreach i = {6-7} in HelpText<"Reserve register r"#i#" (Hexagon only)">; def ffixed_r19: Flag<["-"], "ffixed-r19">, Group<m_Group>, HelpText<"Reserve register r19 (Hexagon/x86_64 only; x86_64 requires APX EGPR)">; +// Deliberately in m_Group, not m_hexagon_Features_Group: the latter is swept +// up by handleTargetFeaturesGroup(), which would turn this into a bogus +// "+scs-reg=" target feature. It is translated explicitly in Hexagon.cpp. +def mhexagon_scs_reg : Joined<["-"], "mscs-reg=">, + Group<m_Group>, + HelpText<"Register holding the shadow call stack pointer: one of the " + "callee-saved registers r16-r27, default r18 (Hexagon only). " + "The chosen register must also be reserved with -ffixed-<reg>.">; } // let Flags = [TargetSpecific] def mmemops : Flag<["-"], "mmemops">, Group<m_hexagon_Features_Group>, Visibility<[ClangOption, CC1Option]>, diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp index c77ba78122a81..e1124ebc4d41f 100644 --- a/clang/lib/Driver/SanitizerArgs.cpp +++ b/clang/lib/Driver/SanitizerArgs.cpp @@ -163,6 +163,38 @@ static std::string describeSanitizeArg(const llvm::opt::Arg *A, /// Sanitizers set. static std::string toString(const clang::SanitizerSet &Sanitizers); +/// Map a Hexagon callee-saved register number (16-27) to its -ffixed-rN +/// option, used to check that the shadow call stack pointer is reserved. +static options::ID getHexagonFixedRegOption(unsigned RegNo) { + switch (RegNo) { + case 16: + return options::OPT_ffixed_r16; + case 17: + return options::OPT_ffixed_r17; + case 18: + return options::OPT_ffixed_r18; + case 19: + return options::OPT_ffixed_r19; + case 20: + return options::OPT_ffixed_r20; + case 21: + return options::OPT_ffixed_r21; + case 22: + return options::OPT_ffixed_r22; + case 23: + return options::OPT_ffixed_r23; + case 24: + return options::OPT_ffixed_r24; + case 25: + return options::OPT_ffixed_r25; + case 26: + return options::OPT_ffixed_r26; + case 27: + return options::OPT_ffixed_r27; + } + llvm_unreachable("not a Hexagon callee-saved register"); +} + /// Produce a string containing comma-separated names of sanitizers and /// sanitizer groups in \p Sanitizers set. static std::string toStringWithGroups(const clang::SanitizerSet &Sanitizers); @@ -784,11 +816,26 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC, } if ((Kinds & SanitizerKind::ShadowCallStack) && - TC.getTriple().getArch() == llvm::Triple::hexagon && - !Args.hasArg(options::OPT_ffixed_r19) && DiagnoseErrors) { - D.Diag(diag::err_drv_argument_only_allowed_with) - << lastArgumentForMask(D, Args, Kinds & SanitizerKind::ShadowCallStack) - << "-ffixed-r19"; + TC.getTriple().getArch() == llvm::Triple::hexagon && DiagnoseErrors) { + // The register holding the shadow call stack pointer must be reserved, so + // that neither the register allocator uses it nor the prologue saves and + // restores it as an ordinary callee-saved register. It defaults to r18 + // and is selectable with -mscs-reg=. + unsigned RegNo = 18; + if (Arg *A = Args.getLastArg(options::OPT_mhexagon_scs_reg)) { + StringRef Val(A->getValue()); + unsigned Parsed = 0; + // An out-of-range or malformed value is diagnosed by the toolchain; fall + // back to the default here so we do not emit a second, confusing error. + if (Val.consume_front("r") && !Val.getAsInteger(10, Parsed) && + Parsed >= 16 && Parsed <= 27) + RegNo = Parsed; + } + if (!Args.hasArg(getHexagonFixedRegOption(RegNo))) + D.Diag(diag::err_drv_argument_only_allowed_with) + << lastArgumentForMask(D, Args, + Kinds & SanitizerKind::ShadowCallStack) + << ("-ffixed-r" + Twine(RegNo)).str(); } // Report error if there are non-trapping sanitizers that require diff --git a/clang/lib/Driver/ToolChains/Hexagon.cpp b/clang/lib/Driver/ToolChains/Hexagon.cpp index 2ddc15ddbd818..76c5cafd77bc9 100644 --- a/clang/lib/Driver/ToolChains/Hexagon.cpp +++ b/clang/lib/Driver/ToolChains/Hexagon.cpp @@ -893,6 +893,23 @@ void HexagonToolChain::addClangTargetOptions(const ArgList &DriverArgs, CC1Args.push_back(Feature); } } + + // Select the shadow call stack pointer register. It has to hold a value + // across arbitrary calls, so only the callee-saved registers r16-r27 are + // allowed (Hexagon ABI, "Register usage across calls"). + if (Arg *A = DriverArgs.getLastArg(options::OPT_mhexagon_scs_reg)) { + StringRef Val(A->getValue()); + unsigned RegNo = 0; + if (!Val.consume_front("r") || Val.getAsInteger(10, RegNo) || RegNo < 16 || + RegNo > 27) { + getDriver().Diag(diag::err_drv_invalid_value) + << A->getSpelling() << A->getValue(); + } else { + CC1Args.push_back("-target-feature"); + CC1Args.push_back(DriverArgs.MakeArgString("+scs-reg-r" + Twine(RegNo))); + } + } + if (isAutoHVXEnabled(DriverArgs)) { CC1Args.push_back("-mllvm"); CC1Args.push_back("-hexagon-autohvx"); diff --git a/clang/test/Driver/fsanitize-shadow-call-stack-hexagon.c b/clang/test/Driver/fsanitize-shadow-call-stack-hexagon.c index 89d186315d9c1..ecab5e4d1ba82 100644 --- a/clang/test/Driver/fsanitize-shadow-call-stack-hexagon.c +++ b/clang/test/Driver/fsanitize-shadow-call-stack-hexagon.c @@ -1,12 +1,59 @@ -// Test that -fsanitize=shadow-call-stack on Hexagon requires -ffixed-r19. +// Test that -fsanitize=shadow-call-stack on Hexagon requires the register +// holding the shadow call stack pointer to be reserved, and that -mscs-reg= +// selects which register that is. // RUN: not %clang --target=hexagon-unknown-linux-musl \ // RUN: -fsanitize=shadow-call-stack %s -### 2>&1 \ -// RUN: | FileCheck %s --check-prefix=HEXAGON-SCS-NO-R19 +// RUN: | FileCheck %s --check-prefix=NO-FIXED // RUN: %clang --target=hexagon-unknown-linux-musl \ +// RUN: -fsanitize=shadow-call-stack -ffixed-r18 %s -### 2>&1 \ +// RUN: | FileCheck %s --check-prefix=DEFAULT + +/// Reserving some other register does not satisfy the requirement. +// RUN: not %clang --target=hexagon-unknown-linux-musl \ // RUN: -fsanitize=shadow-call-stack -ffixed-r19 %s -### 2>&1 \ -// RUN: | FileCheck %s --check-prefix=HEXAGON-SCS-WITH-R19 +// RUN: | FileCheck %s --check-prefix=NO-FIXED + +/// -mscs-reg= moves the requirement to the selected register. +// RUN: not %clang --target=hexagon-unknown-linux-musl \ +// RUN: -fsanitize=shadow-call-stack -mscs-reg=r16 -ffixed-r18 %s -### 2>&1 \ +// RUN: | FileCheck %s --check-prefix=NO-FIXED-R16 + +// RUN: %clang --target=hexagon-unknown-linux-musl \ +// RUN: -fsanitize=shadow-call-stack -mscs-reg=r16 -ffixed-r16 %s -### 2>&1 \ +// RUN: | FileCheck %s --check-prefix=SCS-R16 + +/// -mscs-reg= is accepted without the sanitizer, and still sets the feature. +// RUN: %clang --target=hexagon-unknown-linux-musl -mscs-reg=r27 %s -### 2>&1 \ +// RUN: | FileCheck %s --check-prefix=SCS-R27 + +/// Only the callee-saved registers r16-r27 can hold the pointer across calls. +// RUN: not %clang --target=hexagon-unknown-linux-musl \ +// RUN: -fsanitize=shadow-call-stack -mscs-reg=r15 -ffixed-r15 %s -### 2>&1 \ +// RUN: | FileCheck %s --check-prefix=BAD-REG + +// RUN: not %clang --target=hexagon-unknown-linux-musl \ +// RUN: -fsanitize=shadow-call-stack -mscs-reg=r28 -ffixed-r28 %s -### 2>&1 \ +// RUN: | FileCheck %s --check-prefix=BAD-REG-28 + +// RUN: not %clang --target=hexagon-unknown-linux-musl \ +// RUN: -fsanitize=shadow-call-stack -mscs-reg=sp -ffixed-r18 %s -### 2>&1 \ +// RUN: | FileCheck %s --check-prefix=BAD-REG-SP + +// NO-FIXED: error: invalid argument '-fsanitize=shadow-call-stack' only allowed with '-ffixed-r18' +// DEFAULT-DAG: "-target-feature" "+reserved-r18" +// DEFAULT-DAG: "-fsanitize=shadow-call-stack" + +// NO-FIXED-R16: error: invalid argument '-fsanitize=shadow-call-stack' only allowed with '-ffixed-r16' +// SCS-R16-DAG: "-target-feature" "+reserved-r16" +// SCS-R16-DAG: "-target-feature" "+scs-reg-r16" +// SCS-R16-DAG: "-fsanitize=shadow-call-stack" + +// SCS-R27: "-target-feature" "+scs-reg-r27" +/// -mscs-reg= must not also be swept into a target feature by its option group. +// SCS-R27-NOT: "+scs-reg=" -// HEXAGON-SCS-NO-R19: error: invalid argument '-fsanitize=shadow-call-stack' only allowed with '-ffixed-r19' -// HEXAGON-SCS-WITH-R19: "-fsanitize=shadow-call-stack" +// BAD-REG: error: invalid value 'r15' in '-mscs-reg=' +// BAD-REG-28: error: invalid value 'r28' in '-mscs-reg=' +// BAD-REG-SP: error: invalid value 'sp' in '-mscs-reg=' diff --git a/clang/test/Driver/hexagon-toolchain-linux.c b/clang/test/Driver/hexagon-toolchain-linux.c index 1365c6adc22eb..5f998d2244067 100644 --- a/clang/test/Driver/hexagon-toolchain-linux.c +++ b/clang/test/Driver/hexagon-toolchain-linux.c @@ -272,20 +272,20 @@ // RUN: -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \ // RUN: -mcpu=hexagonv60 \ // RUN: -fuse-ld=lld \ -// RUN: -fsanitize=shadow-call-stack -ffixed-r19 \ +// RUN: -fsanitize=shadow-call-stack -ffixed-r18 \ // RUN: --sysroot=%S/Inputs/basic_linux_libcxx_tree %s 2>&1 | FileCheck -check-prefix=CHECK-SCS %s // CHECK-SCS: "-L{{[^"]*}}basic_linux_libcxx_tree{{/|\\\\}}usr{{/|\\\\}}lib{{/|\\\\}}scs" // CHECK-SCS-SAME: "-L{{[^"]*}}basic_linux_libcxx_tree{{/|\\\\}}usr{{/|\\\\}}lib" // ----------------------------------------------------------------------------- -// Library paths: -ffixed-r19 alone must NOT select the scs multilib +// Library paths: -ffixed-r18 alone must NOT select the scs multilib // ----------------------------------------------------------------------------- // RUN: %clang -### --target=hexagon-unknown-linux-musl \ // RUN: -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \ // RUN: -mcpu=hexagonv60 \ // RUN: -fuse-ld=lld \ -// RUN: -ffixed-r19 \ -// RUN: --sysroot=%S/Inputs/basic_linux_libcxx_tree %s 2>&1 | FileCheck -check-prefix=CHECK-R19-ONLY %s -// CHECK-R19-ONLY-NOT: "-L{{.*}}{{/|\\\\}}scs" +// RUN: -ffixed-r18 \ +// RUN: --sysroot=%S/Inputs/basic_linux_libcxx_tree %s 2>&1 | FileCheck -check-prefix=CHECK-R18-ONLY %s +// CHECK-R18-ONLY-NOT: "-L{{.*}}{{/|\\\\}}scs" // ----------------------------------------------------------------------------- // Startup object: -fsanitize=shadow-call-stack links the scs crt1.o, not the // base crt1.o. Selection is on the multilib in effect, not file presence, so @@ -295,7 +295,7 @@ // RUN: -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \ // RUN: -mcpu=hexagonv60 \ // RUN: -fuse-ld=lld \ -// RUN: -fsanitize=shadow-call-stack -ffixed-r19 \ +// RUN: -fsanitize=shadow-call-stack -ffixed-r18 \ // RUN: --sysroot=%S/Inputs/basic_linux_libcxx_tree %s 2>&1 | FileCheck -check-prefix=CHECK-SCS-CRT %s // CHECK-SCS-CRT: "{{[^"]*}}basic_linux_libcxx_tree{{/|\\\\}}usr{{/|\\\\}}lib{{/|\\\\}}scs{{/|\\\\}}crt1.o" // CHECK-SCS-CRT-NOT: "{{[^"]*}}basic_linux_libcxx_tree{{/|\\\\}}usr{{/|\\\\}}lib{{/|\\\\}}crt1.o" diff --git a/llvm/lib/Target/Hexagon/Hexagon.td b/llvm/lib/Target/Hexagon/Hexagon.td index b6fe3b4c80777..7625307172abf 100644 --- a/llvm/lib/Target/Hexagon/Hexagon.td +++ b/llvm/lib/Target/Hexagon/Hexagon.td @@ -120,6 +120,14 @@ foreach i = {6-28} in def FeatureReservedR#i : SubtargetFeature<"reserved-r"#i, "UserReservedRegister[Hexagon::R"#i#"]", "true", "Reserve register R"#i>; +// Selects which register holds the shadow call stack pointer. Only the +// callee-saved registers (R16-R27, see the Hexagon ABI "Register usage across +// calls") can hold a value across a call, so only those are offered here. At +// most one may be selected; if none is, R18 is used (see getSCSPReg()). +foreach i = {16-27} in + def FeatureSCSRegR#i : SubtargetFeature<"scs-reg-r"#i, + "SCSPointerRegister[Hexagon::R"#i#"]", + "true", "Use register R"#i#" as the shadow call stack pointer">; def FeatureNoreturnStackElim: SubtargetFeature<"noreturn-stack-elim", "NoreturnStackElim", "true", "Eliminate stack allocation in a noreturn function when possible">; diff --git a/llvm/lib/Target/Hexagon/HexagonFrameLowering.cpp b/llvm/lib/Target/Hexagon/HexagonFrameLowering.cpp index 65922736b325f..7271a2b821d96 100644 --- a/llvm/lib/Target/Hexagon/HexagonFrameLowering.cpp +++ b/llvm/lib/Target/Hexagon/HexagonFrameLowering.cpp @@ -163,18 +163,31 @@ static void emitSCSPrologue(MachineFunction &MF, MachineBasicBlock &MBB, if (!HST.getFrameLowering()->hasFP(MF)) return; - Register SCSPReg = Hexagon::R19; - if (!MF.getSubtarget().isRegisterReservedByUser(SCSPReg)) - report_fatal_error("Must reserve r19 to use shadow call stack on Hexagon"); + // The shadow call stack pointer has to survive arbitrary calls, so it is + // always one of the callee-saved registers R16-R27 (Hexagon ABI, "Register + // usage across calls"). It must also be reserved: besides keeping the + // register allocator away from it, reserving it keeps it out of the + // callee-saved set, so it is never spilled and restored as an ordinary + // callee-saved register - which would leave the epilogue below reading the + // *caller's* shadow-stack slot. The spill stubs are handled separately in + // useSpillFunction()/useRestoreFunction(). + Register SCSPReg = HST.getSCSPReg(); + const auto &HRI = *HST.getRegisterInfo(); + if (!HST.isRegisterReservedByUser(SCSPReg)) + // Lower-cased to match the spelling of the -ffixed-<reg> flag the user + // needs to pass; TRI names the register "R18". + report_fatal_error(Twine("Must reserve ") + + StringRef(HRI.getName(SCSPReg)).lower() + + " to use shadow call stack on Hexagon"); const auto &HII = *HST.getInstrInfo(); - // r19 = add(r19, #4) + // SCSPReg = add(SCSPReg, #4) BuildMI(MBB, MI, DL, HII.get(Hexagon::A2_addi), SCSPReg) .addReg(SCSPReg) .addImm(4) .setMIFlag(MachineInstr::FrameSetup); - // memw(r19 + #-4) = r31 + // memw(SCSPReg + #-4) = r31 BuildMI(MBB, MI, DL, HII.get(Hexagon::S2_storeri_io)) .addReg(SCSPReg) .addImm(-4) @@ -188,8 +201,7 @@ static void emitSCSPrologue(MachineFunction &MF, MachineBasicBlock &MBB, // CFI: DW_CFA_val_expression for the SCS register, DW_OP_bregN -4 // Tells the unwinder that the SCS register at entry = current value - 4. - const auto &TRI = *MF.getSubtarget().getRegisterInfo(); - unsigned DwarfSCSReg = TRI.getDwarfRegNum(SCSPReg, /*IsEH=*/true); + unsigned DwarfSCSReg = HRI.getDwarfRegNum(SCSPReg, /*IsEH=*/true); // DW_OP_breg0..DW_OP_breg31 (0x70..0x8f) are 32 opcodes indexed by // register number, so the register number must fit in [0, 31]. assert(DwarfSCSReg < 32 && "SCS register should be < 32"); @@ -216,15 +228,16 @@ static void emitSCSEpilogue(MachineFunction &MF, MachineBasicBlock &MBB, if (!MF.getSubtarget<HexagonSubtarget>().getFrameLowering()->hasFP(MF)) report_fatal_error("SCS epilogue requires a frame"); - Register SCSPReg = Hexagon::R19; - const auto &HII = *MF.getSubtarget<HexagonSubtarget>().getInstrInfo(); + const auto &HST = MF.getSubtarget<HexagonSubtarget>(); + Register SCSPReg = HST.getSCSPReg(); + const auto &HII = *HST.getInstrInfo(); - // r31 = memw(r19 + #-4) + // r31 = memw(SCSPReg + #-4) BuildMI(MBB, MI, DL, HII.get(Hexagon::L2_loadri_io), Hexagon::R31) .addReg(SCSPReg) .addImm(-4) .setMIFlag(MachineInstr::FrameDestroy); - // r19 = add(r19, #-4) + // SCSPReg = add(SCSPReg, #-4) BuildMI(MBB, MI, DL, HII.get(Hexagon::A2_addi), SCSPReg) .addReg(SCSPReg) .addImm(-4) @@ -918,9 +931,8 @@ void HexagonFrameLowering::insertEpilogueInBlock(MachineBasicBlock &MBB) const { // Check for RESTORE_DEALLOC_RET* tail call. Don't emit an extra dealloc- // frame instruction if we encounter it. - // These spill-stub tail calls include r19 in their save range, but SCS - // requires -ffixed-r19, which prevents the allocator from selecting stubs - // that cover r19. The two features are therefore mutually exclusive and no + // These are restore stubs, which useRestoreFunction() never selects when SCS + // is active (they do deallocframe+jumpr, bypassing the SCS epilogue), so no // SCS epilogue is needed here. if (RetOpc == Hexagon::RESTORE_DEALLOC_RET_JMP_V4 || RetOpc == Hexagon::RESTORE_DEALLOC_RET_JMP_V4_PIC || @@ -958,15 +970,14 @@ void HexagonFrameLowering::insertEpilogueInBlock(MachineBasicBlock &MBB) const { if (!MF.getSubtarget<HexagonSubtarget>().isEnvironmentMusl() || !MF.getFunction().isVarArg()) { if (!NeedsDeallocframe) { - // RESTORE_DEALLOC_BEFORE_TAILCALL stubs include r19 in their save range, - // but SCS requires -ffixed-r19 which prevents the allocator from - // selecting stubs that cover r19, so SCS and stubs are mutually - // exclusive. PS_call_nr/PS_callr_nr are noreturn calls so the shadow - // stack entry is never read - no SCS epilogue is needed on either path. + // RESTORE_DEALLOC_BEFORE_TAILCALL is a restore stub, which + // useRestoreFunction() never selects when SCS is active. + // PS_call_nr/PS_callr_nr are noreturn calls so the shadow stack entry + // is never read - no SCS epilogue is needed on either path. if (NeedsSCS && PrevOpc != Hexagon::PS_call_nr && PrevOpc != Hexagon::PS_callr_nr) report_fatal_error("SCS with RESTORE_DEALLOC stub: " - "-ffixed-r19 should have prevented this"); + "useRestoreFunction() should have prevented this"); return; } // If the returning instruction is PS_jmpret, replace it with @@ -1014,9 +1025,9 @@ void HexagonFrameLowering::insertEpilogueInBlock(MachineBasicBlock &MBB) const { BuildMI(MBB, InsertPt, dl, HII.get(Hexagon::A2_addi), SP) .addReg(SP) .addImm(RegisterSavedAreaSizePlusPadding); - // RESTORE_DEALLOC stubs are mutually exclusive with SCS (-ffixed-r19 - // prevents stubs that cover r19), so only emit SCS epilogue when we - // emitted our own deallocframe above. + // RESTORE_DEALLOC stubs are never selected when SCS is active (see + // useRestoreFunction()), so only emit the SCS epilogue when we emitted + // our own deallocframe above. if (NeedsSCS && !HasRestoreStub) emitSCSEpilogue(MF, MBB, InsertPt, dl); } @@ -2991,6 +3002,25 @@ bool HexagonFrameLowering::useSpillFunction(const MachineFunction &MF, if (NumCSI <= 1) return false; + // Every spill stub saves the whole range starting at R16 + // (__save_r16_through_rNN), so a stub whose range reached the shadow call + // stack pointer register would spill it along with the real callee-saved + // registers - and since the SCS register is reserved it is absent from CSI, + // so the stub's fixed frame layout would not match the one the compiler + // assigned. + // + // shouldInlineCSR() above already makes this unreachable: it only lets a + // stub through when CSI is a contiguous run of double registers starting at + // D8, and reserving the SCS register always breaks the double it belongs + // to, leaving its partner in CSI as a lone single register. This is a + // cheap safety net so the guarantee does not rest on that reasoning alone. + if (MF.getFunction().hasFnAttribute(Attribute::ShadowCallStack)) { + const auto &HST = MF.getSubtarget<HexagonSubtarget>(); + Register MaxReg = getMaxCalleeSavedReg(CSI, *HST.getRegisterInfo()); + if (HST.getSCSPReg().id() <= MaxReg.id()) + return false; + } + unsigned Threshold = isOptSize(MF) ? SpillFuncThresholdOs : SpillFuncThreshold; return Threshold < NumCSI; diff --git a/llvm/lib/Target/Hexagon/HexagonSubtarget.cpp b/llvm/lib/Target/Hexagon/HexagonSubtarget.cpp index 66c8b0a67169d..b8838dc54da33 100644 --- a/llvm/lib/Target/Hexagon/HexagonSubtarget.cpp +++ b/llvm/lib/Target/Hexagon/HexagonSubtarget.cpp @@ -138,6 +138,25 @@ HexagonSubtarget::initializeSubtargetDependencies(StringRef CPU, StringRef FS) { std::string FeatureString = Features.getString(); ParseSubtargetFeatures(CPUString, /*TuneCPU*/ CPUString, FeatureString); + // Resolve the shadow call stack pointer register. At most one "scs-reg-rN" + // feature may be given; R18 is the default. R18 is chosen because it is the + // lowest callee-saved register that neither the Hexagon Linux kernel (which + // reserves R19 for the thread-info pointer) nor code that reserves the upper + // callee-saved range already claims. + static_assert(Hexagon::R27 - Hexagon::R16 == 11, + "Callee-saved R16-R27 are assumed to be consecutive"); + SCSPReg = Hexagon::R18; + bool SCSRegSelected = false; + for (unsigned Reg = Hexagon::R16; Reg <= Hexagon::R27; ++Reg) { + if (!SCSPointerRegister[Reg]) + continue; + if (SCSRegSelected) + report_fatal_error( + "Only one shadow call stack pointer register may be selected"); + SCSPReg = Reg; + SCSRegSelected = true; + } + if (useHVXV68Ops()) UseHVXFloatingPoint = UseHVXIEEEFPOps || UseHVXQFloatOps; diff --git a/llvm/lib/Target/Hexagon/HexagonSubtarget.h b/llvm/lib/Target/Hexagon/HexagonSubtarget.h index 2019bbf64b365..2b298f5adfa52 100644 --- a/llvm/lib/Target/Hexagon/HexagonSubtarget.h +++ b/llvm/lib/Target/Hexagon/HexagonSubtarget.h @@ -65,8 +65,13 @@ class HexagonSubtarget : public HexagonGenSubtargetInfo { bool HasMemNoShuf = false; bool EnableDuplex = false; std::bitset<Hexagon::NUM_TARGET_REGS> UserReservedRegister; + std::bitset<Hexagon::NUM_TARGET_REGS> SCSPointerRegister; bool NoreturnStackElim = false; + /// Register holding the shadow call stack pointer, resolved from + /// SCSPointerRegister in initializeSubtargetDependencies(). + Register SCSPReg; + public: Hexagon::ArchEnum HexagonArchVersion; Hexagon::ArchEnum HexagonHVXVersion = Hexagon::ArchEnum::NoArch; @@ -291,6 +296,11 @@ class HexagonSubtarget : public HexagonGenSubtargetInfo { assert(i.id() < Hexagon::NUM_TARGET_REGS && "Register out of range"); return UserReservedRegister[i.id()]; } + + /// Returns the register that holds the shadow call stack pointer. Defaults + /// to R18, overridable with the "scs-reg-rN" subtarget features. + Register getSCSPReg() const { return SCSPReg; } + bool usePredicatedCalls() const; bool noreturnStackElim() const { return NoreturnStackElim; } diff --git a/llvm/test/CodeGen/Hexagon/shadow-call-stack.ll b/llvm/test/CodeGen/Hexagon/shadow-call-stack.ll index ddcca760cd474..3573a7910adc2 100644 --- a/llvm/test/CodeGen/Hexagon/shadow-call-stack.ll +++ b/llvm/test/CodeGen/Hexagon/shadow-call-stack.ll @@ -1,40 +1,74 @@ -; RUN: llc -mtriple=hexagon -mattr=+reserved-r19 < %s | FileCheck %s -;; Test that the backend fatally errors without reserved-r19 (backstop for -;; the driver diagnostic in SanitizerArgs.cpp). +;; r18 is the default shadow call stack pointer register; reserving it is all +;; that is required. +; RUN: llc -mtriple=hexagon -mattr=+reserved-r18 < %s | FileCheck %s +; RUN: llc -mtriple=hexagon -mattr=+reserved-r18 < %s | FileCheck %s --check-prefix=CFI +; RUN: llc -mtriple=hexagon-unknown-linux-musl -mattr=+reserved-r18 < %s | FileCheck %s --check-prefix=MUSL + +;; The backend fatally errors unless the SCS register is reserved (backstop for +;; the driver diagnostic in SanitizerArgs.cpp). Reserving some other register +;; does not help. ; RUN: not --crash llc -mtriple=hexagon < %s 2>&1 | FileCheck %s --check-prefix=ERR -; RUN: llc -mtriple=hexagon -mattr=+reserved-r19 < %s | FileCheck %s --check-prefix=CFI -; RUN: llc -mtriple=hexagon-unknown-linux-musl -mattr=+reserved-r19 < %s | FileCheck %s --check-prefix=MUSL +; RUN: not --crash llc -mtriple=hexagon -mattr=+reserved-r19 < %s 2>&1 | FileCheck %s --check-prefix=ERR + +;; scs-reg-rN selects a different register; the diagnostic follows it. +; RUN: llc -mtriple=hexagon -mattr=+scs-reg-r16,+reserved-r16 < %s \ +; RUN: | FileCheck %s --check-prefix=R16 +; RUN: not --crash llc -mtriple=hexagon -mattr=+scs-reg-r16,+reserved-r18 < %s 2>&1 \ +; RUN: | FileCheck %s --check-prefix=ERR16 +; RUN: not --crash llc -mtriple=hexagon -mattr=+scs-reg-r16,+scs-reg-r17 < %s 2>&1 \ +; RUN: | FileCheck %s --check-prefix=ERRMULTI + +;; Only one scs-reg-rN may be given. +; ERRMULTI: Only one shadow call stack pointer register may be selected + +;; With r16 selected, the prologue/epilogue use r16 and leave r18 alone. +; R16-LABEL: nonleaf: +; R16: r16 = add(r16,#4) +; R16: call bar +; R16: memw(r16+#-4) = r31 +; R16: { +; R16-DAG: r16 = add(r16,#-4) +; R16-DAG: r31 = memw(r16+#-4) +; R16: } +; R16: jumpr r31 + +;; Every spill stub saves the range starting at r16, so with r16 as the SCS +;; register no stub can ever be used - reserving r16 removes d8 from the +;; callee-saved set entirely. +; R16-LABEL: minsize_multicall: +; R16-NOT: __save_ +; R16-NOT: __restore_ -;; Leaf function - no LR spill, SCS should not emit any r19 instructions. +;; Leaf function - no LR spill, SCS should not emit any r18 instructions. ; CHECK-LABEL: leaf: -; CHECK-NOT: r19 +; CHECK-NOT: r18 ; CHECK: jumpr r31 ;; Non-leaf function - SCS emits prologue (addi + store) and epilogue (load + addi). ;; The SCS store is fused into the same packet as the first call; because ;; Hexagon packets use old-value reads the original R31 is saved regardless. ;; The epilogue load and addi are also in the same packet; the load uses the -;; old (pre-decrement) r19 value per Hexagon packet semantics, and the -4 +;; old (pre-decrement) r18 value per Hexagon packet semantics, and the -4 ;; offset correctly addresses the saved slot. ; CHECK-LABEL: nonleaf: -; CHECK: r19 = add(r19,#4) +; CHECK: r18 = add(r18,#4) ; CHECK: call bar -; CHECK: memw(r19+#-4) = r31 +; CHECK: memw(r18+#-4) = r31 ; CHECK: { -; CHECK-DAG: r19 = add(r19,#-4) -; CHECK-DAG: r31 = memw(r19+#-4) +; CHECK-DAG: r18 = add(r18,#-4) +; CHECK-DAG: r31 = memw(r18+#-4) ; CHECK: } ; CHECK: jumpr r31 ;; Multi-call function - only one SCS prologue/epilogue pair, not one per call. ; CHECK-LABEL: twocalls: -; CHECK: r19 = add(r19,#4) +; CHECK: r18 = add(r18,#4) ; CHECK: call bar -; CHECK: memw(r19+#-4) = r31 +; CHECK: memw(r18+#-4) = r31 ; CHECK: call bar ; CHECK: { -; CHECK-DAG: r19 = add(r19,#-4) -; CHECK-DAG: r31 = memw(r19+#-4) +; CHECK-DAG: r18 = add(r18,#-4) +; CHECK-DAG: r31 = memw(r18+#-4) ; CHECK: } ; CHECK: jumpr r31 @@ -42,44 +76,44 @@ ;; has no SCS prologue/epilogue. The call path gets the SCS pair. ; CHECK-LABEL: condcall: ; CHECK: if (!p0.new) jumpr:nt r31 -; CHECK: r19 = add(r19,#4) +; CHECK: r18 = add(r18,#4) ; CHECK: call bar -; CHECK: memw(r19+#-4) = r31 +; CHECK: memw(r18+#-4) = r31 ; CHECK: { -; CHECK-DAG: r19 = add(r19,#-4) -; CHECK-DAG: r31 = memw(r19+#-4) +; CHECK-DAG: r18 = add(r18,#-4) +; CHECK-DAG: r31 = memw(r18+#-4) ; CHECK: } ; CHECK: jumpr r31 ;; Tail call - SCS prologue and epilogue are both emitted; the epilogue ;; instructions and the tail jump are fused into the same packet. ; CHECK-LABEL: tailcall: -; CHECK: r19 = add(r19,#4) -; CHECK: memw(r19+#-4) = r31 +; CHECK: r18 = add(r18,#4) +; CHECK: memw(r18+#-4) = r31 ; CHECK: { -; CHECK-DAG: r19 = add(r19,#-4) -; CHECK-DAG: r31 = memw(r19+#-4) +; CHECK-DAG: r18 = add(r18,#-4) +; CHECK-DAG: r31 = memw(r18+#-4) ; CHECK-DAG: jump bar ; CHECK: } ;; Noreturn call - SCS prologue is emitted but no SCS epilogue since the ;; function never returns. ; CHECK-LABEL: noret: -; CHECK: r19 = add(r19,#4) -; CHECK: memw(r19+#-4) = r31 +; CHECK: r18 = add(r18,#4) +; CHECK: memw(r18+#-4) = r31 ; CHECK-NOT: r31 = memw -; CHECK-NOT: r19 = add(r19,#-4) +; CHECK-NOT: r18 = add(r18,#-4) ; CHECK-LABEL: nonleaf_cfi: ;; Minsize + multiple callee-saved registers: the restore stub ;; (__restore_r16_through_r17_and_deallocframe) must NOT be used when SCS is ;; active because it performs deallocframe+jumpr without the SCS epilogue. ; CHECK-LABEL: minsize_multicall: -; CHECK: r19 = add(r19,#4) -; CHECK: memw(r19+#-4) = r31 +; CHECK: r18 = add(r18,#4) +; CHECK: memw(r18+#-4) = r31 ; CHECK: { -; CHECK-DAG: r19 = add(r19,#-4) -; CHECK-DAG: r31 = memw(r19+#-4) +; CHECK-DAG: r18 = add(r18,#-4) +; CHECK-DAG: r31 = memw(r18+#-4) ; CHECK: } ; CHECK-NOT: __restore_ ; CHECK: jumpr r31 @@ -88,49 +122,60 @@ ;; stub (__restore_r16_through_r17_and_deallocframe_before_tailcall) must NOT be ;; used when SCS is active. The SCS epilogue and tail jump are fused together. ; CHECK-LABEL: minsize_tailcall: -; CHECK: r19 = add(r19,#4) -; CHECK: memw(r19+#-4) = r31 +; CHECK: r18 = add(r18,#4) +; CHECK: memw(r18+#-4) = r31 ; CHECK: { -; CHECK-DAG: r19 = add(r19,#-4) -; CHECK-DAG: r31 = memw(r19+#-4) +; CHECK-DAG: r18 = add(r18,#-4) +; CHECK-DAG: r31 = memw(r18+#-4) ; CHECK-DAG: jump bar ; CHECK: } ; CHECK-NOT: __restore_ ;; Multiple return paths - each exit block gets its own SCS epilogue. ; CHECK-LABEL: multi_return: -; CHECK: r19 = add(r19,#4) -; CHECK: memw(r19+#-4) = r31 -; CHECK: r31 = memw(r19+#-4) -; CHECK: r19 = add(r19,#-4) +; CHECK: r18 = add(r18,#4) +; CHECK: memw(r18+#-4) = r31 +; CHECK: r31 = memw(r18+#-4) +; CHECK: r18 = add(r18,#-4) ; CHECK: jumpr r31 -; CHECK: r31 = memw(r19+#-4) -; CHECK: r19 = add(r19,#-4) +; CHECK: r31 = memw(r18+#-4) +; CHECK: r18 = add(r18,#-4) ; CHECK: jumpr r31 -;; Without r19 reserved, SCS should report an error. -; ERR: Must reserve r19 to use shadow call stack on Hexagon +;; A minsize function using many callee-saved registers - without SCS this is +;; the shape that gets a spill/restore stub. Reserving the SCS register breaks +;; the r19:18 double, which leaves r19 in the callee-saved set as a lone single +;; register and forces inline spills, so no stub covering r18 can be selected. +;; This locks in the invariant that a stub never reaches the SCS register. +; CHECK-LABEL: minsize_manycsr: +; CHECK-NOT: __save_ +; CHECK-NOT: __restore_ +; CHECK: jumpr r31 + +;; Without the SCS register reserved, SCS should report an error naming it. +; ERR: Must reserve r18 to use shadow call stack on Hexagon +; ERR16: Must reserve r16 to use shadow call stack on Hexagon -;; Non-leaf with uwtable - exercises CFI escape (DW_CFA_val_expression for r19) +;; Non-leaf with uwtable - exercises CFI escape (DW_CFA_val_expression for r18) ;; and cfi_restore on epilogue. ; CFI-LABEL: nonleaf_cfi: -; CFI: r19 = add(r19,#4) -; CFI: memw(r19+#-4) = r31 -; CFI: .cfi_escape 0x16, 0x13, 0x02, 0x83, 0x7c +; CFI: r18 = add(r18,#4) +; CFI: memw(r18+#-4) = r31 +; CFI: .cfi_escape 0x16, 0x12, 0x02, 0x82, 0x7c ; CFI: { -; CFI-DAG: r31 = memw(r19+#-4) -; CFI-DAG: r19 = add(r19,#-4) +; CFI-DAG: r31 = memw(r18+#-4) +; CFI-DAG: r18 = add(r18,#-4) ; CFI: } -; CFI: .cfi_restore r19 +; CFI: .cfi_restore r18 ; CFI: jumpr r31 ;; Musl vararg - exercises the vararg epilogue path with SCS. ; MUSL-LABEL: vararg_musl: -; MUSL: r19 = add(r19,#4) -; MUSL: memw(r19+#-4) = r31 +; MUSL: r18 = add(r18,#4) +; MUSL: memw(r18+#-4) = r31 ; MUSL: { -; MUSL-DAG: r19 = add(r19,#-4) -; MUSL-DAG: r31 = memw(r19+#-4) +; MUSL-DAG: r18 = add(r18,#-4) +; MUSL-DAG: r31 = memw(r18+#-4) ; MUSL: } ; MUSL: jumpr r31 @@ -212,3 +257,21 @@ neg: %r2 = call i32 @foo(i32 0) ret i32 %r2 } + +define i32 @minsize_manycsr(i32 %x) shadowcallstack nounwind minsize + "disable-tail-calls"="true" { + %a = call i32 @foo(i32 %x) + %b = call i32 @foo(i32 %a) + %c = call i32 @foo(i32 %b) + %d = call i32 @foo(i32 %c) + %e = call i32 @foo(i32 %d) + %f = call i32 @foo(i32 %e) + %g = call i32 @foo(i32 %f) + %s1 = add i32 %a, %b + %s2 = add i32 %s1, %c + %s3 = add i32 %s2, %d + %s4 = add i32 %s3, %e + %s5 = add i32 %s4, %f + %s6 = add i32 %s5, %g + ret i32 %s6 +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
