Skip to content

Conversation

@Johanmyst
Copy link
Contributor

As requested in pull request #1703, this pull request updates the example to use the updated build system properly.

Note: the GitHub workflow currently fails, but this is due to the SVF NPM package not actually installing the CMake package at all. Instead, the NPM package runs build.sh which only builds SVF:

cmake -D CMAKE_BUILD_TYPE:STRING="${BUILD_TYPE}"   \
    -DSVF_ENABLE_ASSERTIONS:BOOL=true              \
    -DSVF_SANITIZE="${SVF_SANITIZER}"              \
    -DBUILD_SHARED_LIBS=${BUILD_DYN_LIB}            \
    -S "${SVFHOME}" -B "${BUILD_DIR}"
cmake --build "${BUILD_DIR}" -j ${jobs}

As discussed in pull request #1703 however, using the CMake package inside of the build tree could lead to invalid paths and unexpected behaviour and should not be supported, so with that pull request, this example's workflow still fails.

This could be fixed easily by updating build.sh to actually install the SVF package (by adding cmake --install [Release|Debug]-build and adding --install-prefix=<prefix> to the configuration step (or --prefix=<prefix> to the cmake --install ... command)). The default installation prefix (/usr/local) requires root permissions however, so perhaps an alternative prefix should be used by default, though unless the prefix $SVF_DIR is used, this could require some modifications to ensure the environment uses the non-standard installation prefix.

Ideally, I think it'd be much better for maintainability and clarity to move the part of build.sh that pulls, builds, and installs LLVM and Z3 to a separate script (e.g., get_llvm.sh and get_z3.sh). Then, the build.sh and setup.sh scripts could be removed entirely, and the build & installation instructions (on the Wiki) could simply show the standard installation instructions (similar to the README of this example).

@yuleisui yuleisui merged commit ceaf768 into SVF-tools:master May 23, 2025
0 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants