https://issues.dlang.org/show_bug.cgi?id=21690

kinke <kinke@gmx.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kinke@gmx.net

--- Comment #1 from kinke <kinke@gmx.net> ---
The C++ vtable emitted by D does not contain any pointer to any typeinfo AFAIK. Of course we don't generate the C++ typeinfo anyway, but we do generate the D one (for use with the D GC). At least some C++ implementations store the (C++) typeinfo pointer at vtbl index -1. But then again, when we are confronted with some C++ class ref, we have no idea whether it was instantiated on the D side or C++ side and so which vtable its vptr points to.

So while getting proper dynamic casts working is probably quite hard, we could
probably disallow downcasts like this and require an explicit static cast
(`cast(CC) cast(void*) ca`)?

--

https://issues.dlang.org/show_bug.cgi?id=21690

--- Comment #2 from thomas.bockman@gmail.com ---
(In reply to kinke from comment #1)
> So while getting proper dynamic casts working is probably quite hard, we
> could probably disallow downcasts like this and require an explicit static
> cast (`cast(CC) cast(void*) ca`)?

That would be the bare minimum, to prevent people from unknowingly shooting themselves in the foot with this. It's an egregious hole in @safe as it stands now, a violation of the principle of least surprise even in @system code, and potentially a major security issue anywhere.

So, my recommendation is to make extern(C++) dynamic casts a compile-time error as soon as possible, and notify potential users through the announce forum that anyone who might have dynamic extern(C++) class casts in their code needs to update their compiler as soon as possible to identify latent security issues.

Once that has been done the importance of this bug can be dropped down from "critical" to "enhancement".

However, it would be really nice to get proper dynamic casts working somehow,
as they are a rather fundamental feature of class systems, and extern(C++) is
required for C++ interop, and for -betterC (my use case). Additionally, people
sometimes prefer extern(C++) classes simply because they are lighter weight
than extern(D) without the monitor pointer and certain virtual functions which
are unwanted for attribute reasons:
    https://forum.dlang.org/post/s263a0$15h$1@digitalmars.com

--

https://issues.dlang.org/show_bug.cgi?id=21690

--- Comment #3 from thomas.bockman@gmail.com ---
(In reply to kinke from comment #1)
> The C++ vtable emitted by D does not contain any pointer to any typeinfo AFAIK. Of course we don't generate the C++ typeinfo anyway, but we do generate the D one (for use with the D GC). At least some C++ implementations store the (C++) typeinfo pointer at vtbl index -1. But then again, when we are confronted with some C++ class ref, we have no idea whether it was instantiated on the D side or C++ side and so which vtable its vptr points to.

Does that mean that passing a D-initialized extern(C++) class instance to C++ code breaks dynamic casting on the C++ side, too? If so, that is a critical issue, as well.

Given that all current D compilers are also C++ compilers, would it be possible to just auto-generate an actual C++ function that performs the dynamic_cast<>, and then call (and hopefully inline) that function from the D side?

--

https://issues.dlang.org/show_bug.cgi?id=21690

--- Comment #4 from kinke <kinke@gmx.net> ---
(In reply to thomas.bockman from comment #3)
> Does that mean that passing a D-initialized extern(C++) class instance to C++ code breaks dynamic casting on the C++ side, too?

AFAIK, yes.

> Given that all current D compilers are also C++ compilers

They most definitely aren't - they are D compilers, with the ability to interop with C++ in many cases, but quite obviously not in all aspects. See point 33.1 and 33.13 on https://dlang.org/spec/cpp_interface.html.

--

https://issues.dlang.org/show_bug.cgi?id=21690

--- Comment #5 from thomas.bockman@gmail.com ---
(In reply to kinke from comment #4)
> (In reply to thomas.bockman from comment #3)
> > Given that all current D compilers are also C++ compilers
> 
> They most definitely aren't - they are D compilers, with the ability to interop with C++ in many cases, but quite obviously not in all aspects. See point 33.1 and 33.13 on https://dlang.org/spec/cpp_interface.html.

I was thinking of the common heritage with dmc, clang, and g++, but I will, of course, take your word for it that my suggestion is not helpful.

--

https://issues.dlang.org/show_bug.cgi?id=21690

--- Comment #6 from thomas.bockman@gmail.com ---
(In reply to kinke from comment #4)
> (In reply to thomas.bockman from comment #3)
> > Does that mean that passing a D-initialized extern(C++) class instance to C++ code breaks dynamic casting on the C++ side, too?
> 
> AFAIK, yes.

This is a very serious problem. It sounds like D will turn any attempt by C++ code to dynamically cast a D-allocated class instance into a buffer overrun-like bug by trying to access meta-data that isn't actually present, and also possibly by subsequently mis-typing the cast reference (like D does) if the program doesn't just crash in the first step.

APIs generally do not advertise whether they attempt dynamic casts internally or not, so there is no way to know what C++ libraries are affected by this bug, short of auditing their source code, which may not even be publicly available.

Making extern(C++) dynamic casts a compile-time error in D code is an acceptable, albeit disappointing, solution for D code. But, it won't solve the problem of D code poisoning linked C++ code. Is there ANY way to solve that problem, short of fully implementing this feature on the D side in a compatible way?

The only other way I can think of would be to forbid initialization of extern(C++) classes on the D side entirely, at least in @safe code. I suspect that would be a massive and frustrating breaking change for many people, though.

--

https://issues.dlang.org/show_bug.cgi?id=21690

anonymous4 <dfj1esp02@sneakemail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |C++

--- Comment #7 from anonymous4 <dfj1esp02@sneakemail.com> ---
Calypso and dpp are the only hybrid D/C++ compilers around.
I guess, it won't be too difficult to put a null pointer in C++ typeinfo place
as a sentinel.

--

https://issues.dlang.org/show_bug.cgi?id=21690

--- Comment #8 from thomas.bockman@gmail.com ---
(In reply to anonymous4 from comment #7)
> Calypso and dpp are the only hybrid D/C++ compilers around.

I knew about Calypso, but couldn't remember its name. Thanks for reminding me.

> I guess, it won't be too difficult to put a null pointer in C++ typeinfo place as a sentinel.

Good idea. If we can confirm by testing that this really turns unsafe dynamic_casts on the C++ side into null pointer exceptions / segmentation faults, then that plus a compile-time error on the D side should mitigate the security and memory safety issues well enough to drop this from "critical" to "enhancement".

--

https://issues.dlang.org/show_bug.cgi?id=21690

--- Comment #9 from Basile-z <b2.temp@gmx.com> ---
dynamic cast of c++ classes can work to a **very** limited extent that is if the target type is **exactly** the same (i.e not one of its derivate). This can be done by comparing the virtual table address of the instance with the virtual table address of the target type.

---
extern(C++) class A     { void f(){}}
extern(C++) class B : A { override void f(){} }
extern(C++) class C : A { }

extern(C++) T dyncast(T, U)(U u)
{
    const void* u_vtbl = *(cast(void**) u);
    const void* t_vtbl = &T.classinfo.vtbl[0];
    return t_vtbl is u_vtbl ? cast(T) u : null;
}

void main(string[] args)
{
    B b = new B;
    A a = b;
    C c = new C;

    assert(  dyncast!(B)(a) );
    assert( !dyncast!(B)(c) );
}
---

But in now way D CastExp does that for now.

--

https://issues.dlang.org/show_bug.cgi?id=21690

--- Comment #10 from thomas.bockman@gmail.com ---
(In reply to Basile-z from comment #9)
> dynamic cast of c++ classes can work to a **very** limited extent that is if the target type is **exactly** the same (i.e not one of its derivate). This can be done by comparing the virtual table address of the instance with the virtual table address of the target type.

Cool! That actually covers a lot of use cases, and is better than nothing. Maybe I'll publish a dub package with a more fleshed-out version of it eventually.

However, it seems a little too unpredictable and limited to be worth implementing in the compiler itself, given that kinke mentioned earlier that two instances of the same class may still end up pointing to different vtable addresses if they were instantiated on different sides of the language barrier:

(kinke from comment #1)
> But then again, when we are confronted with some C++ class ref, we have no idea whether it was instantiated on the D side or C++ side and so which vtable its vptr points to.

--