Skip to content

Commit 76ee2d3

Browse files
committed
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)
1 parent 23c2f88 commit 76ee2d3

File tree

7 files changed

+47
-22
lines changed

7 files changed

+47
-22
lines changed

llvm/lib/MC/MCAsmStreamer.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -566,6 +566,11 @@ void MCAsmStreamer::emitELFSymverDirective(const MCSymbol *OriginalSym,
566566

567567
void MCAsmStreamer::emitLabel(MCSymbol *Symbol, SMLoc Loc) {
568568
MCStreamer::emitLabel(Symbol, Loc);
569+
// FIXME: Fix CodeGen/AArch64/arm64ec-varargs.ll. emitLabel is followed by
570+
// setVariableValue, leading to an assertion failure if setOffset(0) is
571+
// called.
572+
if (getContext().getObjectFileType() != MCContext::IsCOFF)
573+
Symbol->setOffset(0);
569574

570575
Symbol->print(OS, MAI);
571576
OS << MAI->getLabelSuffix();

llvm/lib/MC/MCParser/AsmParser.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6348,12 +6348,12 @@ bool parseAssignmentExpression(StringRef Name, bool allow_redef,
63486348
//
63496349
// FIXME: Diagnostics. Note the location of the definition as a label.
63506350
// FIXME: Diagnose assignment to protected identifier (e.g., register name).
6351-
if (Sym->isUndefined() && !Sym->isUsed() && !Sym->isVariable())
6351+
if (!Sym->isUnset() && (!allow_redef || !Sym->isRedefinable()))
6352+
return Parser.Error(EqualLoc, "redefinition of '" + Name + "'");
6353+
else if (Sym->isUndefined() && !Sym->isUsed() && !Sym->isVariable())
63526354
; // Allow redefinitions of undefined symbols only used in directives.
63536355
else if (Sym->isVariable() && !Sym->isUsed() && allow_redef)
63546356
; // Allow redefinitions of variables that haven't yet been used.
6355-
else if (!Sym->isUndefined() && (!Sym->isVariable() || !allow_redef))
6356-
return Parser.Error(EqualLoc, "redefinition of '" + Name + "'");
63576357
else if (!Sym->isVariable())
63586358
return Parser.Error(EqualLoc, "invalid assignment to '" + Name + "'");
63596359
else if (!isa<MCConstantExpr>(Sym->getVariableValue()))

llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3080,6 +3080,7 @@ void AMDGPUAsmParser::initializeGprCountSymbol(RegisterKind RegKind) {
30803080
assert(SymbolName && "initializing invalid register kind");
30813081
MCSymbol *Sym = getContext().getOrCreateSymbol(*SymbolName);
30823082
Sym->setVariableValue(MCConstantExpr::create(0, getContext()));
3083+
Sym->setRedefinable(true);
30833084
}
30843085

30853086
bool AMDGPUAsmParser::updateGprCountSymbols(RegisterKind RegKind,

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
## Test redefinition errors. MCAsmStreamer::emitLabel is different from MCObjectStreamer. Test both streamers.
2+
# RUN: not llvm-mc -triple=x86_64 %s -o /dev/null 2>&1 | FileCheck %s --implicit-check-not=error:
3+
# RUN: not llvm-mc -filetype=obj -triple=x86_64 %s -o /dev/null 2>&1 | FileCheck %s --implicit-check-not=error:
4+
5+
l:
6+
.set l, .
7+
# CHECK: [[#@LINE-1]]:9: error: redefinition of 'l'
8+
9+
.equiv a, undef
10+
.set a, 3
11+
# CHECK: [[#@LINE-1]]:9: error: redefinition of 'a'
12+
13+
.equiv a, undef
14+
# CHECK: [[#@LINE-1]]:11: error: redefinition of 'a'

llvm/test/MC/AsmParser/redef.s

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
# RUN: not llvm-mc -filetype=obj -triple=x86_64 %s -o /dev/null 2>&1 | FileCheck %s --implicit-check-not=error:
2+
3+
l:
4+
5+
.data
6+
.set x, 0
7+
.long x
8+
x = .-.data
9+
.long x
10+
.set x,.-.data
11+
# CHECK: [[#@LINE-1]]:8: error: invalid reassignment of non-absolute variable 'x'
12+
## TODO This should be allowed
13+
.long x
14+
15+
.globl l_v
16+
.set l_v, l
17+
.globl l_v
18+
.set l_v, l

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

Lines changed: 0 additions & 17 deletions
This file was deleted.

llvm/test/MC/ELF/weakref.s

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
# RUN: llvm-mc -filetype=obj -triple=x86_64 %s | llvm-readelf -s - | FileCheck %s
22
# 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:
3+
# 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:
34

45
// This is a long test that checks that the aliases created by weakref are
56
// never in the symbol table and that the only case it causes a symbol to
@@ -104,11 +105,14 @@ alias:
104105

105106
.weakref alias2, target
106107
.set alias2, 1
108+
# ERR: [[#@LINE-1]]:14: error: redefinition of 'alias2'
109+
.endif
107110

111+
.ifdef ERR2
108112
.weakref cycle0, cycle1
109113
.weakref cycle1, cycle0
110114
call cycle0
111-
# ERR: <unknown>:0: error: cyclic dependency detected for symbol 'cycle0'
112-
# ERR: [[#@LINE-2]]:1: error: expected relocatable expression
115+
# ERR2: <unknown>:0: error: cyclic dependency detected for symbol 'cycle0'
116+
# ERR2: [[#@LINE-2]]:1: error: expected relocatable expression
113117

114118
.endif

0 commit comments

Comments
 (0)