[clang] [clang] Correct Microsoft mangling of lifetime extended temporary objects. (PR #85529)
@@ -54,6 +54,12 @@ ABI Changes in This Version inline member function that contains a static local variable with a dynamic initializer is declared with ``__declspec(dllimport)``. (#GH83616). +- Fixed Microsoft name mangling of lifetime extended temporary objects. This + change corrects missing back reference registrations that could result in + incorrect back reference indexes and suprising demangled name results. Since + MSVC uses a different mangling for these objects, compatibility is not affected. + (#GH85423). tahonermann wrote: A new issue to address the inconsistent use of the `$S` mangling (as well as an additional issue with mangling of anonymous unions) has been filed as #86525. https://github.com/llvm/llvm-project/pull/85529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Correct Microsoft mangling of lifetime extended temporary objects. (PR #85529)
https://github.com/tahonermann closed https://github.com/llvm/llvm-project/pull/85529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Correct Microsoft mangling of lifetime extended temporary objects. (PR #85529)
@@ -54,6 +54,12 @@ ABI Changes in This Version inline member function that contains a static local variable with a dynamic initializer is declared with ``__declspec(dllimport)``. (#GH83616). +- Fixed Microsoft name mangling of lifetime extended temporary objects. This + change corrects missing back reference registrations that could result in + incorrect back reference indexes and suprising demangled name results. Since + MSVC uses a different mangling for these objects, compatibility is not affected. + (#GH85423). majnemer wrote: LGTM https://github.com/llvm/llvm-project/pull/85529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Correct Microsoft mangling of lifetime extended temporary objects. (PR #85529)
https://github.com/zmodem approved this pull request. This looks reasonable to me, but maybe give rnk and/or majnemer a day to comments as well since they're the real experts. re: the incompatibility, yes if MSVC is mangling these now, clang should probably try to match. https://github.com/llvm/llvm-project/pull/85529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Correct Microsoft mangling of lifetime extended temporary objects. (PR #85529)
@@ -54,6 +54,12 @@ ABI Changes in This Version inline member function that contains a static local variable with a dynamic initializer is declared with ``__declspec(dllimport)``. (#GH83616). +- Fixed Microsoft name mangling of lifetime extended temporary objects. This + change corrects missing back reference registrations that could result in + incorrect back reference indexes and suprising demangled name results. Since + MSVC uses a different mangling for these objects, compatibility is not affected. + (#GH85423). zmodem wrote: > Commit > [e77de75](https://github.com/llvm/llvm-project/commit/e77de75d7e009daaadb03e9b1091c3a9a9c9d311) > claims that Microsoft doesn't have a mangling for these objects and that the > `$RT` mangling is a Clang invention. The claim that MSVC doesn't implement a > mangling for these doesn't seem right though. Consider > https://godbolt.org/z/n715zPbc6. Looks like things have changed then. I tried the example code with MSVC 18.00.31101.0 (from VS2013) and there the object is just `_$S1`: ``` ??__E?singleton@RT1@@2ABU1@B@@YAXXZ PROC; ??__E?singleton@RT1@@2ABU1@B@@YAXXZ, COMDAT ; File z:\tmp\a.cc ; Line 6 pushebp mov ebp, esp pushecx call?f@@YA?AURT1@@XZ; f mov DWORD PTR $T1[ebp], eax mov eax, DWORD PTR $T1[ebp] mov DWORD PTR _$S1, eax mov DWORD PTR ?singleton@RT1@@2ABU1@B, OFFSET _$S1 ; RT1::singleton mov esp, ebp pop ebp ret 0 ``` https://github.com/llvm/llvm-project/pull/85529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Correct Microsoft mangling of lifetime extended temporary objects. (PR #85529)
@@ -54,6 +54,12 @@ ABI Changes in This Version inline member function that contains a static local variable with a dynamic initializer is declared with ``__declspec(dllimport)``. (#GH83616). +- Fixed Microsoft name mangling of lifetime extended temporary objects. This + change corrects missing back reference registrations that could result in + incorrect back reference indexes and suprising demangled name results. Since + MSVC uses a different mangling for these objects, compatibility is not affected. + (#GH85423). tahonermann wrote: Commit https://github.com/llvm/llvm-project/commit/e77de75d7e009daaadb03e9b1091c3a9a9c9d311 claims that Microsoft doesn't have a mangling for these objects and that the `$RT` mangling is a Clang invention. The claim that MSVC doesn't implement a mangling for these doesn't seem right though. Consider https://godbolt.org/z/n715zPbc6. It appears that MSVC (back to at least v19.14) uses a `$S` mangling for these; like that used for other unnamed objects like for structured bindings, anonymous unions, and (non-thread-safe) guards for static local variables. Further, I think https://godbolt.org/z/x1Kh57818 demonstrates a Clang/MSVC compatibility issue. The code Clang emits for `h()` for the inlining of `g()` attempts to import `__imp_?$RT1@singleton@?1??g@@YAHXZ@4ABUS@@B` while MSVC's code attempts to import `__imp_?$S1@?1??g@@YAHXZ@4US@@B`. Assuming agreement with the analysis above, I'll file a new issue for the divergent mangling. https://github.com/llvm/llvm-project/pull/85529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Correct Microsoft mangling of lifetime extended temporary objects. (PR #85529)
llvmbot wrote: @llvm/pr-subscribers-clang Author: Tom Honermann (tahonermann) Changes Lifetime extended temporary objects that are bound to references with static storage duration may have external linkage and therefore require mangled symbol names. Clang uses an extension of the Microsoft ABI to give these symbols an implicit name of '$RT' followed by a discriminator and then mangles them similarly to the variable they are bound to. Clang's mangling scheme differs from the one used by MSVC. Previously, the `$RT` portion of the name was not registered as a back reference candidate and this resulted in incorrect back references for enclosing class and/or namespace scopes that might be referenced in the type of the object. This is an ABI change and has the potential to cause backward compatibility issues with previous Clang releases. Since MSVC uses a different mangling scheme, this change does not affect compatibility with MSVC. This fixes one of the name mangling concerns reported in #85423. --- Full diff: https://github.com/llvm/llvm-project/pull/85529.diff 3 Files Affected: - (modified) clang/docs/ReleaseNotes.rst (+6) - (modified) clang/lib/AST/MicrosoftMangle.cpp (+2-1) - (modified) clang/test/CodeGenCXX/mangle-ms-back-references.cpp (+13) ``diff diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 1ae35e6881d2f8..f0da6ca497b575 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -54,6 +54,12 @@ ABI Changes in This Version inline member function that contains a static local variable with a dynamic initializer is declared with ``__declspec(dllimport)``. (#GH83616). +- Fixed Microsoft name mangling of lifetime extended temporary objects. This + change corrects missing back reference registrations that could result in + incorrect back reference indexes and suprising demangled name results. Since + MSVC uses a different mangling for these objects, compatibility is not affected. + (#GH85423). + AST Dumping Potentially Breaking Changes diff --git a/clang/lib/AST/MicrosoftMangle.cpp b/clang/lib/AST/MicrosoftMangle.cpp index aa26bb7ed46f48..cf9c2093a8f6a1 100644 --- a/clang/lib/AST/MicrosoftMangle.cpp +++ b/clang/lib/AST/MicrosoftMangle.cpp @@ -3911,7 +3911,8 @@ void MicrosoftMangleContextImpl::mangleReferenceTemporary( msvc_hashing_ostream MHO(Out); MicrosoftCXXNameMangler Mangler(*this, MHO); - Mangler.getStream() << "?$RT" << ManglingNumber << '@'; + Mangler.getStream() << "?"; + Mangler.mangleSourceName("$RT" + llvm::utostr(ManglingNumber)); Mangler.mangle(VD, ""); } diff --git a/clang/test/CodeGenCXX/mangle-ms-back-references.cpp b/clang/test/CodeGenCXX/mangle-ms-back-references.cpp index b27a9c5acacb77..8707bff9534070 100644 --- a/clang/test/CodeGenCXX/mangle-ms-back-references.cpp +++ b/clang/test/CodeGenCXX/mangle-ms-back-references.cpp @@ -1,5 +1,18 @@ // RUN: %clang_cc1 -fms-extensions -fblocks -emit-llvm %s -o - -triple=i386-pc-win32 | FileCheck %s +namespace NS { +// The name "RT1" for the name of the class below has been specifically +// chosen to ensure that back reference lookup does not match against the +// implicitly generated "$RT1" name of the reference temporary symbol. +struct RT1 { + static const RT1& singleton; + int i; +}; +const RT1& RT1::singleton = RT1{1}; +} +// CHECK: "?$RT1@singleton@RT1@NS@@2ABU23@B" +// CHECK: "?singleton@RT1@NS@@2ABU12@B" + void f1(const char* a, const char* b) {} // CHECK: "?f1@@YAXPBD0@Z" `` https://github.com/llvm/llvm-project/pull/85529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Correct Microsoft mangling of lifetime extended temporary objects. (PR #85529)
https://github.com/tahonermann created
https://github.com/llvm/llvm-project/pull/85529
Lifetime extended temporary objects that are bound to references with static
storage duration may have external linkage and therefore require mangled symbol
names. Clang uses an extension of the Microsoft ABI to give these symbols an
implicit name of '$RT' followed by a discriminator and then mangles them
similarly to the variable they are bound to. Clang's mangling scheme differs
from the one used by MSVC.
Previously, the `$RT` portion of the name was not registered as
a back reference candidate and this resulted in incorrect back references for
enclosing class and/or namespace scopes that might be referenced in the type of
the object.
This is an ABI change and has the potential to cause backward compatibility
issues with previous Clang releases. Since MSVC uses a different mangling
scheme, this change does not affect compatibility with MSVC.
This fixes one of the name mangling concerns reported in #85423.
>From 4c0149e934aae9d061e22400eb3eace47e4906be Mon Sep 17 00:00:00 2001
From: Tom Honermann
Date: Sat, 16 Mar 2024 07:09:58 -0700
Subject: [PATCH] [clang] Correct Microsoft mangling of lifetime extended
temporary objects.
Lifetime extended temporary objects that are bound to references with static
storage duration may have external linkage and therefore require mangled symbol
names. Clang uses an extension of the Microsoft ABI to give these symbols an
implicit name of '$RT' followed by a discriminator and then mangles them
similarly to the variable they are bound to. Clang's mangling scheme differs
from the one used by MSVC.
Previously, the '$RT' portion of the name was not registered as
a back reference candidate and this resulted in incorrect back references for
enclosing class and/or namespace scopes that might be referenced in the
type of the object.
This is an ABI change and has the potential to cause backward compatibility
issues with previous Clang releases. Since MSVC uses a different mangling
scheme, this change does not affect compatibility with MSVC.
---
clang/docs/ReleaseNotes.rst | 6 ++
clang/lib/AST/MicrosoftMangle.cpp | 3 ++-
clang/test/CodeGenCXX/mangle-ms-back-references.cpp | 13 +
3 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1ae35e6881d2f8..f0da6ca497b575 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -54,6 +54,12 @@ ABI Changes in This Version
inline member function that contains a static local variable with a dynamic
initializer is declared with ``__declspec(dllimport)``. (#GH83616).
+- Fixed Microsoft name mangling of lifetime extended temporary objects. This
+ change corrects missing back reference registrations that could result in
+ incorrect back reference indexes and suprising demangled name results. Since
+ MSVC uses a different mangling for these objects, compatibility is not
affected.
+ (#GH85423).
+
AST Dumping Potentially Breaking Changes
diff --git a/clang/lib/AST/MicrosoftMangle.cpp
b/clang/lib/AST/MicrosoftMangle.cpp
index aa26bb7ed46f48..cf9c2093a8f6a1 100644
--- a/clang/lib/AST/MicrosoftMangle.cpp
+++ b/clang/lib/AST/MicrosoftMangle.cpp
@@ -3911,7 +3911,8 @@ void MicrosoftMangleContextImpl::mangleReferenceTemporary(
msvc_hashing_ostream MHO(Out);
MicrosoftCXXNameMangler Mangler(*this, MHO);
- Mangler.getStream() << "?$RT" << ManglingNumber << '@';
+ Mangler.getStream() << "?";
+ Mangler.mangleSourceName("$RT" + llvm::utostr(ManglingNumber));
Mangler.mangle(VD, "");
}
diff --git a/clang/test/CodeGenCXX/mangle-ms-back-references.cpp
b/clang/test/CodeGenCXX/mangle-ms-back-references.cpp
index b27a9c5acacb77..8707bff9534070 100644
--- a/clang/test/CodeGenCXX/mangle-ms-back-references.cpp
+++ b/clang/test/CodeGenCXX/mangle-ms-back-references.cpp
@@ -1,5 +1,18 @@
// RUN: %clang_cc1 -fms-extensions -fblocks -emit-llvm %s -o -
-triple=i386-pc-win32 | FileCheck %s
+namespace NS {
+// The name "RT1" for the name of the class below has been specifically
+// chosen to ensure that back reference lookup does not match against the
+// implicitly generated "$RT1" name of the reference temporary symbol.
+struct RT1 {
+ static const RT1& singleton;
+ int i;
+};
+const RT1& RT1::singleton = RT1{1};
+}
+// CHECK: "?$RT1@singleton@RT1@NS@@2ABU23@B"
+// CHECK: "?singleton@RT1@NS@@2ABU12@B"
+
void f1(const char* a, const char* b) {}
// CHECK: "?f1@@YAXPBD0@Z"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
