Skip to content

arm64ec incorrect mangling instantiating C++ template #115231

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

Closed
efriedma-quic opened this issue Nov 6, 2024 · 5 comments · Fixed by #115567
Closed

arm64ec incorrect mangling instantiating C++ template #115231

efriedma-quic opened this issue Nov 6, 2024 · 5 comments · Fixed by #115567

Comments

@efriedma-quic
Copy link
Collaborator

template <typename X> struct Wrapper {
  int GetValue(void) const;
};
struct A {
  int GetValue(void) const;
};
template<typename X> int Wrapper<X>::GetValue(void) const { return 3; }
template class Wrapper<A>;

clang produces ?GetValue@$$h?$Wrapper@UA@@@@QEBAHXZ, MSVC produces ?GetValue@?$Wrapper@UA@@@@$$hQEBAHXZ. I guess the algorithm in llvm::getArm64ECMangledFunctionName is wrong.

CC @mstorsjo @cjacek @dpaoliello @steplong

@llvmbot
Copy link
Member

llvmbot commented Nov 7, 2024

@llvm/issue-subscribers-c-1

Author: Eli Friedman (efriedma-quic)

``` template <typename X> struct Wrapper { int GetValue(void) const; }; struct A { int GetValue(void) const; }; template<typename X> int Wrapper<X>::GetValue(void) const { return 3; } template class Wrapper<A>; ```

clang produces ?GetValue@$$h?$Wrapper@<!-- -->UA@@@@<!-- -->QEBAHXZ, MSVC produces ?GetValue@?$Wrapper@<!-- -->UA@@@@$$hQEBAHXZ. I guess the algorithm in llvm::getArm64ECMangledFunctionName is wrong.

CC @mstorsjo @cjacek @dpaoliello @steplong

@frederick-vs-ja frederick-vs-ja added diverges-from:msvc Does the clang frontend diverge from msvc on this issue clang:frontend Language frontend issues, e.g. anything involving "Sema" and removed new issue labels Nov 7, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 7, 2024

@llvm/issue-subscribers-clang-frontend

Author: Eli Friedman (efriedma-quic)

``` template <typename X> struct Wrapper { int GetValue(void) const; }; struct A { int GetValue(void) const; }; template<typename X> int Wrapper<X>::GetValue(void) const { return 3; } template class Wrapper<A>; ```

clang produces ?GetValue@$$h?$Wrapper@<!-- -->UA@@@@<!-- -->QEBAHXZ, MSVC produces ?GetValue@?$Wrapper@<!-- -->UA@@@@$$hQEBAHXZ. I guess the algorithm in llvm::getArm64ECMangledFunctionName is wrong.

CC @mstorsjo @cjacek @dpaoliello @steplong

@dpaoliello
Copy link
Contributor

I believe that I have the correct algorithm - PR out shortly

dpaoliello added a commit that referenced this issue Nov 13, 2024
Arm64EC uses a special name mangling mode that adds `$$h` between the
symbol name and its type. In MSVC's name mangling `@` is used to
separate the name and type BUT it is also used for other purposes, such
as the separator between paths in a fully qualified name.

The original algorithm was quite fragile and made assumptions that
didn't hold true for all MSVC mangled symbols, so instead of trying to
improve this algorithm we are now using the demangler to indicate where
the insertion point should be (i.e., to parse the fully-qualified name
and return the current string offset).

Also fixed `isArm64ECMangledFunctionName` to search for `@$$h` since the
`$$h` must always be after a `@`.

Fixes #115231
@EugeneZelenko EugeneZelenko added llvm:support llvm:ir and removed clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Nov 14, 2024
@dpaoliello dpaoliello added this to the LLVM 19.X Release milestone Nov 14, 2024
@github-project-automation github-project-automation bot moved this to Needs Triage in LLVM Release Status Nov 14, 2024
@dpaoliello
Copy link
Contributor

/cherry-pick fa0cf3d

@llvmbot
Copy link
Member

llvmbot commented Nov 14, 2024

Failed to cherry-pick: fa0cf3d

https://github.com/llvm/llvm-project/actions/runs/11842908473

Please manually backport the fix and push it to your github fork. Once this is done, please create a pull request

dpaoliello added a commit to dpaoliello/llvm-project that referenced this issue Nov 14, 2024
Arm64EC uses a special name mangling mode that adds `$$h` between the
symbol name and its type. In MSVC's name mangling `@` is used to
separate the name and type BUT it is also used for other purposes, such
as the separator between paths in a fully qualified name.

The original algorithm was quite fragile and made assumptions that
didn't hold true for all MSVC mangled symbols, so instead of trying to
improve this algorithm we are now using the demangler to indicate where
the insertion point should be (i.e., to parse the fully-qualified name
and return the current string offset).

Also fixed `isArm64ECMangledFunctionName` to search for `@$$h` since the
`$$h` must always be after a `@`.

Fixes llvm#115231
tru pushed a commit to dpaoliello/llvm-project that referenced this issue Nov 15, 2024
Arm64EC uses a special name mangling mode that adds `$$h` between the
symbol name and its type. In MSVC's name mangling `@` is used to
separate the name and type BUT it is also used for other purposes, such
as the separator between paths in a fully qualified name.

The original algorithm was quite fragile and made assumptions that
didn't hold true for all MSVC mangled symbols, so instead of trying to
improve this algorithm we are now using the demangler to indicate where
the insertion point should be (i.e., to parse the fully-qualified name
and return the current string offset).

Also fixed `isArm64ECMangledFunctionName` to search for `@$$h` since the
`$$h` must always be after a `@`.

Fixes llvm#115231
@tru tru moved this from Needs Triage to Done in LLVM Release Status Nov 15, 2024
nikic pushed a commit to rust-lang/llvm-project that referenced this issue Nov 20, 2024
Arm64EC uses a special name mangling mode that adds `$$h` between the
symbol name and its type. In MSVC's name mangling `@` is used to
separate the name and type BUT it is also used for other purposes, such
as the separator between paths in a fully qualified name.

The original algorithm was quite fragile and made assumptions that
didn't hold true for all MSVC mangled symbols, so instead of trying to
improve this algorithm we are now using the demangler to indicate where
the insertion point should be (i.e., to parse the fully-qualified name
and return the current string offset).

Also fixed `isArm64ECMangledFunctionName` to search for `@$$h` since the
`$$h` must always be after a `@`.

Fixes llvm#115231
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging a pull request may close this issue.

6 participants