Skip to content

Commit

Permalink
[MC/AsmParser] Avoid setting MCSymbol.IsUsed in some cases
Browse files Browse the repository at this point in the history
Avoid marking some MCSymbols as used in MC/AsmParser.cpp when no uses
exist. This fixes a bug in parseAssignmentExpression() which
inadvertently sets IsUsed, thereby triggering:

    "invalid re-assignment of non-absolute variable"

on otherwise valid code. No other functionality change intended.

The original version of this patch touched many calls to MCSymbol
accessors. On rafael's advice, I have stripped this patch down a bit.

As a follow-up, I intend to find the call sites which intentionally set
IsUsed and force them to do so explicitly.

Differential Revision: http://reviews.llvm.org/D12347

llvm-svn: 246457
  • Loading branch information
vedantk committed Aug 31, 2015
1 parent 2d69f96 commit 86dbd92
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 10 deletions.
2 changes: 1 addition & 1 deletion llvm/include/llvm/MC/MCSymbol.h
Expand Up @@ -223,7 +223,7 @@ class MCSymbol {

/// isUsed - Check if this is used.
bool isUsed() const { return IsUsed; }
void setUsed(bool Value) const { IsUsed = Value; }
void setUsed(bool Value) const { IsUsed |= Value; }

/// \brief Check if this symbol is redefinable.
bool isRedefinable() const { return IsRedefinable; }
Expand Down
17 changes: 8 additions & 9 deletions llvm/lib/MC/MCParser/AsmParser.cpp
Expand Up @@ -693,9 +693,9 @@ bool AsmParser::Run(bool NoInitialTextSection, bool NoFinalize) {
// FIXME: We would really like to refer back to where the symbol was
// first referenced for a source location. We need to add something
// to track that. Currently, we just point to the end of the file.
printMessage(
getLexer().getLoc(), SourceMgr::DK_Error,
"assembler local symbol '" + Sym->getName() + "' not defined");
printMessage(getLexer().getLoc(), SourceMgr::DK_Error,
"assembler local symbol '" + Sym->getName() +
"' not defined");
}
}

Expand Down Expand Up @@ -867,11 +867,12 @@ bool AsmParser::parsePrimaryExpr(const MCExpr *&Res, SMLoc &EndLoc) {

// If this is an absolute variable reference, substitute it now to preserve
// semantics in the face of reassignment.
if (Sym->isVariable() && isa<MCConstantExpr>(Sym->getVariableValue())) {
if (Sym->isVariable() &&
isa<MCConstantExpr>(Sym->getVariableValue(/*SetUsed*/ false))) {
if (Variant)
return Error(EndLoc, "unexpected modifier on variable reference");

Res = Sym->getVariableValue();
Res = Sym->getVariableValue(/*SetUsed*/ false);
return false;
}

Expand Down Expand Up @@ -4805,7 +4806,8 @@ bool parseAssignmentExpression(StringRef Name, bool allow_redef,
// FIXME: Diagnose assignment to protected identifier (e.g., register name).
if (isSymbolUsedInExpression(Sym, Value))
return Parser.Error(EqualLoc, "Recursive use of '" + Name + "'");
else if (Sym->isUndefined() && !Sym->isUsed() && !Sym->isVariable())
else if (Sym->isUndefined(/*SetUsed*/ false) && !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.
Expand All @@ -4817,9 +4819,6 @@ bool parseAssignmentExpression(StringRef Name, bool allow_redef,
return Parser.Error(EqualLoc,
"invalid reassignment of non-absolute variable '" +
Name + "'");

// Don't count these checks as uses.
Sym->setUsed(false);
} else if (Name == ".") {
if (Parser.getStreamer().EmitValueToOffset(Value, 0)) {
Parser.Error(EqualLoc, "expected absolute expression");
Expand Down
12 changes: 12 additions & 0 deletions llvm/test/MC/AsmParser/reassign.s
@@ -0,0 +1,12 @@
// RUN: llvm-mc -triple i386-unknown-unknown %s | FileCheck %s

.text
bar:

.data
.globl foo
.set foo, bar
.globl foo
.set foo, bar

// CHECK-NOT: invalid reassignment of non-absolute variable

0 comments on commit 86dbd92

Please sign in to comment.