[llvm] r246457 - [MC/AsmParser] Avoid setting MCSymbol.IsUsed in some cases

[Vedant Kumar via llvm-commits]

Author: vedantk
Date: Mon Aug 31 12:44:53 2015
New Revision: 246457

URL: http://llvm.org/viewvc/llvm-project?rev=246457&view=rev
Log:
[MC/AsmParser] Avoid setting MCSymbol.IsUsed in some cases

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

Added:
    llvm/trunk/test/MC/AsmParser/reassign.s
Modified:
    llvm/trunk/include/llvm/MC/MCSymbol.h
    llvm/trunk/lib/MC/MCParser/AsmParser.cpp

Modified: llvm/trunk/include/llvm/MC/MCSymbol.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCSymbol.h?rev=246457&r1=246456&r2=246457&view=diff
==============================================================================
--- llvm/trunk/include/llvm/MC/MCSymbol.h (original)
+++ llvm/trunk/include/llvm/MC/MCSymbol.h Mon Aug 31 12:44:53 2015
@@ -223,7 +223,7 @@ public:
 
   /// 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; }

Modified: llvm/trunk/lib/MC/MCParser/AsmParser.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCParser/AsmParser.cpp?rev=246457&r1=246456&r2=246457&view=diff
==============================================================================
--- llvm/trunk/lib/MC/MCParser/AsmParser.cpp (original)
+++ llvm/trunk/lib/MC/MCParser/AsmParser.cpp Mon Aug 31 12:44:53 2015
@@ -693,9 +693,9 @@ bool AsmParser::Run(bool NoInitialTextSe
         // 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");
     }
   }
 
@@ -867,11 +867,12 @@ bool AsmParser::parsePrimaryExpr(const M
 
     // 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;
     }
 
@@ -4805,7 +4806,8 @@ bool parseAssignmentExpression(StringRef
     // 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.
@@ -4817,9 +4819,6 @@ bool parseAssignmentExpression(StringRef
       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");

Added: llvm/trunk/test/MC/AsmParser/reassign.s
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/AsmParser/reassign.s?rev=246457&view=auto
==============================================================================
--- llvm/trunk/test/MC/AsmParser/reassign.s (added)
+++ llvm/trunk/test/MC/AsmParser/reassign.s Mon Aug 31 12:44:53 2015
@@ -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