Skip to content

Create new class to handle external validation, and rename existing dll loading class. #7514

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

Merged
merged 56 commits into from
Aug 18, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
d7d1936
implement stage 2
bob80905 Jun 6, 2025
c527502
add crucial headers
bob80905 Jun 6, 2025
7f7dc30
chore: autopublish 2025-06-06T00:27:21Z
github-actions[bot] Jun 6, 2025
e7ec854
address Tex non-header changes
bob80905 Jun 6, 2025
39f913c
handle includes
bob80905 Jun 6, 2025
3700bfd
swap include order
bob80905 Jun 6, 2025
33c88f9
address Tex, and make a flawed attempt at writing a unit test
bob80905 Jun 18, 2025
544a9c7
chore: autopublish 2025-06-18T04:25:45Z
github-actions[bot] Jun 18, 2025
eeb1516
validated that local test passes
bob80905 Jun 18, 2025
9800c91
address tex, update unit test
bob80905 Jun 25, 2025
a4c9629
ifdef for windows vs unix env var addition
bob80905 Jun 25, 2025
a9282fd
add wstring conversion stuff and env var resets
bob80905 Jun 25, 2025
e781627
address tex
bob80905 Jun 25, 2025
2915a8d
special casing for unix systems
bob80905 Jun 25, 2025
57dd853
address tex, make wstrings more universal too
bob80905 Jun 30, 2025
98b25af
add includes for _wgetenv
bob80905 Jun 30, 2025
c274e5b
remove wstring for simplicity
bob80905 Jun 30, 2025
b46def4
fix undeclared variables in non windows case, and pointer lifetime bug
bob80905 Jun 30, 2025
b76fab2
adjust for build errors
bob80905 Jul 1, 2025
89dcc4a
address final forgotten comments
bob80905 Jul 1, 2025
acbf68a
try removing unicode from cmakelists
bob80905 Jul 2, 2025
f2890f5
update comment Justin pointed out was outdated
bob80905 Jul 3, 2025
eb1e96f
remove Linux definitions from windows only testing, address ambiguity…
bob80905 Jul 7, 2025
36c33a2
do not inherit from DxcDllSupport
bob80905 Jul 7, 2025
6577046
add win32 preprocessor gate to allow non windows envs to build
bob80905 Jul 7, 2025
80b50a6
remove test from non-windows since it depends on wide strings
bob80905 Jul 7, 2025
bfc508f
remove friend, remove protected:
bob80905 Jul 8, 2025
a144ca1
address Damyan final concerns
bob80905 Jul 11, 2025
bd01fa5
try another way to emit an hresult
bob80905 Jul 12, 2025
647c946
try E_INVALIDARG
bob80905 Jul 14, 2025
a29f6ee
use an interface
bob80905 Jul 18, 2025
fffb407
fix nix error
bob80905 Jul 18, 2025
5411894
remove comment on header that is likely to become stale
bob80905 Jul 24, 2025
45f74fa
perform rename
bob80905 Jul 28, 2025
64f61a8
resolve merge conflicts
bob80905 Jul 28, 2025
ed23ada
update old class name
bob80905 Jul 28, 2025
2383541
rename to IDllLoader, rename to OverrideDll, remove defaulted functio…
bob80905 Aug 6, 2025
209582f
address all of Justin + Damyan 2c
bob80905 Aug 7, 2025
daf6f0a
add override
bob80905 Aug 7, 2025
c8d867e
add more overrides
bob80905 Aug 7, 2025
0aa3c8d
add virtual to destructor for HlslIntellisense
bob80905 Aug 7, 2025
15fceac
address Damyan: specify constructors in interface, remove cleanup fro…
bob80905 Aug 7, 2025
09be489
address Damyan
bob80905 Aug 8, 2025
be99192
Merge branch 'main' into create_DxcDllExtValidationSupport
bob80905 Aug 8, 2025
cba829b
we keep trying, this should be it
bob80905 Aug 8, 2025
17f26c6
clang format
bob80905 Aug 8, 2025
c0ee094
remove move ctor and copy ctor
bob80905 Aug 8, 2025
cdf321e
address all but justins last comment
bob80905 Aug 11, 2025
c8b704a
do a function rename
bob80905 Aug 11, 2025
965f78c
try undoing spirv change
bob80905 Aug 12, 2025
48fb6fa
remove unneeded forward decl
bob80905 Aug 12, 2025
b100acd
add DxCompilerDllLoader class, and use it
bob80905 Aug 13, 2025
f66408c
address Justin
bob80905 Aug 13, 2025
c8bebee
fix build err
bob80905 Aug 13, 2025
4633dcc
remove specificdllloader from ensureenabled, use new LibraryDllLoader…
bob80905 Aug 14, 2025
005ea83
rename to DXCLibraryDllSupport, address Justin
bob80905 Aug 14, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 2 additions & 21 deletions include/dxc/Support/dxcapi.extval.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,26 +26,7 @@ class DxcDllExtValidationLoader : public DllLoader {

HRESULT Initialize() { return InitializeInternal("DxcCreateInstance"); }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why does InitializeInternal take a fnName argument if it's only ever called with a fixed string? Do we need InitializeInternal at all, or should we just update the implementation to be an implementation of Initialize()?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point, looks like we dont need it at all. I'll remove InitializeInternal and move its implementation to Initialize().


/* Note, OverrideDll takes this dll argument and ignores it
to satisfy the IDllLoader interface. The parameter is ignored
because the relevant dlls are specific and known: dxcompiler.dll
and dxil.dll. This class is not designed to handle any other dlls */
HRESULT OverrideDll(LPCSTR dll, LPCSTR entryPoint) {
return InitializeInternal(entryPoint);
}

bool HasCreateWithMalloc() const {
assert(DxCompilerSupport.HasCreateWithMalloc() &&
DxilExtValSupport.HasCreateWithMalloc());
return true;
}


bool IsEnabled() const override { return DxCompilerSupport.IsEnabled(); }

HMODULE Detach() {
// Can't Detach and return a handle for DxilSupport. Cleanup() instead.
DxilExtValSupport.Cleanup();
return DxCompilerSupport.Detach();
}
};
} // namespace dxc
} // namespace dxc
11 changes: 1 addition & 10 deletions include/dxc/Support/dxcapi.use.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class DllLoader {

public:
DllLoader() = default;
DllLoader(const DllLoader &) = delete;
DllLoader(const DllLoader &) = default; // needed for HlslIntellisenseSupport
Copy link
Member

Choose a reason for hiding this comment

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

Please mark this as deleted and remove the HlslIntellisenseSupport move constructor.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do we want to delete this solely because it's an interface class? Or is there some other reason?

Copy link
Member

Choose a reason for hiding this comment

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

There's no way for someone who has a pointer to a DllLoader instance to safely copy or move it, so we shouldn't have copy/move constructors for it. Plus, since there's no actual need to ever copy these we shouldn't be spending time trying to make it work. As it is now, the default copy constructor for SpecificDllLoader will end up leaking the dll HANDLE, so we don't want people trying to copy that.

DllLoader(DllLoader &&) = delete;

template <typename TInterface>
Expand Down Expand Up @@ -115,15 +115,6 @@ class SpecificDllLoader : public DllLoader {
SpecificDllLoader()
: m_dll(nullptr), m_createFn(nullptr), m_createFn2(nullptr) {}

SpecificDllLoader(SpecificDllLoader &&other) {
m_dll = other.m_dll;
other.m_dll = nullptr;
m_createFn = other.m_createFn;
other.m_createFn = nullptr;
m_createFn2 = other.m_createFn2;
other.m_createFn2 = nullptr;
}

~SpecificDllLoader() override { Cleanup(); }

HRESULT Initialize() {
Expand Down
Loading