Runtime loading dxcompiler on mac/linux

[smbradley]

Hi, I'm wondering if there's currently a supported way to dlopen dxcompiler on mac/linux without linking against the lib. There doesn't seem to be a way to grab the REFIID for types without having access to the static Interface::Interface_ID symbol which will be name mangled.

Would adding an exported C function that can do a lookup be a viable option, or is there some existing workaround that I've missed?

[pow2clk]

I looked into some aspects of this over a year ago at this point. I'm afraid I don't remember terribly many details. I can tell you that the way dxc is linked for Linux and MacOS is rather fragile in part due to these GUIDs. There may be another way to generate them that makes this possible. I think @ehsannas (when he gets back from leave) and/or @jaebaek are probably the ones to ask about it at this point.

Worth noting, during my investigation, I was solely considering how better to link the dxc compiler. Loading the dxcompiler library directly might evade some of these problems. There are some allocators that get need to get initialized that were occasionally missed as well.

[jkunstwald]

@rellensb If you don't mind, how are you linking against dxc on linux in the first place? I can't seem to be able to resolve the references to IDXcLibrary::IDxcLibrary_ID and IDXcCompiler::IDxcCompiler_ID. Im linking against libdxcompiler.so.3.7 and libdxclib.a.

[smbradley]

@pow2clk thanks for the reply. I've patched in a string lookup locally which seems to be working ok, but it'd be good to work out an upstream solution if possible.

@jkunstwald in terms of compile time linking? Nothing special, just regular linking against the .so (eg. assuming gcc: -Lpath/to/library -ldxcompiler ). It might be because the symbols are actually missing from your built library (you can check with nm -DC libdxcompiler.so.3.7). From looking at WinAdapter.h I think UUID emulation is disabled when building with clang on Linux.

[jkunstwald]

@rellensb Thank you! Forcing __EMULATE_UUID on clang did the trick.

[smbradley]

@ehsannas @jaebaek any thoughts on this?

[ehsannas]

Hey @rellensb ! Sorry for the delay. I was on leave for a while. Were you able to use dlopen without linking the lib? If you were able to get it working with some code changes, please feel free to make a pull-request. Your contributions would be much appreciated. I currently don't have the bandwidth to do this work.

