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 optional path/directory argument for DXC/DXIL library #12

Merged
merged 3 commits into from
Dec 14, 2020

Conversation

MarijnS95
Copy link
Member

Depending on the platform/usecase the dlls/so's might not be available in PWD nor is it feasible for the application to change PWD on the fly. This does bloat the interface some but I don't feel like a static global and hassle_rs::set_library_path() are all that pretty either.

@Jasper-Bekkers
Copy link
Member

I think neither compile_hlsl nor the validate_dxil calls should get extra arguments for this more advanced case since they are basic helper functions used for simpler projects to get up and running quickly. Only the bigger underlying structs Dxc and Dxil should get them as more advanced users are expected to use them directly.

src/wrapper.rs Outdated Show resolved Hide resolved
@Jasper-Bekkers Jasper-Bekkers merged commit acf2e4d into master Dec 14, 2020
@MarijnS95 MarijnS95 deleted the custom-lib-path branch December 14, 2020 13:31
MarijnS95 added a commit that referenced this pull request Jan 12, 2022
…of a panic

Not all file loading should be fallible; DXC usually invokes the loader
with multiple paths depending on relative includes, and happily accepts
and expects us to return `-2_147_024_894 // ERROR_FILE_NOT_FOUND /
0x80070002` for some.  As such the precedent should be to not panic but
let DXC decide if file loading failures should be critical or simply
result in a different path being tested.

In my case though the error seems to originate from DXIL compilation
being largely untested when ran on Linux systems through the WinAadapter
codepath.  In this particular stacktrace below DXC seems to "stat" (get
file information) for the current working directory, and somehow ends up
trying to open the directory as file through our include handler:

    #11 dxcutil::DxcArgsFileSystemImpl::TryFindOrOpen (this=<optimized out>, this@entry=0x5570a3b0ae10, lpFileName=lpFileName@entry=0x5570a3b5d0d0 L".../hassle-rs", index=<optimized out>)
        at .../DirectXShaderCompiler/tools/clang/tools/dxcompiler/dxcfilesystem.cpp:327
    #12 0x00007f6e64a41a8d in dxcutil::DxcArgsFileSystemImpl::GetFileAttributesW (this=0x5570a3b0ae10, lpFileName=0x5570a3b5d0d0 L".../hassle-rs")
        at .../DirectXShaderCompiler/tools/clang/tools/dxcompiler/dxcfilesystem.cpp:570
    #13 0x00007f6e64a42048 in dxcutil::DxcArgsFileSystemImpl::Stat (this=0x5570a3b0ae10, lpFileName=<optimized out>, Status=0x7fff9faec1e8)
        at .../DirectXShaderCompiler/tools/clang/tools/dxcompiler/dxcfilesystem.cpp:807
    #14 0x00007f6e64debc91 in llvm::sys::fs::status (Path=..., Result=...) at .../DirectXShaderCompiler/lib/Support/Unix/Path.inc:385
    #15 0x00007f6e64dec708 in llvm::sys::fs::current_path (result=...) at .../DirectXShaderCompiler/lib/Support/Unix/Path.inc:195
    #16 0x00007f6e64e64535 in clang::CodeGen::CGDebugInfo::getCurrentDirname (this=this@entry=0x5570a3b598f0)
        at .../DirectXShaderCompiler/tools/clang/lib/CodeGen/CGDebugInfo.cpp:301
    #17 0x00007f6e64e6230e in clang::CodeGen::CGDebugInfo::CreateCompileUnit (this=this@entry=0x5570a3b598f0)
        at .../DirectXShaderCompiler/tools/clang/lib/CodeGen/CGDebugInfo.cpp:369

This stacktrace originates from the following panic:

    thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 21, kind: IsADirectory, message: "Is a directory" }', src/utils.rs:51:48
MarijnS95 added a commit that referenced this pull request Jan 25, 2022
…of a panic (#27)

Not all file loading should be fallible; DXC usually invokes the loader
with multiple paths depending on relative includes, and happily accepts
and expects us to return `-2_147_024_894 // ERROR_FILE_NOT_FOUND /
0x80070002` for some.  As such the precedent should be to not panic but
let DXC decide if file loading failures should be critical or simply
result in a different path being tested.

In my case though the error seems to originate from DXIL compilation
being largely untested when ran on Linux systems through the WinAadapter
codepath.  In this particular stacktrace below DXC seems to "stat" (get
file information) for the current working directory, and somehow ends up
trying to open the directory as file through our include handler:

    #11 dxcutil::DxcArgsFileSystemImpl::TryFindOrOpen (this=<optimized out>, this@entry=0x5570a3b0ae10, lpFileName=lpFileName@entry=0x5570a3b5d0d0 L".../hassle-rs", index=<optimized out>)
        at .../DirectXShaderCompiler/tools/clang/tools/dxcompiler/dxcfilesystem.cpp:327
    #12 0x00007f6e64a41a8d in dxcutil::DxcArgsFileSystemImpl::GetFileAttributesW (this=0x5570a3b0ae10, lpFileName=0x5570a3b5d0d0 L".../hassle-rs")
        at .../DirectXShaderCompiler/tools/clang/tools/dxcompiler/dxcfilesystem.cpp:570
    #13 0x00007f6e64a42048 in dxcutil::DxcArgsFileSystemImpl::Stat (this=0x5570a3b0ae10, lpFileName=<optimized out>, Status=0x7fff9faec1e8)
        at .../DirectXShaderCompiler/tools/clang/tools/dxcompiler/dxcfilesystem.cpp:807
    #14 0x00007f6e64debc91 in llvm::sys::fs::status (Path=..., Result=...) at .../DirectXShaderCompiler/lib/Support/Unix/Path.inc:385
    #15 0x00007f6e64dec708 in llvm::sys::fs::current_path (result=...) at .../DirectXShaderCompiler/lib/Support/Unix/Path.inc:195
    #16 0x00007f6e64e64535 in clang::CodeGen::CGDebugInfo::getCurrentDirname (this=this@entry=0x5570a3b598f0)
        at .../DirectXShaderCompiler/tools/clang/lib/CodeGen/CGDebugInfo.cpp:301
    #17 0x00007f6e64e6230e in clang::CodeGen::CGDebugInfo::CreateCompileUnit (this=this@entry=0x5570a3b598f0)
        at .../DirectXShaderCompiler/tools/clang/lib/CodeGen/CGDebugInfo.cpp:369

This stacktrace originates from the following panic:

    thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 21, kind: IsADirectory, message: "Is a directory" }', src/utils.rs:51:48
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

Successfully merging this pull request may close these issues.

2 participants