llvm-project icon indicating copy to clipboard operation
llvm-project copied to clipboard

ms mangler for template values: poor nyi error messages and nyi opte pointer

Open memory-thrasher opened this issue 1 year ago • 3 comments

I am reporting this bug for completeness. I am actively working on a patch submission.

The official MSVC mangler lacks support for some c++ that is valid according to the standard, clang, and every decent compiler. Concerning template instance name mangling, specifically, there are presently 6 cases where the function MicrosoftCXXNameMangler::mangleTemplateArgValue gives up mangling and emits the rather unhelpful error:

cannot mangle this template argument yet

I improved that error message on my local build to tell which of the 6 possible cases I was running into. That case is when the non-type template parameter's value is an l-value point to one-past-the-end of some constexpr array. This case is not possible in c++ code that supports the official MSVC compiler, so consistency with MSVC is both impossible and uneeded. Furthermore, templates are not generally called across an ABI boudnary, but rather, each binary contains its own instances.

I arbitrarily chose a mangling pseudonym to represent this case. I foresee two potential future issues, though both seem to me to be highly unlikely and neither, any worse than failing to compile the code in the first place.

  • Compatibility between binaries output by clang using this patch, with binaries produced by some other compiler or future version of clang that might use a different arbitrary pseudonym. I doubt this case will ever happen because for the reasons already declared above; each binary should contain its own template instance with its own mangling rules.
  • Collision between two template function instances of the same template function where the template argument for each is the OPTE value for two different arrays, and that is the only difference between the instances, and yet the two instances should have different behavior. I cannot think of a good reason the two functions should have different behavior when handed a pointer that is essentially invalid (does not point to a usable resource). Even a reverse iterator has better solutions.

The patch will include a minimal test case.

memory-thrasher avatar Jul 04 '24 18:07 memory-thrasher

@llvm/issue-subscribers-clang-frontend

Author: None (memory-thrasher)

## I am reporting this bug for completeness. I am actively working on a patch submission.

The official MSVC mangler lacks support for some c++ that is valid according to the standard, clang, and every decent compiler. Concerning template instance name mangling, specifically, there are presently 6 cases where the function MicrosoftCXXNameMangler::mangleTemplateArgValue gives up mangling and emits the rather unhelpful error: > cannot mangle this template argument yet

I improved that error message on my local build to tell which of the 6 possible cases I was running into. That case is when the non-type template parameter's value is an l-value point to one-past-the-end of some constexpr array. This case is not possible in c++ code that supports the official MSVC compiler, so consistency with MSVC is both impossible and uneeded. Furthermore, templates are not generally called across an ABI boudnary, but rather, each binary contains its own instances.

I arbitrarily chose a mangling pseudonym to represent this case. I foresee two potential future issues, though both seem to me to be highly unlikely and neither, any worse than failing to compile the code in the first place.

  • Compatibility between binaries output by clang using this patch, with binaries produced by some other compiler or future version of clang that might use a different arbitrary pseudonym. I doubt this case will ever happen because for the reasons already declared above; each binary should contain its own template instance with its own mangling rules.
  • Collision between two template function instances of the same template function where the template argument for each is the OPTE value for two different arrays, and that is the only difference between the instances, and yet the two instances should have different behavior. I cannot think of a good reason the two functions should have different behavior when handed a pointer that is essentially invalid (does not point to a usable resource). Even a reverse iterator has better solutions.

The patch will include a minimal test case.

llvmbot avatar Jul 04 '24 21:07 llvmbot

A minimal test case would be helpful for the issue report as well.

shafik avatar Jul 04 '24 22:07 shafik

Here you go. Maybe not quite minimal but I wanted to make the usefulness when it works also apparent. Host system is ubuntu 22.04 so the default target uses itanium mangler. It's cross-compiling to msvc/windows that's broken.

ms_mangler_templatearg_opte.cpp.txt

memory-thrasher avatar Jul 05 '24 05:07 memory-thrasher

This case is not possible in c++ code that supports the official MSVC compiler, so consistency with MSVC is both impossible and uneeded. Furthermore, templates are not generally called across an ABI boundary, but rather, each binary contains its own instances.

For reference we were discussing this here: https://github.com/llvm/llvm-project/issues/94650.

While is this mostly true the original intention, afaik, is that -msvc triple is supposed to adhere to MSVC mangling including all its bugs. At least I regularly have code shipped to us from third-parties that are only built with MSVC and needs to link against clang-cl. We want the mangling to cohere even if it is to ensure we do not have duplicate symbols bloating a binary especially in debug builds (link times are a concern for us). See https://github.com/llvm/llvm-project/issues/83616 and https://github.com/llvm/llvm-project/pull/85300 which recently also fixed clang not adhering to MSVC mangling among other fixes in MicrosoftMangle.cpp.

In general I do agree especially in your example that if clang has a custom mangling it probably will never be noticed in debug builds and in release builds where it will most likely be inlined. However if any of these methods have static locals then multiple definitions across objects built with msvc vs clang isn't ideal.

I am writing up a discourse to get consensus on this as was discussed in https://github.com/llvm/llvm-project/issues/94650. Hopefully that goes up sometime tomorrow but I am fairly certain the outcome will be -msvc triple adheres to MSVC and we may have a separate triple for those who want MSVC-like mangling behaviour especially for templates.

MaxEW707 avatar Jul 06 '24 05:07 MaxEW707

verified working on trunc after that pr was merged.

memory-thrasher avatar Jul 24 '24 17:07 memory-thrasher