[llvm] 343428c - MC: Detect cyclic dependency for variable symbols
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Mon May 26 15:08:16 PDT 2025
Author: Fangrui Song
Date: 2025-05-26T15:08:11-07:00
New Revision: 343428c666f9293ae260bbcf79130562b830b268
URL: https://github.com/llvm/llvm-project/commit/343428c666f9293ae260bbcf79130562b830b268
DIFF: https://github.com/llvm/llvm-project/commit/343428c666f9293ae260bbcf79130562b830b268.diff
LOG: MC: Detect cyclic dependency for variable symbols
We report cyclic dependency errors for variable symbols and rely on
isSymbolUsedInExpression in parseAssignmentExpression at parse time,
which does not catch all setVariableValue cases (e.g. cyclic .weakref).
Instead, add a bit to MCSymbol and check it when walking the variable
value MCExpr. When a cycle is detected when we have a final layout,
report an error and set the variable to a constant to avoid duplicate
errors.
isSymbolUsedInExpression is considered deprecated, but it is still used
by AMDGPU (#112251).
Added:
llvm/test/MC/AsmParser/equate-cycle.s
Modified:
llvm/include/llvm/MC/MCSymbol.h
llvm/lib/MC/MCExpr.cpp
llvm/lib/MC/MCParser/AsmParser.cpp
llvm/test/MC/ARM/thumb_set-diagnostics.s
llvm/test/MC/AsmParser/variables-invalid.s
llvm/test/MC/ELF/weakref.s
llvm/test/MC/Mips/set-sym-recursive.s
Removed:
################################################################################
diff --git a/llvm/include/llvm/MC/MCSymbol.h b/llvm/include/llvm/MC/MCSymbol.h
index f6a892afdb162..627c92ec77f91 100644
--- a/llvm/include/llvm/MC/MCSymbol.h
+++ b/llvm/include/llvm/MC/MCSymbol.h
@@ -112,6 +112,9 @@ class MCSymbol {
/// True if we have created a relocation that uses this symbol.
mutable unsigned IsUsedInReloc : 1;
+ /// Used to detect cyclic dependency like `a = a + 1` and `a = b; b = a`.
+ unsigned IsResolving : 1;
+
/// This is actually a Contents enumerator, but is unsigned to avoid sign
/// extension and achieve better bitpacking with MSVC.
unsigned SymbolContents : 3;
@@ -164,7 +167,7 @@ class MCSymbol {
MCSymbol(SymbolKind Kind, const MCSymbolTableEntry *Name, bool isTemporary)
: IsTemporary(isTemporary), IsRedefinable(false), IsUsed(false),
IsRegistered(false), IsExternal(false), IsPrivateExtern(false),
- IsWeakExternal(false), Kind(Kind), IsUsedInReloc(false),
+ IsWeakExternal(false), Kind(Kind), IsUsedInReloc(false), IsResolving(0),
SymbolContents(SymContentsUnset), CommonAlignLog2(0), Flags(0) {
Offset = 0;
HasName = !!Name;
@@ -240,6 +243,9 @@ class MCSymbol {
}
}
+ bool isResolving() const { return IsResolving; }
+ void setIsResolving(bool V) { IsResolving = V; }
+
/// @}
/// \name Associated Sections
/// @{
diff --git a/llvm/lib/MC/MCExpr.cpp b/llvm/lib/MC/MCExpr.cpp
index eef69652c10f7..6c9fc94a5fdf9 100644
--- a/llvm/lib/MC/MCExpr.cpp
+++ b/llvm/lib/MC/MCExpr.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "llvm/MC/MCExpr.h"
+#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/Statistic.h"
#include "llvm/Config/llvm-config.h"
#include "llvm/MC/MCAsmBackend.h"
@@ -497,13 +498,25 @@ bool MCExpr::evaluateAsRelocatableImpl(MCValue &Res, const MCAssembler *Asm,
case SymbolRef: {
const MCSymbolRefExpr *SRE = cast<MCSymbolRefExpr>(this);
- const MCSymbol &Sym = SRE->getSymbol();
+ MCSymbol &Sym = const_cast<MCSymbol &>(SRE->getSymbol());
const auto Kind = SRE->getKind();
bool Layout = Asm && Asm->hasLayout();
// Evaluate recursively if this is a variable.
+ if (Sym.isResolving()) {
+ if (Asm && Asm->hasFinalLayout()) {
+ Asm->getContext().reportError(
+ Sym.getVariableValue()->getLoc(),
+ "cyclic dependency detected for symbol '" + Sym.getName() + "'");
+ Sym.IsUsed = false;
+ Sym.setVariableValue(MCConstantExpr::create(0, Asm->getContext()));
+ }
+ return false;
+ }
if (Sym.isVariable() && (Kind == MCSymbolRefExpr::VK_None || Layout) &&
canExpand(Sym, InSet)) {
+ Sym.setIsResolving(true);
+ auto _ = make_scope_exit([&] { Sym.setIsResolving(false); });
bool IsMachO =
Asm && Asm->getContext().getAsmInfo()->hasSubsectionsViaSymbols();
if (Sym.getVariableValue()->evaluateAsRelocatableImpl(Res, Asm,
@@ -709,9 +722,16 @@ MCFragment *MCExpr::findAssociatedFragment() const {
return MCSymbol::AbsolutePseudoFragment;
case SymbolRef: {
- const MCSymbolRefExpr *SRE = cast<MCSymbolRefExpr>(this);
- const MCSymbol &Sym = SRE->getSymbol();
- return Sym.getFragment();
+ auto &Sym =
+ const_cast<MCSymbol &>(cast<MCSymbolRefExpr>(this)->getSymbol());
+ if (Sym.Fragment)
+ return Sym.Fragment;
+ if (Sym.isResolving())
+ return MCSymbol::AbsolutePseudoFragment;
+ Sym.setIsResolving(true);
+ auto *F = Sym.getFragment();
+ Sym.setIsResolving(false);
+ return F;
}
case Unary:
diff --git a/llvm/lib/MC/MCParser/AsmParser.cpp b/llvm/lib/MC/MCParser/AsmParser.cpp
index f06bfd35763c3..75e2a2a908a84 100644
--- a/llvm/lib/MC/MCParser/AsmParser.cpp
+++ b/llvm/lib/MC/MCParser/AsmParser.cpp
@@ -6348,9 +6348,7 @@ 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 (Value->isSymbolUsedInExpression(Sym))
- return Parser.Error(EqualLoc, "Recursive use of '" + Name + "'");
- else if (Sym->isUndefined() && !Sym->isUsed() && !Sym->isVariable())
+ 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.
diff --git a/llvm/test/MC/ARM/thumb_set-diagnostics.s b/llvm/test/MC/ARM/thumb_set-diagnostics.s
index dedc9dddc406c..4c8c927f5dbac 100644
--- a/llvm/test/MC/ARM/thumb_set-diagnostics.s
+++ b/llvm/test/MC/ARM/thumb_set-diagnostics.s
@@ -1,9 +1,11 @@
-@ RUN: not llvm-mc -triple armv7-eabi -o /dev/null 2>&1 %s | FileCheck %s
+@ RUN: not llvm-mc -triple armv7-eabi -o /dev/null %s 2>&1 | FileCheck %s
+@ RUN: not llvm-mc -filetype=obj -triple=armv7-eabi -o /dev/null %s --defsym LOOP=1 2>&1 | FileCheck %s --check-prefix=ERR2 --implicit-check-not=error:
.syntax unified
.thumb
+.ifndef LOOP
.thumb_set
@ CHECK: error: expected identifier after '.thumb_set'
@@ -52,13 +54,6 @@ beta:
@ CHECK: error: redefinition of 'beta'
@ CHECK: .thumb_set beta, alpha
-@ CHECK: ^
-
- .type recursive_use,%function
- .thumb_set recursive_use, recursive_use + 1
-
-@ CHECK: error: Recursive use of 'recursive_use'
-@ CHECK: .thumb_set recursive_use, recursive_use + 1
@ CHECK: ^
variable_result = alpha + 1
@@ -68,3 +63,15 @@ beta:
@ CHECK: error: invalid reassignment of non-absolute variable 'variable_result'
@ CHECK: .thumb_set variable_result, 1
@ CHECK: ^
+
+.else
+.type recursive_use,%function
+.thumb_set recursive_use, recursive_use + 1
+@ ERR2: [[#@LINE-1]]:41: error: cyclic dependency detected for symbol 'recursive_use'
+@ ERR2: [[#@LINE-2]]:41: error: expression could not be evaluated
+
+.type recursive_use2,%function
+.thumb_set recursive_use2, recursive_use2 + 1
+@ ERR2: [[#@LINE-1]]:43: error: cyclic dependency detected for symbol 'recursive_use2'
+@ ERR2: [[#@LINE-2]]:43: error: expression could not be evaluated
+.endif
diff --git a/llvm/test/MC/AsmParser/equate-cycle.s b/llvm/test/MC/AsmParser/equate-cycle.s
new file mode 100644
index 0000000000000..f253bfa374f59
--- /dev/null
+++ b/llvm/test/MC/AsmParser/equate-cycle.s
@@ -0,0 +1,16 @@
+# RUN: not llvm-mc -filetype=obj %s -o /dev/null 2>&1 | FileCheck %s
+
+# CHECK: [[#@LINE+2]]:7: error: cyclic dependency detected for symbol 'a'
+# CHECK: [[#@LINE+1]]:7: error: expression could not be evaluated
+a = a + 1
+
+# CHECK: [[#@LINE+3]]:6: error: cyclic dependency detected for symbol 'b1'
+# CHECK: [[#@LINE+1]]:6: error: expression could not be evaluated
+b0 = b1
+b1 = b2
+b2 = b0
+
+# CHECK: [[#@LINE+3]]:6: error: cyclic dependency detected for symbol 'c1'
+# CHECK: [[#@LINE+1]]:9: error: expression could not be evaluated
+c0 = c1 + 1
+c1 = c0
diff --git a/llvm/test/MC/AsmParser/variables-invalid.s b/llvm/test/MC/AsmParser/variables-invalid.s
index c466d422b9699..354bc1fbfc968 100644
--- a/llvm/test/MC/AsmParser/variables-invalid.s
+++ b/llvm/test/MC/AsmParser/variables-invalid.s
@@ -1,9 +1,7 @@
-// RUN: not llvm-mc -triple i386-unknown-unknown %s 2> %t
+// RUN: not llvm-mc -triple=x86_64 %s 2> %t
// RUN: FileCheck --input-file %t %s
.data
-// CHECK: Recursive use of 't0_v0'
- t0_v0 = t0_v0 + 1
t1_v1 = 1
t1_v1 = 2
@@ -17,12 +15,3 @@ t2_s0:
// CHECK: invalid reassignment of non-absolute variable 't3_s0'
t3_s0 = 1
-
-// CHECK: Recursive use of 't4_s2'
- t4_s0 = t4_s1
- t4_s1 = t4_s2
- t4_s2 = t4_s0
-
-// CHECK: Recursive use of 't5_s1'
- t5_s0 = t5_s1 + 1
- t5_s1 = t5_s0
diff --git a/llvm/test/MC/ELF/weakref.s b/llvm/test/MC/ELF/weakref.s
index aae049f4b9f19..ce011dc869ba4 100644
--- a/llvm/test/MC/ELF/weakref.s
+++ b/llvm/test/MC/ELF/weakref.s
@@ -105,4 +105,10 @@ alias:
.weakref alias2, target
.set alias2, 1
+.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
+
.endif
diff --git a/llvm/test/MC/Mips/set-sym-recursive.s b/llvm/test/MC/Mips/set-sym-recursive.s
index 4e6b589813ac1..85e468db1a294 100644
--- a/llvm/test/MC/Mips/set-sym-recursive.s
+++ b/llvm/test/MC/Mips/set-sym-recursive.s
@@ -1,5 +1,9 @@
-# RUN: not llvm-mc -triple mips-unknown-linux %s 2>&1 | FileCheck %s
+# RUN: not llvm-mc -filetype=obj -triple=mips64 %s -o /dev/null 2>&1 | FileCheck %s --implicit-check-not=error:
+.set B, B + 1
+
+# CHECK: :[[@LINE+1]]:11: error: cyclic dependency detected for symbol 'A'
.set A, A + 1
-# CHECK: :[[@LINE-1]]:9: error: Recursive use of 'A'
+# CHECK: :[[@LINE+1]]:7: error: expected relocatable expression
+.word A
.word A
More information about the llvm-commits
mailing list