Skip to content

MC,AsmPrinter: Report redefinition error instead of crashing in more cases #145460

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

MaskRay
Copy link
Member

@MaskRay MaskRay commented Jun 24, 2025

  • Fix the crash for .equiv b, undef; b: (.equiv equates a symbol to an expression and reports an error if the symbol was already defined).
  • Remove redundant isVariable check from emitFunctionEntryLabel

Created using spr 1.3.5-bogner
@llvmbot llvmbot added backend:X86 llvm:codegen mc Machine (object) code labels Jun 24, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 24, 2025

@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-x86

Author: Fangrui Song (MaskRay)

Changes
  • Fix the crash for .equiv b, undef; b: (.equiv equates a symbol to an expression and reports an error if the symbol was already defined).
  • Remove redundant isVariable check from emitFunctionEntryLabel

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

5 Files Affected:

  • (modified) llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp (-7)
  • (modified) llvm/lib/MC/MCAsmStreamer.cpp (+2-1)
  • (modified) llvm/lib/MC/MCObjectStreamer.cpp (+4)
  • (modified) llvm/test/CodeGen/X86/equiv_with_fndef.ll (+2-2)
  • (modified) llvm/test/MC/AsmParser/redef-err.s (+4)
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 403963f33b65c..1bfa551d71966 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -1081,13 +1081,6 @@ void AsmPrinter::emitFunctionHeader() {
 /// function.  This can be overridden by targets as required to do custom stuff.
 void AsmPrinter::emitFunctionEntryLabel() {
   CurrentFnSym->redefineIfPossible();
-
-  // The function label could have already been emitted if two symbols end up
-  // conflicting due to asm renaming.  Detect this and emit an error.
-  if (CurrentFnSym->isVariable())
-    report_fatal_error("'" + Twine(CurrentFnSym->getName()) +
-                       "' is a protected alias");
-
   OutStreamer->emitLabel(CurrentFnSym);
 
   if (TM.getTargetTriple().isOSBinFormatELF()) {
diff --git a/llvm/lib/MC/MCAsmStreamer.cpp b/llvm/lib/MC/MCAsmStreamer.cpp
index c43619d712172..c6dbc625642d4 100644
--- a/llvm/lib/MC/MCAsmStreamer.cpp
+++ b/llvm/lib/MC/MCAsmStreamer.cpp
@@ -569,7 +569,8 @@ void MCAsmStreamer::emitLabel(MCSymbol *Symbol, SMLoc Loc) {
   // FIXME: Fix CodeGen/AArch64/arm64ec-varargs.ll. emitLabel is followed by
   // setVariableValue, leading to an assertion failure if setOffset(0) is
   // called.
-  if (getContext().getObjectFileType() != MCContext::IsCOFF)
+  if (!Symbol->isVariable() &&
+      getContext().getObjectFileType() != MCContext::IsCOFF)
     Symbol->setOffset(0);
 
   Symbol->print(OS, MAI);
diff --git a/llvm/lib/MC/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp
index 1bb2143ed6ab2..e959a242dfcf5 100644
--- a/llvm/lib/MC/MCObjectStreamer.cpp
+++ b/llvm/lib/MC/MCObjectStreamer.cpp
@@ -225,6 +225,10 @@ void MCObjectStreamer::emitCFIEndProcImpl(MCDwarfFrameInfo &Frame) {
 
 void MCObjectStreamer::emitLabel(MCSymbol *Symbol, SMLoc Loc) {
   MCStreamer::emitLabel(Symbol, Loc);
+  // If Symbol is a non-redefiniable variable, emitLabel has reported an error.
+  // Bail out.
+  if (Symbol->isVariable())
+    return;
 
   getAssembler().registerSymbol(*Symbol);
 
diff --git a/llvm/test/CodeGen/X86/equiv_with_fndef.ll b/llvm/test/CodeGen/X86/equiv_with_fndef.ll
index 3da0aa60250c2..a858ec2bf4537 100644
--- a/llvm/test/CodeGen/X86/equiv_with_fndef.ll
+++ b/llvm/test/CodeGen/X86/equiv_with_fndef.ll
@@ -1,4 +1,4 @@
-; RUN: not --crash llc < %s 2>&1 | FileCheck %s
+; RUN: not llc < %s 2>&1 | FileCheck %s
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
@@ -7,4 +7,4 @@ module asm ".equiv pselect, __pselect"
 define void @pselect() {
   ret void
 }
-; CHECK: 'pselect' is a protected alias
+; CHECK: <unknown>:0: error: symbol 'pselect' is already defined
diff --git a/llvm/test/MC/AsmParser/redef-err.s b/llvm/test/MC/AsmParser/redef-err.s
index e191d01003f41..ff0b1cc221549 100644
--- a/llvm/test/MC/AsmParser/redef-err.s
+++ b/llvm/test/MC/AsmParser/redef-err.s
@@ -12,3 +12,7 @@ l:
 
 .equiv a, undef
 # CHECK: [[#@LINE-1]]:11: error: redefinition of 'a'
+
+.equiv b, undef
+b:
+# CHECK: [[#@LINE-1]]:1: error: symbol 'b' is already defined

@MaskRay MaskRay merged commit 290fc1e into main Jun 24, 2025
9 of 11 checks passed
@MaskRay MaskRay deleted the users/MaskRay/spr/mcasmprinter-report-redefinition-error-instead-of-crashing-in-more-cases branch June 24, 2025 06:00
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Jun 24, 2025
…ng in more cases

* Fix the crash for `.equiv b, undef; b:` (.equiv equates a symbol to an expression and reports an error if the symbol was already defined).
* Remove redundant isVariable check from emitFunctionEntryLabel

Pull Request: llvm/llvm-project#145460
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants