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

Differentiate C++ catch clauses from D ones with __cpp_type_info_ptr #2590

Merged
merged 1 commit into from
Feb 24, 2018

Conversation

Syniurge
Copy link
Contributor

@Syniurge Syniurge commented Feb 21, 2018

The following code currently segfaults:

extern (C++) void throwStdException();

void main() {
    import std.stdio : writeln;

    try {
        throwStdException();
    } catch (Throwable t) {
        writeln("Caught a Throwable");
    }
}
#include <exception>
void throwStdException() { throw std::exception(); }

Because the personality routine in druntime/rt/dwarfeh.d makes the wrong assumption that when it encounters a C++ exception, the catch clauses in the action table that it gets to examine are raw std::type_info pointers, which isn't the case if it's a Throwable (or derived) catch.

To make it possible for the personality routine to differentiate the two kinds of catch clauses, DMD wraps std::type_info pointers inside __cpp_type_info_ptr class instances, and this is what this PR implements for LDC.


Associated ldc/druntime PR: ldc-developers/druntime#119

BTW should the druntime submodule be updated in the ldc commit too?

@kinke
Copy link
Member

kinke commented Feb 21, 2018

Looking good, thanks a lot, I remember having trouble to understand what DMD was doing there.

BTW should the druntime submodule be updated in the ldc commit too?

Only possible if the commit is in the LDC repo, i.e., already in the main ldc branch or on its own branch - the latter is what I just did, so please include the submodule update for proper CI testing.

We'll also have to add your testcase to dmd-testsuite, too bad this quite important aspect of DMD's implementation isn't tested at all. Edit: Like here, throwing C++ function already available.

@Syniurge
Copy link
Contributor Author

We'll also have to add your testcase to dmd-testsuite, too bad this quite important aspect of DMD's implementation isn't tested at all. Edit: Like here, throwing C++ function already available.

Added a try...catch(Throwable) block inside that test: ldc-developers/dmd-testsuite#33

OutBuffer mangleBuf;
mangleBuf.writestring("_D");
mangleToBuffer(p.cd, &mangleBuf);
mangleBuf.printf("%d%s", 18, "_cpp_type_info_ptr");
Copy link
Member

@kinke kinke Feb 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have the concept of 'IR mangles', to give the TargetABI a chance to tweak the mangled name (e.g., to prevent LLVM from implicitly prepending an underscore). So please use getIRMangledVarName(mangleBuf.peekString(), LINKd) as name for the getOrCreateGlobal() call below. While at it, please also include the dmd-testsuite update, I'll merge then.

…o pointers inside __cpp_type_info_ptr class instances like DMD does.

Previously assuming in the personality routine that encountering a C++ exception meant that catch clauses were raw std::type_info pointers was wrong in cases like:

try {
   throwStdException(); // external C++ function throwing a std::exception
} catch (Throwable t) {
   (...)
}

Generated __cpp_type_info_ptr constants receive the same mangling that they do within DMD.
@kinke kinke merged commit bf8cd78 into ldc-developers:master Feb 24, 2018
@Syniurge
Copy link
Contributor Author

Ready for m-.. oh you already did, thanks!

Syniurge added a commit to Syniurge/Calypso that referenced this pull request Feb 24, 2018
…o pointers inside __cpp_type_info_ptr class instances like DMD does. (ldc-developers#2590)

Previously assuming in the personality routine that encountering a C++ exception meant that catch clauses were raw std::type_info pointers was wrong in cases like:

try {
   throwStdException(); // external C++ function throwing a std::exception
} catch (Throwable t) {
   (...)
}

Generated __cpp_type_info_ptr constants receive the same mangling that they do within DMD.

(cherry picked from commit bf8cd78)
@Syniurge Syniurge deleted the ldc-cppeh-fix branch March 8, 2018 16:51
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

Successfully merging this pull request may close these issues.

2 participants