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

Scalar deleting destructor link error #13

Open
lucianpin opened this issue Mar 22, 2023 · 2 comments
Open

Scalar deleting destructor link error #13

lucianpin opened this issue Mar 22, 2023 · 2 comments

Comments

@lucianpin
Copy link

lucianpin commented Mar 22, 2023

The issue occurred as soon as some struct allocated through jxy::make_unique contained some members of other class types that required a constructor and destructor.

struct Globals
{
    Globals() : port(PORT_NAME)
    {
    }

    CommunicationPort port;
};

class CommunicationPort
{
public:

    CommunicationPort(_In_ const wchar_t * portName) : m_portName(portName)
    {
    }

private:

    const jxy::wstring<NonPagedPool, TAG> m_portName;
};

The error: LNK2019: unresolved external symbol "void __cdecl operator delete(void *,unsigned __int64)" (??3@YAXPEAX_K@Z) referenced in function "public: void * __cdecl SomeClass::'scalar deleting destructor'(unsigned int)"

Background: So what is 'scalar deleting destructor'(unsigned int)? According to this article: https://ofekshilon.com/2014/06/09/on-vector-deleting-destructors-and-some-newdelete-internals/

the compiler does not call ~Whatever() and operator-delete directly, but rather generates and invokes a helper function that wraps these two calls. This helper is called scalar deleting destructor

The unsigned int parameter is a flag telling if the function should perform a vector deleting destructor or a scalar deleting destructor and whether to deallocate memory, which is optional.

Raymond Chen spelled out pseudo code for it:

void Whatever::vector deleting destructor(int flags)
{
  if (flags & 2) { // if vector destruct
    size_t* a = reinterpret_cast<size_t*>(this) - 1;
    size_t howmany = *a;
    vector destructor iterator(p, sizeof(Whatever),
      howmany, Whatever::~Whatever);
    if (flags & 1) { // if delete too
      operator delete(a);
    }
  } else { // else scalar destruct
    this->~Whatever(); // destruct one
    if (flags & 1) { // if delete too
      operator delete(this);
    }
  }
}

The actual issue:
11111

When the default deleter calls Pointer->~T(), the compiler is using the hidden wrapper function SomeClass::'scalar deleting destructor', but with flag set not to perform deallocation because obviously the intention was only to call de destructor of SomeClass.

222222

It can be seen that flag is tested and if not set, after calling the destructor, it would jump at the end without calling the operator delete.

However, the code calling operator delete is there so the compiler needs the definition for it. But Jxy library is not providing any code for such signature: void __cdecl operator delete(void *,unsigned __int64) because no code path should presumably call it. In kernel-mode we would eventually use void __cdecl operator delete(void* Memory, POOL_TYPE PoolType, ULONG PoolTag). So that explains the link error.

Solution would be to define void __cdecl operator delete(void* Memory, size_t Size) and maybe also void __cdecl operator delete(void* Memory) noexcept, but to issue a bug check if these operators would ever be called - that should not happen. All this just because the compiler is using the multi-purpose wrapper and the code inside needs to link also to not used delete operator in kernel.

Any other ideas or thoughts?

@jxy-s
Copy link
Owner

jxy-s commented Mar 22, 2023

This should already be linked to by way of the vcrtl dependency: https://github.com/avakar/vcrtl/blob/master/src/runtime.cpp

Is it possible to provide an example as to how you're using jxy::unique_ptr and jxy_make_shared? I just put in this unit test to the project by inferring how you might be using using it and it compiles fine.

➜ git diff
diff --git a/stltest/memory_tests.cpp b/stltest/memory_tests.cpp
index 97ffcdc..7892139 100644
--- a/stltest/memory_tests.cpp
+++ b/stltest/memory_tests.cpp
@@ -7,9 +7,22 @@
 #include "tests_common.hpp"
 #include <jxy/memory.hpp>

+#include <jxy/string.hpp>
+
 namespace jxy::Tests
 {

+struct TestObject
+{
+    ~TestObject() = default;
+
+    TestObject(_In_ const wchar_t* n) : Name(n)
+    {
+    }
+
+    const jxy::wstring<NonPagedPoolNx, '0GAT'> Name;
+};
+
 void MemoryTests()
 {
     {
@@ -70,6 +83,10 @@ void MemoryTests()
         auto ptr = jxy::make_shared<int, NonPagedPoolNx, '0GAT'>(1);
         UT_ASSERT(*ptr == 1);
     }
+    {
+        auto ptr = jxy::make_shared<TestObject, NonPagedPoolNx, '0GAT'>(L"Testing");
+        UT_ASSERT(ptr->Name == L"Testing");
+    }
 }

That said, there is still a problem with storing an object that requires virtual destruction in a unique_ptr. This is because of an assumption that is made in the code and the difference between how smart pointers in the standard manage the allocator that was used. unique_ptr is a little special... since it assumes the supplied deleter has the information necessary in the inheritance tree. By "default" the STL deleter leverages the compiler-specific semantics of the "default" delete in the case of virtual inheritance. As you've explained. But in the case of storing a pointer to a virtual interface then expecting to delete it later inside of a unique_ptr, it will free the incorrect pointer, for example:

jxy::unique_ptr<IInterface, PagedPool, '0GAT'> ptr(new (PagedPool, '0GAT') Implementation());

In this scenario we will store an offset pointer, destruct through the scalar deleting destructor, then free the virtual pointer - when in reality we should have freed the implementation pointer.

That said, there is a property of how MSVC implements the scalar deleting destructor where (despite there technically not being a return value (e.g. ptr->~IInterface();) - the call does in fact store the "final" pointer in rax. Meaning, we could amend the way the default deleter works, but it would be a a compiler-specific "hack" (I guess that's fine given the goals of this library, however). Alternatively I should investigate pairing the tagged delete implementations in the default deleter instead.

Regardless, what you're explaining should work for the time being since the implementation mentioned previously for delete should be handled by the vcrtl - I expect possibly the build settings are incorrect for your project?

I will investigate the problem explained previously when I have time. But, I expect it isn't what you're encountering, based on what you've described.

@lucianpin
Copy link
Author

Thank you for your quick response and explanation.
Yes, how you wrote in the example is more or less the same as I did, simple jxy::make_shared.

Because you mentioned about vcrtl dependency, I immediately realized that I am working on a fork (https://github.com/lucianpin/stlkrn/tree/main-no-exceptions) where I deliberately switched off exceptions and removed the vcrtl dependency. All I wanted was the STL in kernel even with no exception handling and your repository is the winner.

Now all seems clear, I really overlooked the https://github.com/avakar/vcrtl/blob/master/src/runtime.cpp. So there is no need for further investigation. I must provide that code on my own.

Thanks again and your work and info is really appreciated.

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