-
Notifications
You must be signed in to change notification settings - Fork 14.3k
draft: inline asm mode #146215
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
base: main
Are you sure you want to change the base?
draft: inline asm mode #146215
Conversation
@llvm/pr-subscribers-backend-x86 @llvm/pr-subscribers-mc Author: Brian Cain (androm3da) ChangesThis is a feature inspired by a rust lint-run-amok: rust-lang/rust#143021 Could we (and should we?) generalize this for the sake of all frontends who want to avoid the problems caused by global/public labels in inline assembly? I'm not even 100% sure that I captured the rationale here but I took a stab at it. Full diff: https://github.com/llvm/llvm-project/pull/146215.diff 6 Files Affected:
diff --git a/llvm/docs/InlineAsmSafetyDirective.md b/llvm/docs/InlineAsmSafetyDirective.md
new file mode 100644
index 0000000000000..0285020cbf6f1
--- /dev/null
+++ b/llvm/docs/InlineAsmSafetyDirective.md
@@ -0,0 +1,110 @@
+# Inline Assembly Safety Directive
+
+## Overview
+
+The `.inline_asm_mode` directive provides enhanced safety for inline assembly blocks by warning about potentially unsafe label usage. This directive helps prevent common errors where programmers create non-local labels in inline assembly that could be inadvertently jumped to from external code.
+
+## Syntax
+
+```assembly
+.inline_asm_mode strict # Enable strict mode - warn on non-local labels
+.inline_asm_mode relaxed # Disable strict mode (default)
+```
+
+## Description
+
+When `.inline_asm_mode strict` is active, the assembler will emit warnings for labels that are considered potentially unsafe for inline assembly:
+
+- **Safe labels** (no warnings):
+ - Local labels starting with `.L` (e.g., `.L_loop`, `.L_end`)
+ - Numeric labels (e.g., `1:`, `42:`)
+ - Labels starting with special prefixes (`$`, `__`)
+ - Labels starting with `.` (local scope)
+
+- **Unsafe labels** (warnings emitted):
+ - Global labels without special prefixes (e.g., `my_function:`, `loop:`)
+ - Labels that could be accessed from outside the inline assembly block
+
+## Use Cases
+
+### Frontend Integration
+
+Compiler frontends can use this directive when generating inline assembly:
+
+```c++
+// Emitted by the compiler: .inline_asm_mode strict
+// C++ inline assembly example
+asm(
+ ".L_loop:\n" // Safe - no warning
+ " add %0, %1\n"
+ " jne .L_loop\n" // Safe - local jump
+ "exit:\n" // Warning
+ : "=r"(result) : "r"(input));
+```
+// Emitted by the compiler: .inline_asm_mode relaxed
+
+## Rationale
+
+Inline assembly blocks are often embedded within larger functions or modules. Non-local labels in these blocks can create several problems:
+
+1. **Naming conflicts**: Global labels may conflict with other symbols in the compilation unit
+2. **Unintended control flow**: External code might accidentally jump to labels intended for internal use
+3. **Maintenance issues**: Global labels make inline assembly less encapsulated
+
+The strict mode helps identify these potential issues during compilation, allowing developers to use safer local labels instead.
+
+## Error Handling
+
+Invalid directive usage will produce parse errors:
+
+```assembly
+.inline_asm_mode invalid_mode
+# Error: expected 'strict' or 'relaxed'
+
+.inline_asm_mode
+# Error: expected 'strict' or 'relaxed' after '.inline_asm_mode'
+```
+
+## Implementation Details
+
+- The directive affects only subsequent label definitions until changed
+- Default mode is `relaxed` (no additional warnings)
+- The directive state is maintained in the MC streamer
+- Warnings are emitted through the standard LLVM diagnostic system
+
+## Examples
+
+### Complete Example
+
+```assembly
+.text
+.globl example_function
+example_function:
+ # Regular function labels (outside inline asm) - no warnings
+
+ # Simulate inline assembly block with safety
+ .inline_asm_mode strict
+
+ # These are safe
+ .L_inline_start:
+ mov $1, %eax
+ test %eax, %eax
+ jz .L_inline_end
+
+ 1: # Numeric label
+ inc %eax
+ cmp $10, %eax
+ jl 1b
+
+ .L_inline_end:
+ # End of safe inline block
+
+ # This would generate a warning
+ # global_inline_label: # Warning would be emitted
+
+ .inline_asm_mode relaxed
+
+ # Back to normal mode
+ ret
+```
+
diff --git a/llvm/include/llvm/MC/MCStreamer.h b/llvm/include/llvm/MC/MCStreamer.h
index 8f2e137ea0c84..834425a17c723 100644
--- a/llvm/include/llvm/MC/MCStreamer.h
+++ b/llvm/include/llvm/MC/MCStreamer.h
@@ -260,6 +260,11 @@ class LLVM_ABI MCStreamer {
/// discussion for future inclusion.
bool AllowAutoPadding = false;
+ /// Is strict inline assembly mode enabled? When enabled, the assembler
+ /// will warn about non-local labels that could be unsafe jump targets
+ /// in inline assembly blocks.
+ bool InlineAsmStrictMode = false;
+
protected:
// Symbol of the current epilog for which we are processing SEH directives.
WinEH::FrameInfo::Epilog *CurrentWinEpilog = nullptr;
@@ -325,6 +330,9 @@ class LLVM_ABI MCStreamer {
void setAllowAutoPadding(bool v) { AllowAutoPadding = v; }
bool getAllowAutoPadding() const { return AllowAutoPadding; }
+ void setInlineAsmMode(bool v) { InlineAsmStrictMode = v; }
+ bool getInlineAsmMode() const { return InlineAsmStrictMode; }
+
MCSymbol *emitLineTableLabel();
/// When emitting an object file, create and emit a real label. When emitting
diff --git a/llvm/lib/MC/MCParser/AsmParser.cpp b/llvm/lib/MC/MCParser/AsmParser.cpp
index bb8c45bd901cd..131f1b8de98af 100644
--- a/llvm/lib/MC/MCParser/AsmParser.cpp
+++ b/llvm/lib/MC/MCParser/AsmParser.cpp
@@ -533,6 +533,7 @@ class AsmParser : public MCAsmParser {
DK_LTO_SET_CONDITIONAL,
DK_CFI_MTE_TAGGED_FRAME,
DK_MEMTAG,
+ DK_INLINE_ASM_MODE,
DK_END
};
@@ -703,6 +704,9 @@ class AsmParser : public MCAsmParser {
// ".lto_discard"
bool parseDirectiveLTODiscard();
+ // ".inline_asm_mode"
+ bool parseDirectiveInlineAsmMode(SMLoc DirectiveLoc);
+
// Directives to support address-significance tables.
bool parseDirectiveAddrsig();
bool parseDirectiveAddrsigSym();
@@ -2222,6 +2226,8 @@ bool AsmParser::parseStatement(ParseStatementInfo &Info,
return parseDirectiveLTODiscard();
case DK_MEMTAG:
return parseDirectiveSymbolAttribute(MCSA_Memtag);
+ case DK_INLINE_ASM_MODE:
+ return parseDirectiveInlineAsmMode(IDLoc);
}
return Error(IDLoc, "unknown directive");
@@ -5590,6 +5596,7 @@ void AsmParser::initializeDirectiveKindMap() {
DirectiveKindMap[".lto_discard"] = DK_LTO_DISCARD;
DirectiveKindMap[".lto_set_conditional"] = DK_LTO_SET_CONDITIONAL;
DirectiveKindMap[".memtag"] = DK_MEMTAG;
+ DirectiveKindMap[".inline_asm_mode"] = DK_INLINE_ASM_MODE;
}
MCAsmMacro *AsmParser::parseMacroLikeBody(SMLoc DirectiveLoc) {
@@ -5912,6 +5919,26 @@ bool AsmParser::parseDirectiveLTODiscard() {
return parseMany(ParseOp);
}
+/// parseDirectiveInlineAsmMode
+/// ::= ".inline_asm_mode" ( "strict" | "relaxed" )
+bool AsmParser::parseDirectiveInlineAsmMode(SMLoc DirectiveLoc) {
+ if (getLexer().isNot(AsmToken::Identifier)) {
+ return Error(DirectiveLoc, "expected 'strict' or 'relaxed' after '.inline_asm_mode'");
+ }
+
+ StringRef Mode = getTok().getIdentifier();
+ if (Mode == "strict") {
+ getStreamer().setInlineAsmMode(true);
+ } else if (Mode == "relaxed") {
+ getStreamer().setInlineAsmMode(false);
+ } else {
+ return Error(getTok().getLoc(), "expected 'strict' or 'relaxed'");
+ }
+
+ Lex(); // consume mode identifier
+ return false;
+}
+
// We are comparing pointers, but the pointers are relative to a single string.
// Thus, this should always be deterministic.
static int rewritesSort(const AsmRewrite *AsmRewriteA,
diff --git a/llvm/lib/MC/MCStreamer.cpp b/llvm/lib/MC/MCStreamer.cpp
index 6cd6b4abdd327..da2ab9a408257 100644
--- a/llvm/lib/MC/MCStreamer.cpp
+++ b/llvm/lib/MC/MCStreamer.cpp
@@ -400,6 +400,18 @@ void MCStreamer::emitLabel(MCSymbol *Symbol, SMLoc Loc) {
return getContext().reportError(Loc, "symbol '" + Twine(Symbol->getName()) +
"' is already defined");
+ if (InlineAsmStrictMode) {
+ StringRef Name = Symbol->getName();
+ if (!Name.empty() && !Name.starts_with(".L") && !Name.starts_with("L..") &&
+ !Name.starts_with("$") && !Name.starts_with("__") && Name.front() != '.' &&
+ !std::isdigit(Name.front())) {
+ getContext().reportWarning(
+ Loc, "non-local label '" + Name +
+ "' in inline assembly strict mode may be unsafe for "
+ "external jumps; consider using local labels (.L*) instead");
+ }
+ }
+
assert(!Symbol->isVariable() && "Cannot emit a variable symbol!");
assert(getCurrentSectionOnly() && "Cannot emit before setting section!");
assert(!Symbol->getFragment() && "Unexpected fragment on symbol data!");
diff --git a/llvm/test/MC/X86/inline-asm-mode-basic.s b/llvm/test/MC/X86/inline-asm-mode-basic.s
new file mode 100644
index 0000000000000..e15194980c8f7
--- /dev/null
+++ b/llvm/test/MC/X86/inline-asm-mode-basic.s
@@ -0,0 +1,25 @@
+# RUN: llvm-mc -triple x86_64-unknown-unknown %s 2>&1 | FileCheck %s
+
+# Test basic .inline_asm_mode directive functionality
+
+.text
+
+# Test that the directive is parsed correctly
+.inline_asm_mode strict
+.inline_asm_mode relaxed
+
+# Test strict mode warnings
+.inline_asm_mode strict
+
+# This should produce a warning
+# CHECK: warning: non-local label 'unsafe_global' in inline assembly strict mode may be unsafe for external jumps; consider using local labels (.L*) instead
+unsafe_global:
+ nop
+
+# This should not warn (local label)
+.L_safe_local:
+ nop
+
+# Test error handling
+.inline_asm_mode invalid
+# CHECK: error: expected 'strict' or 'relaxed'
\ No newline at end of file
diff --git a/llvm/test/MC/X86/inline-asm-mode-directive.s b/llvm/test/MC/X86/inline-asm-mode-directive.s
new file mode 100644
index 0000000000000..534948e81081c
--- /dev/null
+++ b/llvm/test/MC/X86/inline-asm-mode-directive.s
@@ -0,0 +1,57 @@
+# RUN: llvm-mc -triple x86_64-unknown-unknown %s 2>&1 | FileCheck %s --check-prefix=RELAX
+# RUN: llvm-mc -triple x86_64-unknown-unknown %s 2>&1 | FileCheck %s --check-prefix=STRICT
+
+# Test the .inline_asm_mode directive for safer inline assembly label handling
+
+.text
+
+# Test relaxed mode (default) - no warnings
+.inline_asm_mode relaxed
+
+# These labels should not produce warnings in relaxed mode
+my_label:
+ nop
+global_symbol:
+ nop
+.L_local_label:
+ nop
+
+# RELAX-NOT: warning
+
+# Test strict mode - should warn about non-local labels
+.inline_asm_mode strict
+
+# Local labels - should not warn
+.L_local1:
+ nop
+.L_local_with_numbers_123:
+ nop
+1:
+ nop
+42:
+ nop
+
+# Non-local labels - should warn
+# STRICT: :[[@LINE+1]]:1: warning: non-local label 'unsafe_label' in inline assembly strict mode may be unsafe for external jumps; consider using local labels (.L*) instead
+unsafe_label:
+ nop
+
+# STRICT: :[[@LINE+1]]:1: warning: non-local label 'another_global' in inline assembly strict mode may be unsafe for external jumps; consider using local labels (.L*) instead
+another_global:
+ nop
+
+# Switch back to relaxed mode
+.inline_asm_mode relaxed
+
+# This should not warn again
+yet_another_label:
+ nop
+
+# RELAX-NOT: warning
+
+# Test error cases
+.inline_asm_mode invalid_mode
+# CHECK: :[[@LINE-1]]:18: error: expected 'strict' or 'relaxed'
+
+.inline_asm_mode
+# CHECK: :[[@LINE-1]]:17: error: expected 'strict' or 'relaxed' after '.inline_asm_mode'
\ No newline at end of file
|
|
||
```assembly | ||
.inline_asm_mode strict # Enable strict mode - warn on non-local labels | ||
.inline_asm_mode relaxed # Disable strict mode (default) |
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.
I'm especially willing to consider a better name for this directive.
✅ With the latest revision this PR passed the C/C++ code formatter. |
d30a5f5
to
6b622b3
Compare
- Local labels starting with `.L` (e.g., `.L_loop`, `.L_end`) | ||
- Numeric labels (e.g., `1:`, `42:`) | ||
- Labels starting with special prefixes (`$`, `__`) | ||
- Labels starting with `.` (local scope) |
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.
Only numeric labels are suitable for inline asm. The problem with local labels is that they are file local which means that you can still get duplicate symbol errors if the optimizer decides to clone the function containing the inline asm.
This is a feature inspired by a rust lint-run-amok: rust-lang/rust#143021
Could we (and should we?) generalize this for the sake of all frontends who want to avoid the problems caused by global/public labels in inline assembly?
I'm not even 100% sure that I captured the rationale here but I took a stab at it.