Skip to content

[24364 & 24365] Add UBSan workflow and solve its errors#6386

Open
cferreiragonz wants to merge 24 commits into
masterfrom
add_ubsan
Open

[24364 & 24365] Add UBSan workflow and solve its errors#6386
cferreiragonz wants to merge 24 commits into
masterfrom
add_ubsan

Conversation

@cferreiragonz
Copy link
Copy Markdown
Contributor

@cferreiragonz cferreiragonz commented Apr 30, 2026

Description

This PR adds a new workflow to Fast DDS: UBSan.

  • It adds a new valid value for -DSANITIZER=Undefined, which sets -fsanitize=undefined -fno-omit-frame-pointer -fsanitize=float-cast-overflow -fsanitize=float-divide-by-zero -fsanitize=bounds-strict
  • It fixes build of Tests with the new UBSan flags and solves one ODR warning
  • It adds a new regression test which must fail as it guarantees an UB error.
  • It fixes every UB error of the whole Fast DDS Suite. Most of the errors where test-related, although some minor fixes also applied to the library itself.

Merge after

Contributor Checklist

  • Commit messages follow the project guidelines.
  • The code follows the style guidelines of this project.
  • Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added; the added tests pass locally
  • N/A: Any new/modified methods have been properly documented using Doxygen.
  • N/A: Any new configuration API has an equivalent XML API (with the corresponding XSD extension)
  • N/A: Changes are backport compatible: they do NOT break ABI nor change library core behavior.
  • N/A: Changes are API compatible.
  • N/A: New feature has been added to the versions.md file (if applicable).
  • N/A: New feature has been documented/Current behavior is correctly described in the documentation.
  • N/A: Applicable backports have been included in the description.

Reviewer Checklist

  • The PR has a milestone assigned.
  • The title and description correctly express the PR's purpose.
  • Check contributor checklist is correct.
  • If this is a critical bug fix, backports to the critical-only supported branches have been requested.
  • Check CI results: changes do not issue any warning.
  • Check CI results: failing tests are unrelated with the changes.

@cferreiragonz cferreiragonz added this to the v3.6.2 milestone Apr 30, 2026
@cferreiragonz cferreiragonz requested review from richiprosima and removed request for richiprosima April 30, 2026 13:43
@github-actions github-actions Bot added the ci-pending PR which CI is running label Apr 30, 2026
@cferreiragonz
Copy link
Copy Markdown
Contributor Author

cferreiragonz commented Apr 30, 2026

I am expecting the ubsan_fastdds_test step to fail due to failing tests. They will be solved in the following commits once it has been verified that the workflow properly fails when it should

@cferreiragonz cferreiragonz changed the title Add ubsan [24364 & 24365] Add UBSan workflow and solve its errors Apr 30, 2026
Comment thread .github/workflows/config/ubsan.meta Outdated
Comment on lines +4 to +17
- "-DNO_TLS=OFF"
- "-DSECURITY=ON"
- "-DFASTDDS_STATISTICS=ON"
- "-DSANITIZER=Undefined"
- "-DCMAKE_CXX_FLAGS_INIT='-Werror'"
- "-DEPROSIMA_BUILD_TESTS=ON"
- "-DRTPS_API_TESTS=ON"
- "-DFASTDDS_PIM_API_TESTS=ON"
- "-DPERFORMANCE_TESTS=ON"
googletest-distribution:
cmake-args:
- "-Dgtest_force_shared_crt=ON"
- "-DBUILD_SHARED_LIBS=ON"
- "-DBUILD_GMOCK=ON"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
- "-DNO_TLS=OFF"
- "-DSECURITY=ON"
- "-DFASTDDS_STATISTICS=ON"
- "-DSANITIZER=Undefined"
- "-DCMAKE_CXX_FLAGS_INIT='-Werror'"
- "-DEPROSIMA_BUILD_TESTS=ON"
- "-DRTPS_API_TESTS=ON"
- "-DFASTDDS_PIM_API_TESTS=ON"
- "-DPERFORMANCE_TESTS=ON"
googletest-distribution:
cmake-args:
- "-Dgtest_force_shared_crt=ON"
- "-DBUILD_SHARED_LIBS=ON"
- "-DBUILD_GMOCK=ON"
- -DNO_TLS=OFF
- -DSECURITY=ON
- -DFASTDDS_STATISTICS=ON
- -DSANITIZER=Undefined
- -DCOMPILE_WARNING_AS_ERROR=ON
- -DEPROSIMA_BUILD_TESTS=ON
- -DRTPS_API_TESTS=ON
- -DFASTDDS_PIM_API_TESTS=ON
- -DPERFORMANCE_TESTS=ON
googletest-distribution:
cmake-args:
- -Dgtest_force_shared_crt=ON
- -DBUILD_SHARED_LIBS=ON
- -DBUILD_GMOCK=ON

@cferreiragonz cferreiragonz requested a review from richiprosima May 4, 2026 08:22
@cferreiragonz cferreiragonz changed the base branch from master to bugfix/security-tests May 12, 2026 12:29
@cferreiragonz cferreiragonz requested review from richiprosima and removed request for richiprosima May 12, 2026 12:33
Base automatically changed from bugfix/security-tests to master May 14, 2026 07:28
Signed-off-by: Carlos Ferreira González <[email protected]>
Signed-off-by: Carlos Ferreira González <[email protected]>
Signed-off-by: Carlos Ferreira González <[email protected]>
Signed-off-by: Carlos Ferreira González <[email protected]>
Signed-off-by: Carlos Ferreira González <[email protected]>
Signed-off-by: Carlos Ferreira González <[email protected]>
Signed-off-by: Carlos Ferreira González <[email protected]>
Signed-off-by: Carlos Ferreira González <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-pending PR which CI is running needs rebase

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants