Skip to content
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.

Sign up for GitHub

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

Fixing sanitizer handling. Fixes nasa/fprime#2678 #227

Merged
merged 1 commit into from
Nov 23, 2024
Merged

Fixing sanitizer handling. Fixes nasa/fprime#2678 #227

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/fprime/fbuild/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,11 @@ def run_fbuild_cli(
toolchain = build.find_toolchain()
print(f"[INFO] Generating build directory at: {build.build_dir}")
print(f"[INFO] Using toolchain file {toolchain} for platform {parsed.platform}")
if parsed.ut and not parsed.disable_sanitizers:
if parsed.disable_sanitizers:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is the only place in the system (currently) where sanitizers do get enabled. Without it, sanitizers will always be disabled unless the ENABLE_SANITIZERS_* flags are passed to CMake.

Is this the intended behavior? It feels to me that sanitizers on UTs are a good practice.

Not sure why nasa/fprime#2678 fails though...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I should have read nasa/fprime#2678 prior to posting this lol

Are you intending to make a PR to F´ to have SANITIZERS_* = BUILD_TESTING

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My intent was to set sanitizers in CMake in F´, yes.

# The following options are defined in F' to have CMake enable the sanitizers
cmake_args["ENABLE_SANITIZER_LEAK"] = "ON"
cmake_args["ENABLE_SANITIZER_ADDRESS"] = "ON"
cmake_args["ENABLE_SANITIZER_UNDEFINED_BEHAVIOR"] = "ON"
cmake_args["ENABLE_SANITIZER_LEAK"] = "OFF"
cmake_args["ENABLE_SANITIZER_ADDRESS"] = "OFF"
cmake_args["ENABLE_SANITIZER_UNDEFINED_BEHAVIOR"] = "OFF"
if toolchain is not None:
cmake_args["CMAKE_TOOLCHAIN_FILE"] = toolchain
if parsed.ninja:
Expand Down
Loading