-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[MC] Make .note.GNU-stack explicit for the trampoline case #151754
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.
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
Conversation
@llvm/pr-subscribers-mc @llvm/pr-subscribers-backend-aarch64 Author: None (ssijaric-nv) ChangesIn the presence of trampolines, the .note.GNU-stack section is not emitted. The The GNU ld 2.43 on x86 machines, for example, issues the following: missing .note.GNU-stack section implies executable stack On one of the ARM machines, the absence of .note.GNU-stack results in the stack STACK off 0x0000000000000000 vaddr 0x0000000000000000 paddr 0x0000000000000000 align 2**4 This change just emits the explicit .note.GNU-stack and marks it executable if required. Full diff: https://github.com/llvm/llvm-project/pull/151754.diff 6 Files Affected:
diff --git a/llvm/include/llvm/MC/MCAsmInfo.h b/llvm/include/llvm/MC/MCAsmInfo.h
index 6c12cd347901a..1e87ce23b158b 100644
--- a/llvm/include/llvm/MC/MCAsmInfo.h
+++ b/llvm/include/llvm/MC/MCAsmInfo.h
@@ -471,6 +471,12 @@ class LLVM_ABI MCAsmInfo {
return nullptr;
}
+ /// Targets can implement this method to specify a section to switch to if the
+ /// translation has trampolines that require an executable stack.
+ virtual MCSection *getExecutableStackSection(MCContext &Ctx) const {
+ return nullptr;
+ }
+
virtual const MCExpr *getExprForPersonalitySymbol(const MCSymbol *Sym,
unsigned Encoding,
MCStreamer &Streamer) const;
diff --git a/llvm/include/llvm/MC/MCAsmInfoELF.h b/llvm/include/llvm/MC/MCAsmInfoELF.h
index c05e4ad78ecd1..c5dff900227fd 100644
--- a/llvm/include/llvm/MC/MCAsmInfoELF.h
+++ b/llvm/include/llvm/MC/MCAsmInfoELF.h
@@ -16,6 +16,7 @@ namespace llvm {
class MCAsmInfoELF : public MCAsmInfo {
virtual void anchor();
MCSection *getNonexecutableStackSection(MCContext &Ctx) const final;
+ MCSection *getExecutableStackSection(MCContext &Ctx) const final;
void printSwitchToSection(const MCSection &, uint32_t, const Triple &,
raw_ostream &) const final;
bool useCodeAlign(const MCSection &Sec) const final;
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 1641c3eb535a9..cb697cd64d199 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -2840,9 +2840,13 @@ bool AsmPrinter::doFinalization(Module &M) {
// If we don't have any trampolines, then we don't require stack memory
// to be executable. Some targets have a directive to declare this.
Function *InitTrampolineIntrinsic = M.getFunction("llvm.init.trampoline");
+ MCSection *S = nullptr;
if (!InitTrampolineIntrinsic || InitTrampolineIntrinsic->use_empty())
- if (MCSection *S = MAI->getNonexecutableStackSection(OutContext))
- OutStreamer->switchSection(S);
+ S = MAI->getNonexecutableStackSection(OutContext);
+ else
+ S = MAI->getExecutableStackSection(OutContext);
+ if (S)
+ OutStreamer->switchSection(S);
if (TM.Options.EmitAddrsig) {
// Emit address-significance attributes for all globals.
diff --git a/llvm/lib/MC/MCAsmInfoELF.cpp b/llvm/lib/MC/MCAsmInfoELF.cpp
index cdae9d7860f33..495343a2c89d2 100644
--- a/llvm/lib/MC/MCAsmInfoELF.cpp
+++ b/llvm/lib/MC/MCAsmInfoELF.cpp
@@ -35,6 +35,14 @@ MCSection *MCAsmInfoELF::getNonexecutableStackSection(MCContext &Ctx) const {
return Ctx.getELFSection(".note.GNU-stack", ELF::SHT_PROGBITS, 0);
}
+MCSection *MCAsmInfoELF::getExecutableStackSection(MCContext &Ctx) const {
+ MCSectionELF *section =
+ static_cast<MCSectionELF *>(getNonexecutableStackSection(Ctx));
+ if (section)
+ section->setFlags(section->getFlags() | ELF::SHF_EXECINSTR);
+ return section;
+}
+
bool MCAsmInfoELF::useCodeAlign(const MCSection &Sec) const {
return static_cast<const MCSectionELF &>(Sec).getFlags() & ELF::SHF_EXECINSTR;
}
diff --git a/llvm/test/CodeGen/AArch64/trampoline.ll b/llvm/test/CodeGen/AArch64/trampoline.ll
index 0e682704afbf8..6689425bf5832 100644
--- a/llvm/test/CodeGen/AArch64/trampoline.ll
+++ b/llvm/test/CodeGen/AArch64/trampoline.ll
@@ -263,3 +263,4 @@ define i64 @func2() {
%fp = call ptr @llvm.adjust.trampoline(ptr @trampg)
ret i64 0
}
+; CHECK-LINUX: .section ".note.GNU-stack","x",@progbits
diff --git a/llvm/test/CodeGen/RISCV/rv64-trampoline.ll b/llvm/test/CodeGen/RISCV/rv64-trampoline.ll
index 34d46579518ea..69c905002032d 100644
--- a/llvm/test/CodeGen/RISCV/rv64-trampoline.ll
+++ b/llvm/test/CodeGen/RISCV/rv64-trampoline.ll
@@ -71,6 +71,7 @@ define i64 @test0(i64 %n, ptr %p) nounwind {
; RV64-LINUX-NEXT: ld ra, 56(sp) # 8-byte Folded Reload
; RV64-LINUX-NEXT: addi sp, sp, 64
; RV64-LINUX-NEXT: ret
+; RV64-LINUX: .section ".note.GNU-stack","x",@progbits
%alloca = alloca [32 x i8], align 8
call void @llvm.init.trampoline(ptr %alloca, ptr @f, ptr %p)
%tramp = call ptr @llvm.adjust.trampoline(ptr %alloca)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To quote the top of this file:
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
You need ; UTC_ARGS: --disable
before this to mark a region that UTC should not be managing. Maybe also best to put an enable back after it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @jrtc27. I'll update the test cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Same as AArch64, this is a UTC file
- This belongs outside the function
- Does non-Linux not need this too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @jrtc27. I updated the test cases.
llvm/include/llvm/MC/MCAsmInfo.h
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add a bool parameter to getNonexecutableStackSection
and rename it to getStackSection
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @MaskRay. I renamed the getNonexecutableStackSection.
llvm/tools/llvm-mc/llvm-mc.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Canonical way (preferred by clang-tidy) /*Exec=*/false
. ditto above
30b44ed
to
6050654
Compare
6050654
to
5f5a54b
Compare
In the presence of trampolines, the .note.GNU-stack section is not emitted. The absence of .note.GNU-stack results in the stack marked executable by some linkers. But others require an explict .note.GNU-stack section. The GNU ld 2.43 on x86 machines, for example, issues the following: missing .note.GNU-stack section implies executable stack NOTE: This behaviour is deprecated and will be removed in a future version of the linker On one of the ARM machines, the absence of .note.GNU-stack results in the stack marked as non-executable: STACK off 0x0000000000000000 vaddr 0x0000000000000000 paddr 0x0000000000000000 align 2**4 filesz 0x0000000000000000 memsz 0x0000000000000000 flags rw- This change just emits the explicit .note.GNU-stack and marks it executable.
5f5a54b
to
1309d6a
Compare
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/88/builds/16788 Here is the relevant piece of the build log for the reference
|
This is breaking the Mac build:
|
Thank you for letting me know, @arsenm. Is there a link to the buildbot log? I thought I removed all the traces of 'getNonexecutableStackSection'. I can revert while I investigate. |
In the presence of trampolines, the .note.GNU-stack section is not emitted. The absence of .note.GNU-stack results in the stack marked executable by some linkers. But others require an explict .note.GNU-stack section. The GNU ld 2.43 on x86 machines, for example, issues the following: missing .note.GNU-stack section implies executable stack NOTE: This behaviour is deprecated and will be removed in a future version of the linker On one of the ARM machines, the absence of .note.GNU-stack results in the stack marked as non-executable: STACK off 0x0000000000000000 vaddr 0x0000000000000000 paddr 0x0000000000000000 align 2**4 filesz 0x0000000000000000 memsz 0x0000000000000000 flags rw- This change just emits the explicit .note.GNU-stack and marks it executable if required.
Not sure there' a bot, we have fairly poor macOS bot coverage. touching those targets MCTargetDesc seems to have worked, so maybe there's a cmake issue somewhere? |
In the presence of trampolines, the .note.GNU-stack section is not emitted. The
absence of .note.GNU-stack results in the stack marked executable by some
linkers. But others require an explict .note.GNU-stack section.
The GNU ld 2.43 on x86 machines, for example, issues the following:
missing .note.GNU-stack section implies executable stack
NOTE: This behaviour is deprecated and will be removed in a future version of the linker
On one of the ARM machines, the absence of .note.GNU-stack results in the stack
marked as non-executable:
STACK off 0x0000000000000000 vaddr 0x0000000000000000 paddr 0x0000000000000000 align 2**4
filesz 0x0000000000000000 memsz 0x0000000000000000 flags rw-
This change just emits the explicit .note.GNU-stack and marks it executable if required.