Fix error source; free leaked allocations; fix refcounting; BSTR is w…

…ide, not UTF-8; convert return_hr to expression (#10)

* utils: Mark libloading error as source

In case we want to walk the .source() trace and print all inner
exceptions (when these are usually rewrapped in thiserror types in
higher layers), this exception needs to be marked as source to be
returned from that function.

* intellisense: Simplify DxcCursor retrieval and free leaked allocation

Solves the following Valgrind trace:

    ==168242== 24 bytes in 1 blocks are definitely lost in loss record 2 of 14
    ==168242==    at 0x483A77F: malloc (vg_replace_malloc.c:307)
    ==168242==    by 0x5AFC3F4: ???
    ==168242==    by 0x5AFCFB3: ???
    ==168242==    by 0x12BD23: hassle_rs::intellisense::ffi::IDxcCursor::get_children (macros.rs:108)
    ==168242==    by 0x1167E6: hassle_rs::intellisense::wrapper::DxcCursor::get_all_children (wrapper.rs:210)
    ==168242==    by 0x111B8A: intellisense_tu::main (intellisense-tu.rs:43)
    ==168242==    by 0x112F5A: core::ops::function::FnOnce::call_once (function.rs:227)
    ==168242==    by 0x1132FD: std::sys_common::backtrace::__rust_begin_short_backtrace (backtrace.rs:137)
    ==168242==    by 0x113510: std::rt::lang_start::{{closure}} (rt.rs:66)
    ==168242==    by 0x14F810: call_once<(),Fn<()>> (function.rs:259)
    ==168242==    by 0x14F810: do_call<&Fn<()>,i32> (panicking.rs:373)
    ==168242==    by 0x14F810: try<i32,&Fn<()>> (panicking.rs:337)
    ==168242==    by 0x14F810: catch_unwind<&Fn<()>,i32> (panic.rs:379)
    ==168242==    by 0x14F810: std::rt::lang_start_internal (rt.rs:51)
    ==168242==    by 0x1134E6: std::rt::lang_start (rt.rs:65)

* Convert return_hr to expression

Make return_hr more versatile by providing Ok or Err as expession
instead of forcing a return out of the current function. This follows
Rusts convention to provide a block result as an unterminated expression
on the last line, and allows it to be used in an expression context
including with the question mark operator.

* Free allocated memory using the appropriate API (free/CoTaskMemFree)

On Windows the right API needs to be used to free this pointer in the
same way as it was allocated by DXC (using CoTaskMemAlloc). On
non-Windows platforms this call is [delegated] to `free` from libc.

[delegated]: https://github.com/microsoft/DirectXShaderCompiler/blob/a8d9780046cb64a1cea842fa6fc28a250e3e2c09/include/dxc/Support/WinAdapter.h#L46

* Linux: util: Free BSTR after conversion to String

The same call to SysFreeString happens in the Windows counterpart.

Solves the following leaks found by valgrind:

    ==213075== 40 bytes in 1 blocks are definitely lost in loss record 4 of 13
    ==213075==    at 0x483A77F: malloc (vg_replace_malloc.c:307)
    ==213075==    by 0x5B03395: ???
    ==213075==    by 0x5AFC77E: ???
    ==213075==    by 0x12C766: hassle_rs::intellisense::ffi::IDxcCursor::get_display_name (macros.rs:108)
    ==213075==    by 0x116373: hassle_rs::intellisense::wrapper::DxcCursor::get_display_name (wrapper.rs:236)
    ==213075==    by 0x11286D: intellisense_tu::main (intellisense-tu.rs:33)
    ==213075==    by 0x11371A: core::ops::function::FnOnce::call_once (function.rs:227)
    ==213075==    by 0x11348D: std::sys_common::backtrace::__rust_begin_short_backtrace (backtrace.rs:137)
    ==213075==    by 0x111E10: std::rt::lang_start::{{closure}} (rt.rs:66)
    ==213075==    by 0x150A20: call_once<(),Fn<()>> (function.rs:259)
    ==213075==    by 0x150A20: do_call<&Fn<()>,i32> (panicking.rs:373)
    ==213075==    by 0x150A20: try<i32,&Fn<()>> (panicking.rs:337)
    ==213075==    by 0x150A20: catch_unwind<&Fn<()>,i32> (panic.rs:379)
    ==213075==    by 0x150A20: std::rt::lang_start_internal (rt.rs:51)
    ==213075==    by 0x111DE6: std::rt::lang_start (rt.rs:65)
    ==213075==
    ==213075== 56 (16 direct, 40 indirect) bytes in 1 blocks are definitely lost in loss record 5 of 13
    ==213075==    at 0x483ADEF: operator new(unsigned long) (vg_replace_malloc.c:342)
    ==213075==    by 0x4FD058D: ???
    ==213075==    by 0x4FC04D9: ???
    ==213075==    by 0x40112DD: call_init.part.0 (in /usr/lib/ld-2.32.so)
    ==213075==    by 0x40113C7: _dl_init (in /usr/lib/ld-2.32.so)
    ==213075==    by 0x4A100E4: _dl_catch_exception (in /usr/lib/libc-2.32.so)
    ==213075==    by 0x4015704: dl_open_worker (in /usr/lib/ld-2.32.so)
    ==213075==    by 0x4A10087: _dl_catch_exception (in /usr/lib/libc-2.32.so)
    ==213075==    by 0x4014F3D: _dl_open (in /usr/lib/ld-2.32.so)
    ==213075==    by 0x489434B: ??? (in /usr/lib/libdl-2.32.so)
    ==213075==    by 0x4A10087: _dl_catch_exception (in /usr/lib/libc-2.32.so)
    ==213075==    by 0x4A10152: _dl_catch_error (in /usr/lib/libc-2.32.so)
    ==213075==
    ==213075== 124 bytes in 3 blocks are definitely lost in loss record 9 of 13
    ==213075==    at 0x483A77F: malloc (vg_replace_malloc.c:307)
    ==213075==    by 0x5B03395: ???
    ==213075==    by 0x5AFC77E: ???
    ==213075==    by 0x12C766: hassle_rs::intellisense::ffi::IDxcCursor::get_display_name (macros.rs:108)
    ==213075==    by 0x116373: hassle_rs::intellisense::wrapper::DxcCursor::get_display_name (wrapper.rs:236)
    ==213075==    by 0x112F63: intellisense_tu::main (intellisense-tu.rs:52)
    ==213075==    by 0x11371A: core::ops::function::FnOnce::call_once (function.rs:227)
    ==213075==    by 0x11348D: std::sys_common::backtrace::__rust_begin_short_backtrace (backtrace.rs:137)
    ==213075==    by 0x111E10: std::rt::lang_start::{{closure}} (rt.rs:66)
    ==213075==    by 0x150A20: call_once<(),Fn<()>> (function.rs:259)
    ==213075==    by 0x150A20: do_call<&Fn<()>,i32> (panicking.rs:373)
    ==213075==    by 0x150A20: try<i32,&Fn<()>> (panicking.rs:337)
    ==213075==    by 0x150A20: catch_unwind<&Fn<()>,i32> (panic.rs:379)
    ==213075==    by 0x150A20: std::rt::lang_start_internal (rt.rs:51)
    ==213075==    by 0x111DE6: std::rt::lang_start (rt.rs:65)

Fixes: 94fbad7 ("Support libdxcompiler.so on Linux (#5)")

* Linux: util: BSTR contains a wide string instead of UTF-8

Resulting strings were only one character long because the second byte
for ASCII text is NULL.

On Linux DXC properly uses wchar_t which is 32-bit instead of 16-bit.

Fixes: 94fbad7 ("Support libdxcompiler.so on Linux (#5)")

* Free leaking DxcIncludeHandlerWrapper

Valgrind shows that we are leaking the DxcIncludeHandlerWrapper
allocation:

    ==96758== 485 (80 direct, 405 indirect) bytes in 1 blocks are definitely lost in loss record 13 of 17
    ==96758==    at 0x483A77F: malloc (vg_replace_malloc.c:307)
    ==96758==    by 0x1A1E4B: alloc::alloc::alloc (alloc.rs:74)
    ==96758==    by 0x1A1F09: alloc::alloc::Global::alloc_impl (alloc.rs:153)
    ==96758==    by 0x1A2069: <alloc::alloc::Global as core::alloc::AllocRef>::alloc (alloc.rs:203)
    ==96758==    by 0x1A1D9A: alloc::alloc::exchange_malloc (alloc.rs:281)
    ==96758==    by 0x199A1A: new<hassle_rs::wrapper::DxcIncludeHandlerWrapper> (boxed.rs:175)
    ==96758==    by 0x199A1A: hassle_rs::wrapper::DxcCompiler::prep_include_handler (wrapper.rs:248)
    ==96758==    by 0x199D6F: hassle_rs::wrapper::DxcCompiler::compile (wrapper.rs:278)
    ==96758==    by 0x195C70: hassle_rs::utils::compile_hlsl (utils.rs:115)
    ==96758==    by 0x127BDF: include::main (autogen_ops.rs:0)
    ==96758==    by 0x1275CA: core::ops::function::FnOnce::call_once (autogen_context.rs:1683)
    ==96758==    by 0x127ADD: std::sys_common::backtrace::__rust_begin_short_backtrace (backtrace.rs:137)
    ==96758==    by 0x127530: std::rt::lang_start::{{closure}} (rt.rs:66)

This happens because of Box::into_raw, which moves the pointer out of
the box. Taking this pointer out of a referenced Box makes it live on
till the end of the function where it is appropriately dropped.

Properly freeing DxcIncludeHandlerWrapper exposes a second issue: The
blobs are now freed but have already been freed within DXC becaues of a
missing refcount: we are supposed to increment it before writing the
pointer to it to `include_source` [1].
Now that the blobs have a proper refcount and live on until DXC cleans
them up when not needed, they do not need to be kept alive inside
DxcIncludeHandlerWrapper anymore.

Finally, clean up the unnecessary clone of `source`: This String is
already retrieved by value and can be moved straight into the Rc.

[1]: https://github.com/microsoft/DirectXShaderCompiler/blob/a8d9780046cb64a1cea842fa6fc28a250e3e2c09/tools/clang/unittests/dxc_batch/dxc_batch.cpp#L553-L554

Cargo.toml

@@ -31,7 +31,10 @@ widestring = "0.4.2"
 thiserror = "1.0"
 
 [target.'cfg(windows)'.dependencies]
-winapi = { version = "0.3.9", features = ["wtypes", "oleauto"] }
+winapi = { version = "0.3.9", features = ["wtypes", "oleauto", "combaseapi"] }
+
+[target.'cfg(not(windows))'.dependencies]
+libc = "0.2"
 
 [dev-dependencies]
 rspirv = "0.6.0"

examples/intellisense-tu.rs

@@ -32,6 +32,7 @@ fn main() {
 
         let name = cursor.get_display_name().unwrap();
         println!("Name {:?}", name);
+        assert_eq!(name, "copy.hlsl");
 
         let cursor_kind = cursor.get_kind().unwrap();
         println!("CursorKind {:?}", cursor_kind);
@@ -42,6 +43,19 @@ fn main() {
 
     let child_cursors = cursor.get_all_children().unwrap();
 
+    assert_eq!(
+        child_cursors[0].get_display_name(),
+        Ok("g_input".to_owned())
+    );
+    assert_eq!(
+        child_cursors[1].get_display_name(),
+        Ok("g_output".to_owned())
+    );
+    assert_eq!(
+        child_cursors[2].get_display_name(),
+        Ok("copyCs(uint3)".to_owned())
+    );
+
     for child_cursor in child_cursors {
         let range = child_cursor.get_extent().unwrap();
         println!("Child Range {:?}", range);