Support linking with LLVM shared libraries #1593
Conversation
This must be manually enabled by setting the variable `llvm-shared` to a true value in the current opam switch, similar to the existing `llvm-config` variable used by conf-bap-llvm. * bap_llvm/config/llvm_configurator.ml: use `llvm-config --shared-mode` to get the appropriate linking mode * bap_llvm/llvm_disasm.cpp: add missing include for Optional.h
There was a problem hiding this comment.
Some questions in the code. And the main question: it is not clear to me that the static mode is the default (which I would like to have). Is that right? We need statically linked and self-contained bap. Of course, having an ability to dynamically link with llvm would be nice (especially since some distributions do not distribute statically linked llvm) as we do have with the OASIS and OMake build systems.
| let linkmode = | ||
| "--link-" ^ | ||
| (String.strip @@ | ||
| C.Process.run_capture_exn self llvm_config ["--shared-mode"]) in |
There was a problem hiding this comment.
I wonder if the --shared-mode command-line option is present on all llvm versions that we support? Maybe, it is better to be more defensive here (both against previous and future versions) and anticipate possible failures by sticking with the static mode by default.
There was a problem hiding this comment.
It exists at least as far back as llvm 4.0. I don't know if there's a great solution here in general, since the installed version of llvm could change since bap was built, and then we might have a mismatch of the linking mode anyway.
| @@ -3,6 +3,7 @@ build: [ | |||
| [ | |||
| "ocaml" "tools/configure.ml" | |||
| "--with-llvm-config=%{conf-bap-llvm:config}%" | |||
| "--%{llvm-shared?disable:enable}%-llvm-static" | |||
There was a problem hiding this comment.
wow, didn't know about this syntax, is it new? (I mean will it work with opam 2.0)
There was a problem hiding this comment.
Yes, this was added in opam 1.2.1.
|
Is anything preventing this from being merged? |
No, not really. There was an issue with the build action, but it looks like it wasn't the problem of this PR, I will look at it later! Thanks for the contribution! |
This must be manually enabled by setting the variable
llvm-sharedto a true value in the current opam switch, similar to the existingllvm-configvariable used by conf-bap-llvm.bap_llvm/config/llvm_configurator.ml: use
llvm-config --shared-modeto get the appropriate linking modebap_llvm/llvm_disasm.cpp: add missing include for Optional.h