(side note: There has been some improvement in the generation of UUIDs (#2796))

[MarijnS95]

@ehsannas I'm particularly interested in a method that retains compatibility with external FFI wrappers (from other languages) that dlopen the library and rely on passing a full UUID (which, afaik, is the intended usecase). While #2796 gives a consistent hash those wrappers need changes to pass the hash instead of pointer to UUID when using a GCC-compiled DXC. As it stands the virtual interfaces are incompatible too (on clang as well; either padding, virtual destructor or something else: allow me to validate what it is).

After locally playing with this for some time, here are my thoughts:

• These UUIDs should be truly cross-platform and accept a proper 128-bit GUID wherever they are used;
• They should be easy to define, without requiring macros for definition and declaration in separate places;
• The UUID is preferably only written down exactly once.

Then, the nitty-gritty details on how I came to the final implementation adhering to those rules:

The existing macros are updated to take a rewritten variant of the GUID. These are returned as REFIID from uuidof() and that pretty much completes the solution, but requires the GUID to be specified twice, in odd formats. Returning a REFIID requires a static variable which can not be defined in-class:

error: ‘constexpr’ needed for in-class initialization of static data member ‘const IID IUnknown::_IID’ of non-integral type

Unfortunately static inline constexpr is a C++17 feature which I would have used to define IIDs inline without having a definition outside the header with the GUID. As a workaround a static local variable can be used inside uuidof().

Yet this relies on specifying the UUID twice - in different formats - which is not ideal. Fortunately some macro magic allows a struct header to be created that generates the __declspec(uuid(...)) or static const IID _IID = {...}; based on the platform.

When this question (for #1342) was posted over at dotnet the suggestion came up to use a trait type with template specializations that contains these IIDs. This is used in the final revision with the advantage that inherited types do not need to be passed through the macro, because it doesn't emit the opening brace { to contain the members anymore.
The disadvantage is that uuids can no longer be retrieved through ::uuidof() as they are now part of this trait type instead. This means taking the IID of a subclass that itself doesn't carry an IID, if this usecase even is allowed by __uuidof() on Windows, will no longer be supported:

struct MyImplementationForICustomInterface : public IMyCustomInterface {
	// ...
};
// The following is expected to return the IID of IMyCustomInterface, as MyImplementationForICustomInterface is simply an implementation of that interface which does not have an IID itself:
auto unknown_uuid = __uuidof(MyImplementationForICustomInterface);
// Error: No template specialization for TInterfaceIids<MyImplementationForICustomInterface>

This seems to be a valid usecase where an implementation creates various specializations of the same interface by means of sub-classing.

All changes can be found on this branch that compiles and runs on GCC and Clang on Linux, and MSVC on Windows. This branch includes a couple picks to reduce warnings (though more are needed, GCC and Clang are pretty verbose about this project), as well as the necessary updates to CMakeSettings.json to get it to work properly under VS 2019. Let me know if these you want these upstreamed too. The docs here don't mention anything about the new "variables" layout being incompatible with 2017.
(Final note: The code has not been properly autoformatted yet, and the duplicate macro definition should probably be moved to its own separate header file).

Looking forward to your thoughts and suggestions!

[ehsannas]

Hi @MarijnS95
Thanks for putting in the time for the code and for the explanations.

Do I understand correctly that usages of __uuidof(...) would not work? There are many usages of this in the repository (example).

[MarijnS95]

Hi @ehsannas: All instances of __uuidof(...) in dxc work just fine, otherwise I wouldn't be able to compile and use this daily 😉

The issue lies specifically in the use of subclasses that themselves don't carry an IID. I don't know how common (if at all) this case is. See the code example given in my comment.

EDIT: Apologies for the confusion, looks like I totally messed up that section while rewriting the comment. It is now rewritten to mention this subclass specialization explicitly.

[ehsannas]

Oh I see. Let me take a closer look at this later this week. Thanks

[ehsannas]

@MarijnS95 Sorry for the delay. What you have done looks reasonable to me. Would you please post a pull-request? We can add any improvement suggestions there. Thanks!

[MarijnS95]

@ehsannas took me some time as well to set up the PR, you can find it here now: #3062

As for the vtable issue, the problem is virtual ~IUnknown(); generating a complete object destructor and deleting destructor:

VTable in GDB

(gdb) info vtbl this->m_Library
vtable for 'DxcLibrary' @ 0x7ffff7cbc5f8 (subobject @ 0x5555556bb9e0):
[0]: 0x7ffff6a56d40 <DxcLibrary::QueryInterface(_GUID const&, void**)>
[1]: 0x7ffff6a56d20 <DxcLibrary::AddRef()>
[2]: 0x7ffff6a56d30 <DxcLibrary::Release()>
[3]: 0x7ffff6b36bc0 <IUnknown::~IUnknown()> // Complete object destructor
[4]: 0x7ffff6a57130 <DxcLibrary::~DxcLibrary()> // Deleting destructor
[5]: 0x7ffff6a56d50 <DxcLibrary::SetMalloc(IMalloc*)>
[6]: 0x7ffff6a56d60 <DxcLibrary::CreateBlobFromBlob(IDxcBlob*, unsigned int, unsigned int, IDxcBlob**)>
[7]: 0x7ffff6a56d70 <DxcLibrary::CreateBlobFromFile(wchar_t const*, unsigned int*, IDxcBlobEncoding**)>
[8]: 0x7ffff6a56d80 <DxcLibrary::CreateBlobWithEncodingFromPinned(void const*, unsigned int, unsigned int, IDxcBlobEncoding**)>
[9]: 0x7ffff6a56d90 <DxcLibrary::CreateBlobWithEncodingOnHeapCopy(void const*, unsigned int, unsigned int, IDxcBlobEncoding**)>
[10]: 0x7ffff6a56da0 <DxcLibrary::CreateBlobWithEncodingOnMalloc(void const*, IMalloc*, unsigned int, unsigned int, IDxcBlobEncoding**)>
[11]: 0x7ffff6a56db0 <DxcLibrary::CreateIncludeHandler(IDxcIncludeHandler**)>
[12]: 0x7ffff6a56dc0 <DxcLibrary::CreateStreamFromBlobReadOnly(IDxcBlob*, IStream**)>
[13]: 0x7ffff6a56dd0 <DxcLibrary::GetBlobAsUtf8(IDxcBlob*, IDxcBlobEncoding**)>
[14]: 0x7ffff6a56e90 <DxcLibrary::GetBlobAsUtf16(IDxcBlob*, IDxcBlobEncoding**)>

This breaks vtable layout and requires external wrappers (in a different language, not compiled with dxc headers) to insert extra padding, as we do in our Rust wrapper here.

The destructors are actively used in IUnknown::Release() though I doubt objects should be freed that way seeing most implementation instances are created and freed using DXC_MICROCOM_TM_{ALLOC,ADDREF_RELEASE_IMPL} except the few in WinAdapter.

Perhaps the destructor can be removed or replaced with a nonvirtual one (with the resulting warning disabled) that is directly called, followed by an explicit free?

[smbradley]

Thanks for pushing this forward everyone!