labath added a comment.
In D77287#1957472 <https://reviews.llvm.org/D77287#1957472>, @compnerd wrote:
> Thanks for the hint about the string conversion, however, I think that it's
> going to complicate things as the string is going to be a mixture of UTF-8
> and UTF-16 content.
That's true. Ok, plan B: have clang do the conversion for us. If you form the
expression like `printf("LoadLibraryW(L\"%s\")", path_in_utf8)` then the path
will get transformed to utf-16 by the compiler. However the path should also
get sanitized/escaped into a form suitable for passing to the C compiler.
(godbolt link <https://godbolt.org/z/75sHMK>)
> As to the testing, `functionalities/load_using_paths/TestLoadUsingPaths.py`
> is not applicable to Windows. In fact, I don't really see a good way to
> really test much of this outside the context of the swift REPL which forces
> the loading of a DLL which is in fact how I discovered this. If there is an
> easy way to ensure that the dll that is needed is in the user's `PATH`, then
> I suppose creating an empty dll and loading that is theoretically possible,
> but that too can have a lot of flakiness due to dependencies to build and all.
Ok, so your patch does not implement that functionality. It does not sound like
there's a fundamental limitation there, as you could do the same thing as what
the posix code
<https://github.com/llvm/llvm-project/blob/master/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp#L661>
does ( == iterate over the path), but that's fine -- you don't need to
implement everything straight away. In that case it would be good to check that
the `paths` argument is empty and return an error instead of attempting to load
a random library.
However, I believe you are implementing sufficient functionality for the
`SBProcess::LoadImage` (aka "process load" in CLI) command. So, maybe you could
take a look at `API/functionalities/load_unload/TestLoadUnload.py` and see if
anything there can be enabled ? Or (if the test contains too many posix
specifics), you could could create a windows version of that test doing
something similar.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77287/new/
https://reviews.llvm.org/D77287
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
