search

lifting-bits / remill

Library for lifting machine code to LLVM bitcode
Apache License 2.0
1.22k stars 142 forks source link

Adjust LLVM version to include minor version #630

Closed maxammann closed 1 year ago

maxammann commented 1 year ago

Remill expects the minor version to be included when looking for semantics: https://github.com/lifting-bits/remill/blob/eef338df00e0ee82e7648a28c7979c661a79526f/lib/BC/Util.cpp#L538

CLAassistant commented 1 year ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

pgoodman commented 1 year ago

@maxammann Can you update the binary name of remill lift to use the LLVM major version number, instead of the remill llvm number? This should resolve test failures.

image

Another reasonable question is: should semantics search look at MAJOR_MINOR or just focus on major version. I recall a while back we swapped from major.minor to major, right around when LLVM changed its versioning approach (LLVM 4 iirc). If the semantics are installed into remill//whatever or something like that, then it seems like the actual fix should be to change Util.cpp, and leave the cmake as is (though exporting whatever ends up being the right thing in remillConfig.cmake is a good idea).

maxammann commented 1 year ago

Another reasonable question is: should semantics search look at MAJOR_MINOR or just focus on major version.

I guess you are right and we should use only the major version. Which means that this should be fixed in remill. At least on ubuntu binaries are called clang-14 instead of clang-14.0.

maxammann commented 1 year ago

@pgoodman I would just close this and fix the issue in irene3 if that is fine for you. I think using only the major version is the correct way of handling this.