Skip to content

Revert "MC: Support quoted symbol names" #139296

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

joker-eph
Copy link
Collaborator

Reverts #138817

The BOLT testing is failing after this change.

@joker-eph joker-eph requested a review from MaskRay May 9, 2025 17:23
@joker-eph joker-eph added the skip-precommit-approval PR for CI feedback, not intended for review label May 9, 2025
@llvmbot llvmbot added the mc Machine (object) code label May 9, 2025
@llvmbot
Copy link
Member

llvmbot commented May 9, 2025

@llvm/pr-subscribers-mc

Author: Mehdi Amini (joker-eph)

Changes

Reverts llvm/llvm-project#138817

The BOLT testing is failing after this change.


Full diff: https://github.com/llvm/llvm-project/pull/139296.diff

5 Files Affected:

  • (modified) llvm/lib/MC/MCContext.cpp (-21)
  • (modified) llvm/lib/MC/MCSymbol.cpp (-2)
  • (modified) llvm/test/MC/AsmParser/quoted.s (-3)
  • (modified) llvm/test/MC/COFF/safeseh.s (+2-2)
  • (modified) llvm/test/MC/ELF/symbol-names.s (+3-19)
diff --git a/llvm/lib/MC/MCContext.cpp b/llvm/lib/MC/MCContext.cpp
index 41caf20b331b2..f70087e14f702 100644
--- a/llvm/lib/MC/MCContext.cpp
+++ b/llvm/lib/MC/MCContext.cpp
@@ -212,27 +212,6 @@ MCDataFragment *MCContext::allocInitialFragment(MCSection &Sec) {
 MCSymbol *MCContext::getOrCreateSymbol(const Twine &Name) {
   SmallString<128> NameSV;
   StringRef NameRef = Name.toStringRef(NameSV);
-  if (NameRef.contains('\\')) {
-    NameSV = NameRef;
-    size_t S = 0;
-    // Support escaped \\ and \" as in GNU Assembler. GAS issues a warning for
-    // other characters following \\, which we do not implement due to code
-    // structure.
-    for (size_t I = 0, E = NameSV.size(); I < E; ++I) {
-      char C = NameSV[I];
-      if (C == '\\') {
-        switch (NameSV[I + 1]) {
-        case '"':
-        case '\\':
-          C = NameSV[++I];
-          break;
-        }
-      }
-      NameSV[S++] = C;
-    }
-    NameSV.resize(S);
-    NameRef = NameSV;
-  }
 
   assert(!NameRef.empty() && "Normal symbols cannot be unnamed!");
 
diff --git a/llvm/lib/MC/MCSymbol.cpp b/llvm/lib/MC/MCSymbol.cpp
index 3ca85b76a35d9..2a709f4aef80c 100644
--- a/llvm/lib/MC/MCSymbol.cpp
+++ b/llvm/lib/MC/MCSymbol.cpp
@@ -74,8 +74,6 @@ void MCSymbol::print(raw_ostream &OS, const MCAsmInfo *MAI) const {
       OS << "\\n";
     else if (C == '"')
       OS << "\\\"";
-    else if (C == '\\')
-      OS << "\\\\";
     else
       OS << C;
   }
diff --git a/llvm/test/MC/AsmParser/quoted.s b/llvm/test/MC/AsmParser/quoted.s
index 710f2d0155f1c..16b0997827603 100644
--- a/llvm/test/MC/AsmParser/quoted.s
+++ b/llvm/test/MC/AsmParser/quoted.s
@@ -9,9 +9,6 @@
 "a b":
   call "a b"
 
-# CHECK: "a b\\":
-"a b\\":
-
 #--- err.s
  "a\":
 # ERR: 1:2: error: unterminated string constant
diff --git a/llvm/test/MC/COFF/safeseh.s b/llvm/test/MC/COFF/safeseh.s
index afcdc771ff994..d21628daff5fa 100644
--- a/llvm/test/MC/COFF/safeseh.s
+++ b/llvm/test/MC/COFF/safeseh.s
@@ -2,5 +2,5 @@
 
 // check that we quote the output of .safeseh
 
-.safeseh "\\foo"
-// CHECK: .safeseh "\\foo"
+.safeseh "\01foo"
+// CHECK: .safeseh "\01foo"
diff --git a/llvm/test/MC/ELF/symbol-names.s b/llvm/test/MC/ELF/symbol-names.s
index 427187c329acf..f605c723d4d4d 100644
--- a/llvm/test/MC/ELF/symbol-names.s
+++ b/llvm/test/MC/ELF/symbol-names.s
@@ -1,28 +1,12 @@
-// RUN: llvm-mc -triple=x86_64 -filetype=obj %s | llvm-objdump -tdr - | FileCheck %s
+// RUN: llvm-mc -triple i686-pc-linux -filetype=obj %s -o - | llvm-readobj --symbols - | FileCheck %s
 
 // MC allows ?'s in symbol names as an extension.
 
-// CHECK-LABEL:SYMBOL TABLE:
-// CHECK-NEXT: 0000000000000001 l     F .text  0000000000000000 a"b\{{$}}
-// CHECK-NEXT: 0000000000000006 l       .text  0000000000000000 a\{{$}}
-// CHECK-NEXT: 0000000000000000 g     F .text  0000000000000000 foo?bar
-// CHECK-NEXT: 0000000000000000 *UND*          0000000000000000 a"b\q{{$}}
-// CHECK-EMPTY:
-
 .text
 .globl foo?bar
 .type foo?bar, @function
 foo?bar:
 ret
 
-// CHECK-LABEL:<a"b\>:
-// CHECK-NEXT:   callq  {{.*}} <a"b\>
-// CHECK-LABEL:<a\>:
-// CHECK-NEXT:   callq  {{.*}}
-// CHECK-NEXT:     R_X86_64_PLT32 a"b\q-0x4
-.type "a\"b\\", @function
-"a\"b\\":
-  call "a\"b\\"
-"a\\":
-/// GAS emits a warning for \q
-  call "a\"b\q"
+// CHECK: Symbol
+// CHECK: Name: foo?bar

@jurahul jurahul self-requested a review May 9, 2025 20:13
@joker-eph joker-eph merged commit bd0d048 into main May 9, 2025
14 checks passed
@joker-eph joker-eph deleted the revert-138817-users/MaskRay/spr/mc-support-quoted-symbol-names branch May 9, 2025 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mc Machine (object) code skip-precommit-approval PR for CI feedback, not intended for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants