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

Add support for initializing vtable pointer of types without constructors #31

Open
PathogenDavid opened this issue Sep 10, 2020 · 4 comments
Labels
Area-OutputGeneration Issues concerning the process of generating output from Biohazrd Blocks-PhysX Concept-CppFeatures Issues concerning unsupported C++ features Concept-InlineExpectation Issues concerning problems around C++'s expectation for something to be inlined. Concept-OutputUsability Issues concerning whether or not the output from Biohazrd is actually usable

Comments

@PathogenDavid
Copy link
Member

For types without constructors, the C++ compiler may initialize the vtable pointer wherever the type is initialized.

This was encountered with PxDefaultAllocator.

Here's a simple example: (Godbolt)

class AbstractBase
{
public:
    virtual void* allocate(int size);
};

class DefaultImpl : public AbstractBase
{
public:
    void* allocate(int size)
    {
        return nullptr;
    }
};

void UseAllocator(AbstractBase* allocator)
{
    allocator->allocate(100);
}

void Test()
{
    DefaultImpl impl;
    UseAllocator(&impl);
}

With GCC x86-64 10.2, Test is generated as follows:

Test():
        push    rbp
        mov     rbp, rsp
        sub     rsp, 16
        mov     eax, OFFSET FLAT:vtable for DefaultImpl+16
        mov     QWORD PTR [rbp-8], rax
        lea     rax, [rbp-8]
        mov     rdi, rax
        call    UseAllocator(AbstractBase*)
        nop
        leave
        ret

The easiest solution here is probably to use C trampolines for object construction (when necessary?)

@PathogenDavid PathogenDavid added Area-OutputGeneration Issues concerning the process of generating output from Biohazrd Concept-CppFeatures Issues concerning unsupported C++ features Concept-OutputUsability Issues concerning whether or not the output from Biohazrd is actually usable labels Sep 21, 2020
@PathogenDavid
Copy link
Member Author

Some C++ compilers will sometimes generate an implicit constructor to handle this.

For example, x86-64 clang 10.0.0 on Godbolt produces:

Test():                               # @Test()
        push    rbp
        mov     rbp, rsp
        sub     rsp, 16
        lea     rdi, [rbp - 8]
        call    DefaultImpl::DefaultImpl() [base object constructor]
        lea     rax, [rbp - 8]
        mov     rdi, rax
        call    UseAllocator(AbstractBase*)
        add     rsp, 16
        pop     rbp
        ret

and x64 msvc v19.27:

impl$ = 32
void Test(void) PROC                                 ; Test
$LN3:
        sub     rsp, 56                             ; 00000038H
        lea     rcx, QWORD PTR impl$[rsp]
        call    DefaultImpl::DefaultImpl(void)
        lea     rcx, QWORD PTR impl$[rsp]
        call    void UseAllocator(AbstractBase *)  ; UseAllocator
        add     rsp, 56                             ; 00000038H
        ret     0
void Test(void) ENDP                                 ; Test

@PathogenDavid PathogenDavid added the Concept-InlineExpectation Issues concerning problems around C++'s expectation for something to be inlined. label Sep 21, 2020
@PathogenDavid PathogenDavid mentioned this issue Nov 9, 2020
6 tasks
@PathogenDavid PathogenDavid mentioned this issue Dec 21, 2020
8 tasks
@PathogenDavid
Copy link
Member Author

PathogenDavid commented Dec 6, 2021

This is blocking the use of PxDefaultAllocator in PhysX. We should just leverage the inline export helper infrastructure for this.

@PathogenDavid
Copy link
Member Author

Turns out this will be a bit of a pain since we can't synthesize TranslatedFunctions and InlineExportHelper assumes we have a `ClanSharp.FunctionDecl.

We'll probably need to add synthesizing functions (which is very odd outside this situation) as well as special-case for generating these trampolines. It might also be worth checking if Clang is internally synthesizing a constructor declaration or not so we can maybe avoid doing it ourselves.

PathogenDavid added a commit that referenced this issue Dec 19, 2021
…nstructors since they can't be initialized from C# yet.

Relates to #31
PathogenDavid added a commit to PathogenPlayground/Mochi.PhysX that referenced this issue Dec 19, 2021
@PathogenDavid
Copy link
Member Author

I added a warning to CSharpTranslationVerifier to help alert people to this missing feature and to make it easier to enumerate the types affected by it. (Although ideally we should just add this sooner rather than later.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-OutputGeneration Issues concerning the process of generating output from Biohazrd Blocks-PhysX Concept-CppFeatures Issues concerning unsupported C++ features Concept-InlineExpectation Issues concerning problems around C++'s expectation for something to be inlined. Concept-OutputUsability Issues concerning whether or not the output from Biohazrd is actually usable
Projects
None yet
Development

No branches or pull requests

1 participant