Skip to content
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

register_terminate_handler fails on MSVC, when default call convention is not __cdecl #197

Closed
DNKpp opened this issue Dec 23, 2024 · 5 comments

Comments

@DNKpp
Copy link
Contributor

DNKpp commented Dec 23, 2024

Hello,
I've encountered a rather niche issue. I'm currently working on an integration of cpptrace into my mocking framework mimic++, which has support for arbitrary call conventions.
In my ci/cd build I've one build with an alternative default call convention enabled (/Gv switch => __vectorcall), which is set via CXXFLAGS. This requires cpptrace to compile with that convention, too. Unfortunately the corecrt_terminate.h requires the terminate handler explicitly to use the __cdecl call convention, which then results in a compilation error in utils.cpp, register_terminate_handler.
Definition in corecrt_terminate.h:
typedef void (__CRTDECL* terminate_handler )(void);

I don't know whether other compilers support alternative default call conventions too and if they have the same requirement for the terminate handler. But maybe we can somehow workaround this?

EDIT: Here is the link to the relevant run:
https://github.com/DNKpp/mimicpp/actions/runs/12467234585/job/34796176790?pr=85

@jeremy-rifkin
Copy link
Owner

Hi,
It sounds like you're using FetchContent and setting flags that are then set for cpptrace's build? The idea that external code could mess with the calling conventions inside cpptrace is scary to me.

That could cause a lot of problems even beyond the terminate handler, pretty much any time cpptrace passes a function pointer (set_terminate, SymEnumSymbols, libbacktrace if that's used, ...). On one hand it's just a few places internally where adding __cdecl annotations would make this work but on the other hand I don't test for this and I'm hesitant to try to support it from the cpptrace side of things. In my opinion the right thing to do here is to set the flag for your project in such a way that it won't affect cpptrace. Thoughts?

@DNKpp
Copy link
Contributor Author

DNKpp commented Dec 28, 2024

Hey,
thank you for your response. You're right; I'm using FetchContent and applying global CXXFLAGS for the build, which indeed affects the compilation of cpptrace.
I don't mean to blame cpptrace for being incompatible with alternative default calling conventions. As I mentioned, mimic++ supports this kind of mechanism, and I'm currently working on a straightforward integration of cpptrace into it. Since call-conventions are by no means an official C++ feature, I understand your hesitation regarding the explicit annotation. I have no problem with you stating that cpptrace does not support such a feature, as it's obviously quite niche and mainly a matter of curiosity. I just wanted to reach out and see if we could possibly make this work.

Greetings
Dominic

@jeremy-rifkin
Copy link
Owner

Thanks for the thoughtful reply. I can look into this more this week. If this is something easy to do on the cpptrace side of things I think it may as well be done.

@jeremy-rifkin jeremy-rifkin added the resolved in next release Resolved in dev label Dec 31, 2024
@jeremy-rifkin
Copy link
Owner

I've gone ahead and added __cdecl for the terminate handler. I think there's nowhere else that needs this currently for any windows back-ends so it should be good to go.

@DNKpp
Copy link
Contributor Author

DNKpp commented Jan 1, 2025

Cool, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants