-
Notifications
You must be signed in to change notification settings - Fork 479
CI, Formatting, API & SVFG Builder Improvements #1702
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
Conversation
The current GitHub Workflow coverage test removes coverage information from certain files (i.e., with `lcov --remove coverage.info '/usr/*' --output-file coverage.info`). If the given directory (e.g., `/usr/*`) contains no files with coverage information, trying to remove this coverage information throws an error. The current workflow only adds `--ignore-errors unused` for the files with coverage information in `${{github.workspace}}/svf/include/FastCluster/*`, but not for the other directories. As this information should be removed anyway (also for the other directories), not having coverage information to begin with shouldn't be seen as an error. This commit applies the `--ignore-errors unused` flag consistently to the other `lcov --remove ...` invocations.
While there are formatting description files for C/C++ (i.e., `.clang-format`), no equivalent formatting specification exists for CMake (to automatically format CMake files (e.g., `CMakeLists.txt`) consistently with `cmake-format`). This commit adds such a format description to ensure all CMake files are formatted consistently.
Most `get...()` functions have `has...()` equivalents (e.g., `getDefSVFGNode()` and `hasDefSVFGNode()`). However, certain getters (e.g., `getLLVMValue()`, `getLHSTopLevelPtr()` had no such existence-checking counterparts. This commit adds a number of these checking functions in cases where they were missing. Note that these are for cases where the getters fail an assertion when no valid value exists. Cases where the getter simply returns a null-pointer on failure to find any corresponding values (e.g., `ICFG::getFunEntryBlock()`, `ICFG::getFunExitBlock()`, etc.), so a null-check can be used to catch cases where the getter fails to get any corresponding object, and thus don't need a corresponding existence checker. To avoid changing the current behaviour of the assertions and getters, I added additional existence-checking functions. This does mean the container will be searched twice (once in the existence-check and then again in the getter itself) and thus wastes performance. To avoid this, all current getters could be modified so that all getters return null-pointers on failures, but I didn't want to make such an intrusive change in this commit.
…istent with other WPA implementations)
Currently, most WPA implementations (e.g., `AndersenWaveDiff`) expose read-only functions (e.g., `printStat()`, `dumpTopLevelPtsTo()`, etc.) publically, as well as "constructors" like `solveAndWritePtsToFile()`, `readPtsFromFile()`, etc. However, `VersionedFlowSensitive` is the only one to mark these functions as `private`/`protected`. This makes it annoying to use polymorphism over the underlying PTA base class (i.e., `BVDataPTAImpl`) and the overridden virtual functions. This commit fixes that by marking these functions as public in the `VersionedFlowSensitive` WPA class.
This change allows me to use polymorphism to conveniently try different WPA backends like this:
```
SVF::BVDataPTAImpl *pta = nullptr;
switch(getChosenBackend()) {
case Andersen_BASE:
pta = new Andersen(...);
break;
case VFS_WPA:
pta = new VersionedFlowSensitive(...);
break;
default:
break;
}
```
…existing results The current `SVFGBuilder` optimises the SVFG based on the command-line argument. However, when using SVF as a library, this is inconvenient. This commit changes the builder to use an additional flag to control optimisation behaviour. Moreover, this commit fixes the reading/writing issues for optimised SVFGs. Currently, the writing happens post-optimisation, which causes the reading step to fail due to missing/unexpected nodes. This commit also ensures the *unoptimised SVFG* is written to a file *prior to optimising it*, and reads an unoptimised SVFG from a file and then optimises it. This fixes these crashing issues.
…ress errant warnings; added definitions of C++ aligned allocators (i.e., `new(align)`); defined all plain/no-throw/aligned/aligned-no-throw variants of C++ allocators for `size_t` defined as `unsigned int` and `unsigned long`; and defined `pvalloc()` and `strndup()` as allocators
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1702 +/- ##
==========================================
- Coverage 63.70% 63.68% -0.03%
==========================================
Files 244 244
Lines 25962 25975 +13
Branches 4575 4574 -1
==========================================
+ Hits 16539 16541 +2
- Misses 9423 9434 +11
🚀 New features to boost your workflow:
|
|
Thanks for splitting it. Please address my comments in this patch: def7c04 I shall merge. |
|
No problem, you can find the requested changes in 97bb59c. The reason I'd implemented the |
As per @yuleisui's request in this pull request (1697), this pull request contains the various fixes, improvements, and other changes unrelated to the build system. Note that this pull request contains one additional commit [c1306f4] not in the original pull request [1697], which adds some additional allocator definitions to
extapi.cand ensures existing ones are consistent.📝 Overview
This PR consolidates six orthogonal areas of work into one release:
cmake-formatconfig for consistent styling & formattinghas…()checks alongside existing gettersFor full details on each change, see the linked commits below.
🚀 Highlights
--ignore-errors unusedon alllcov --removecmake-formatspec for automatic stylinghas…()functions for previously unchecked gettersVersionedFlowSensitive⚙️ Detailed Changes
1️⃣ CI/Workflow Fix ([96c021b])
--ignore-errors unusedto alllcov --remove …calls/usr/*or other paths2️⃣ Automated CMake Formatting ([0bc1472])
.cmake-format.pysocmake-formatcan lint & format all CMake filesCMakeLists.txt,*.cmake, etc.3️⃣ Existence-Check APIs ([ce288cf])
hasLLVMValue(),hasLHSTopLevelPtr(), etc., for getters that assert on missing entries4️⃣ WPA Backend Polymorphism ([93446f6])
VersionedFlowSensitive’sprintStat(),dumpTopLevelPtsTo(),solveAndWritePtsToFile(),readPtsFromFile(), etc., as public5️⃣ Optimised SVFG Read/Write Fix ([def7c04])
6️⃣ Allocators ([c1306f4])
newwith alignment operators;newwith different internal definitions ofsize_t, etc.)pvalloc(),strndup(), etc.)restrictkeyword usage to suppress compiler/IDE warnings (use__restrictinstead (supported by C & C++))NULLconditional (already defined in included header) to suppress compiler/IDE warnings