[PATCH] COFF MC: better handling of tricky symbol and section names

Hans Wennborg hans at chromium.org
Tue Oct 15 17:40:41 PDT 2013



================
Comment at: lib/MC/MCParser/AsmParser.cpp:796
@@ -795,1 +795,3 @@
     std::pair<StringRef, StringRef> Split = Identifier.split('@');
+    if (FirstTokenKind == AsmToken::String) {
+      // The identifier was quoted (a string), so don't split it.
----------------
David Majnemer wrote:
> Instead of doing the split and then choosing to ignore it, why not split conditionally?
Yeah, I guess this is a bit backwards :) Fixing..

================
Comment at: lib/MC/MCParser/AsmParser.cpp:795
@@ -794,2 +794,3 @@
     // This is a symbol reference.
     std::pair<StringRef, StringRef> Split = Identifier.split('@');
+    if (FirstTokenKind == AsmToken::String) {
----------------
Reid Kleckner wrote:
> Maybe '@' symbols should form separate tokens, but that would be a large change.
Yeah, I don't want to go there right now.

================
Comment at: lib/MC/MCSectionCOFF.cpp:42-49
@@ -41,1 +41,10 @@
 
+static bool isAcceptableSectionNameChar(char C) {
+  if ((C < 'a' || C > 'z') &&
+      (C < 'A' || C > 'Z') &&
+      (C < '0' || C > '9') &&
+      C != '_' && C != '$' && C != '.')
+    return false;
+  return true;
+}
+
----------------
Reid Kleckner wrote:
> This seems easier to implement in the forward sense, like:
>   return ('a' <= C && C <= 'z') ||
>          ('A' <= C && ...
>          C == '_' || C == '$' || ...
Will do.

================
Comment at: lib/MC/MCSectionCOFF.cpp:71
@@ +70,3 @@
+  if (sectionNameNeedsQuoting(getSectionName()))
+    OS << "\t.section\t" << '"' << getSectionName() << '"' << ",\"";
+  else
----------------
Reid Kleckner wrote:
> We should escape the quotes.  There's stuff in support that'll help.
I copied this approach from MCSymbol::print().

Looking at AsmLexer::LexQuote(), it doesn't look like we could parse escaped quotes back in, so I don't think we should add escaping right now.

Let's just leave this until we run into some symbols with quotes in them.


http://llvm-reviews.chandlerc.com/D1945



More information about the llvm-commits mailing list