Skip to content

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

androm3da
Copy link
Member

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.

@androm3da androm3da requested a review from Amanieu June 28, 2025 15:15
@androm3da androm3da self-assigned this Jun 28, 2025
@llvmbot llvmbot added backend:X86 mc Machine (object) code labels Jun 28, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 28, 2025

@llvm/pr-subscribers-backend-x86

@llvm/pr-subscribers-mc

Author: Brian Cain (androm3da)

Changes

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.


Full diff: https://github.com/llvm/llvm-project/pull/146215.diff

6 Files Affected:

  • (added) llvm/docs/InlineAsmSafetyDirective.md (+110)
  • (modified) llvm/include/llvm/MC/MCStreamer.h (+8)
  • (modified) llvm/lib/MC/MCParser/AsmParser.cpp (+27)
  • (modified) llvm/lib/MC/MCStreamer.cpp (+12)
  • (added) llvm/test/MC/X86/inline-asm-mode-basic.s (+25)
  • (added) llvm/test/MC/X86/inline-asm-mode-directive.s (+57)
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)
Copy link
Member Author

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.

Copy link

github-actions bot commented Jun 28, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@androm3da androm3da force-pushed the bcain/inline_asm_mode branch from d30a5f5 to 6b622b3 Compare June 28, 2025 15:54
Comment on lines +19 to +22
- 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)
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:X86 mc Machine (object) code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants