-
-
Notifications
You must be signed in to change notification settings - Fork 324
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 file extension for clang in libafl_cc/build.rs #1237
Conversation
Add CLANG and CLANG_PP env variables for Windows. Resolves issue if clang and llvm-config are not in the same location.
libafl_cc/build.rs
Outdated
@@ -234,8 +234,14 @@ pub const LIBAFL_CC_LLVM_VERSION: Option<usize> = None; | |||
let llvm_bindir = exec_llvm_config(&["--bindir"]); | |||
let bindir_path = Path::new(&llvm_bindir); | |||
|
|||
let clang = bindir_path.join("clang"); | |||
let clangcpp = bindir_path.join("clang++"); | |||
let clang_path = env::var_os("CLANG").unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these always set? If not, we should not unwrap
but fall back to the original names, instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, it should be the same as with LLVM_CONFIG
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's a variable set by user?
i think it should have more general name like LIBAFL_CLANG_PATH, LIBAFL_CLANGPP_PATH
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I have discovered another problem. The bindir_path
variable is also used to build the passes, so even if we change the env variables, bindir_path
wouldn't change. It didn't want to build previously because it didn't want to find "clang" because of the missing ".exe" extension. Therefore, I have added a config check (cf. 14bca18) and also added the exe extension in the build_pass
function for the windows config.
Now, however, the user has to have llvm-config
, clang.exe
and clang++.exe
in the same directory (which should be default anyways... So the immediate solution, is to copy the clang files from the bin directory to wherever llvm-config
is. I hope this doesn't break anything, but for now it builds without a problem).
User should have llvm-config and clang.exe clang++.exe in the same directory anyways.
All that's missing now is a |
Also, the title of the PR is now outdated |
thanks ! |
#1234
Add CLANG and CLANG_PP env variables for Windows. Resolves issue if clang and llvm-config are not in the same location.