[llvm] 76ee2d3 - MCParser: Error when .set reassigns a non-redefinable variable

Fangrui Song via llvm-commits llvm-commits at lists.llvm.org
Mon May 26 20:19:57 PDT 2025


Author: Fangrui Song
Date: 2025-05-26T20:19:52-07:00
New Revision: 76ee2d34f787357eec1a5dec16b294578151881e

URL: https://github.com/llvm/llvm-project/commit/76ee2d34f787357eec1a5dec16b294578151881e
DIFF: https://github.com/llvm/llvm-project/commit/76ee2d34f787357eec1a5dec16b294578151881e.diff

LOG: MCParser: Error when .set reassigns a non-redefinable variable

The conditions in parseAssignmentExpression are conservative. We should
also report an error when a non-redefiniable variable (e.g. .equiv
followed by .set; .weakref followed by .set).

Make MCAsmStreamer::emitLabel call setOffset to make the behavior
similar to MCObjectStreamer. `isUndefined()` can now be replaced with
`isUnset()`.

Additionally, fix an AMDGPU API user (tested by a few tests including
MC/AMDGPU/hsa-v4.s)

Added: 
    llvm/test/MC/AsmParser/redef-err.s
    llvm/test/MC/AsmParser/redef.s

Modified: 
    llvm/lib/MC/MCAsmStreamer.cpp
    llvm/lib/MC/MCParser/AsmParser.cpp
    llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
    llvm/test/MC/ELF/weakref.s

Removed: 
    llvm/test/MC/AsmParser/variables-invalid.s


################################################################################
diff  --git a/llvm/lib/MC/MCAsmStreamer.cpp b/llvm/lib/MC/MCAsmStreamer.cpp
index f670335045cb1..da0d99e70d9ea 100644
--- a/llvm/lib/MC/MCAsmStreamer.cpp
+++ b/llvm/lib/MC/MCAsmStreamer.cpp
@@ -566,6 +566,11 @@ void MCAsmStreamer::emitELFSymverDirective(const MCSymbol *OriginalSym,
 
 void MCAsmStreamer::emitLabel(MCSymbol *Symbol, SMLoc Loc) {
   MCStreamer::emitLabel(Symbol, 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)
+    Symbol->setOffset(0);
 
   Symbol->print(OS, MAI);
   OS << MAI->getLabelSuffix();

diff  --git a/llvm/lib/MC/MCParser/AsmParser.cpp b/llvm/lib/MC/MCParser/AsmParser.cpp
index 75e2a2a908a84..8019953f5fee9 100644
--- a/llvm/lib/MC/MCParser/AsmParser.cpp
+++ b/llvm/lib/MC/MCParser/AsmParser.cpp
@@ -6348,12 +6348,12 @@ bool parseAssignmentExpression(StringRef Name, bool allow_redef,
     //
     // FIXME: Diagnostics. Note the location of the definition as a label.
     // FIXME: Diagnose assignment to protected identifier (e.g., register name).
-    if (Sym->isUndefined() && !Sym->isUsed() && !Sym->isVariable())
+    if (!Sym->isUnset() && (!allow_redef || !Sym->isRedefinable()))
+      return Parser.Error(EqualLoc, "redefinition of '" + Name + "'");
+    else if (Sym->isUndefined() && !Sym->isUsed() && !Sym->isVariable())
       ; // Allow redefinitions of undefined symbols only used in directives.
     else if (Sym->isVariable() && !Sym->isUsed() && allow_redef)
       ; // Allow redefinitions of variables that haven't yet been used.
-    else if (!Sym->isUndefined() && (!Sym->isVariable() || !allow_redef))
-      return Parser.Error(EqualLoc, "redefinition of '" + Name + "'");
     else if (!Sym->isVariable())
       return Parser.Error(EqualLoc, "invalid assignment to '" + Name + "'");
     else if (!isa<MCConstantExpr>(Sym->getVariableValue()))

diff  --git a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
index e64001fd655c9..2cd1ad7306e24 100644
--- a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
+++ b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
@@ -3080,6 +3080,7 @@ void AMDGPUAsmParser::initializeGprCountSymbol(RegisterKind RegKind) {
   assert(SymbolName && "initializing invalid register kind");
   MCSymbol *Sym = getContext().getOrCreateSymbol(*SymbolName);
   Sym->setVariableValue(MCConstantExpr::create(0, getContext()));
+  Sym->setRedefinable(true);
 }
 
 bool AMDGPUAsmParser::updateGprCountSymbols(RegisterKind RegKind,

diff  --git a/llvm/test/MC/AsmParser/redef-err.s b/llvm/test/MC/AsmParser/redef-err.s
new file mode 100644
index 0000000000000..e191d01003f41
--- /dev/null
+++ b/llvm/test/MC/AsmParser/redef-err.s
@@ -0,0 +1,14 @@
+## Test redefinition errors. MCAsmStreamer::emitLabel is 
diff erent from MCObjectStreamer. Test both streamers.
+# RUN: not llvm-mc -triple=x86_64 %s -o /dev/null 2>&1 | FileCheck %s --implicit-check-not=error:
+# RUN: not llvm-mc -filetype=obj -triple=x86_64 %s -o /dev/null 2>&1 | FileCheck %s --implicit-check-not=error:
+
+l:
+.set l, .
+# CHECK: [[#@LINE-1]]:9: error: redefinition of 'l'
+
+.equiv a, undef
+.set a, 3
+# CHECK: [[#@LINE-1]]:9: error: redefinition of 'a'
+
+.equiv a, undef
+# CHECK: [[#@LINE-1]]:11: error: redefinition of 'a'

diff  --git a/llvm/test/MC/AsmParser/redef.s b/llvm/test/MC/AsmParser/redef.s
new file mode 100644
index 0000000000000..56a13b10169ea
--- /dev/null
+++ b/llvm/test/MC/AsmParser/redef.s
@@ -0,0 +1,18 @@
+# RUN: not llvm-mc -filetype=obj -triple=x86_64 %s -o /dev/null 2>&1 | FileCheck %s --implicit-check-not=error:
+
+l:
+
+.data
+.set x, 0
+.long x
+x = .-.data
+.long x
+.set x,.-.data
+# CHECK: [[#@LINE-1]]:8: error: invalid reassignment of non-absolute variable 'x'
+## TODO This should be allowed
+.long x
+
+.globl l_v
+.set l_v, l
+.globl l_v
+.set l_v, l

diff  --git a/llvm/test/MC/AsmParser/variables-invalid.s b/llvm/test/MC/AsmParser/variables-invalid.s
deleted file mode 100644
index 354bc1fbfc968..0000000000000
--- a/llvm/test/MC/AsmParser/variables-invalid.s
+++ /dev/null
@@ -1,17 +0,0 @@
-// RUN: not llvm-mc -triple=x86_64 %s 2> %t
-// RUN: FileCheck --input-file %t %s
-
-        .data
-
-        t1_v1 = 1
-        t1_v1 = 2
-
-t2_s0:
-// CHECK: redefinition of 't2_s0'
-        t2_s0 = 2
-
-        t3_s0 = t2_s0 + 1
-        .long t3_s0
-// CHECK: invalid reassignment of non-absolute variable 't3_s0'
-        t3_s0 = 1
-

diff  --git a/llvm/test/MC/ELF/weakref.s b/llvm/test/MC/ELF/weakref.s
index ce011dc869ba4..a861625269538 100644
--- a/llvm/test/MC/ELF/weakref.s
+++ b/llvm/test/MC/ELF/weakref.s
@@ -1,5 +1,6 @@
 # RUN: llvm-mc -filetype=obj -triple=x86_64 %s | llvm-readelf -s - | FileCheck %s
 # RUN: not llvm-mc -filetype=obj -triple=x86_64 --defsym ERR=1 %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=ERR --implicit-check-not=error:
+# RUN: not llvm-mc -filetype=obj -triple=x86_64 --defsym ERR2=1 %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=ERR2 --implicit-check-not=error:
 
 // This is a long test that checks that the aliases created by weakref are
 // never in the symbol table and that the only case it causes a symbol to
@@ -104,11 +105,14 @@ alias:
 
 .weakref alias2, target
 .set alias2, 1
+# ERR: [[#@LINE-1]]:14: error: redefinition of 'alias2'
+.endif
 
+.ifdef ERR2
 .weakref cycle0, cycle1
 .weakref cycle1, cycle0
 call cycle0
-# ERR: <unknown>:0: error: cyclic dependency detected for symbol 'cycle0'
-# ERR: [[#@LINE-2]]:1: error: expected relocatable expression
+# ERR2: <unknown>:0: error: cyclic dependency detected for symbol 'cycle0'
+# ERR2: [[#@LINE-2]]:1: error: expected relocatable expression
 
 .endif


        


More information about the llvm-commits mailing list