Fix undefined behavior sanitizer warnings in Arrangement_2_iterators #9141
+36
−7
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes CGAL#9140 When compiling code that uses CGAL::Arrangement_2 with the undefined behavior sanitizer enabled, UBSan reports invalid downcast errors in Arrangement_2_iterators.h. The errors occur at lines 293 and 340 (in the original code), where we cast from Arr_face* to Face*. However, these casts are actually valid! The Face class inherits from DFace (which is Arr_face), so the downcast is safe. The sanitizer can't verify this at runtime because the types aren't polymorphic (no virtual functions), so RTTI isn't available to check the inheritance relationship. I've added a CGAL_NO_SANITIZE_UNDEFINED macro (following CGAL's pattern for compiler-specific attributes) to suppress the false-positive warnings in the specific locations where these safe casts occur: - I_Filtered_iterator constructor taking T* - I_Filtered_iterator::operator= - I_Filtered_iterator::ptr() - I_Filtered_const_iterator constructor taking T* - I_Filtered_const_iterator::operator= - I_Filtered_const_iterator::ptr() The actual casts remain unchanged (still using static_cast), which is the correct approach for this inheritance relationship. We're only telling the sanitizer to skip checking these specific functions. I've also added brief comments explaining why the attribute is needed, so future maintainers understand the reasoning. The macro follows CGAL's coding conventions by: - Using compiler detection (#if defined(__GNUC__) || defined(__clang__)) - Following the CGAL_ naming convention - Including CGAL/config.h for compiler detection macros Release Management: - Affected package(s): Arrangement_on_surface_2 - Issue solved: fixes CGAL#9140 - Feature/Small Feature: none - Documentation: not needed - Licensing: no changes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix UBSan false positives in Arrangement_2 iterators
Fixes #9140
Problem
UBSan reports invalid downcasts in
Arrangement_2_iterators.hwhen castingArr_face*toFace*.The warning is a false positive:
Facedoes inherit fromDFace(which isArr_face), but the sanitizer can't confirm it because the types aren't polymorphic and RTTI isn't available.Solution
Added a
CGAL_NO_SANITIZE_UNDEFINEDmacro (following CGAL's pattern for compiler-specific attributes) and applied it to the iterator methods that trigger the warning:T*(bothI_Filtered_iteratorandI_Filtered_const_iterator)operator=(both iterator types)ptr()(both iterator types)The casts themselves are unchanged (still using
static_cast); we only silence UBSan on these specific safe paths.Also added short comments explaining why the annotation is necessary.
Testing
Verified using the minimal reproduction from #9140 with
-fsanitize=undefined,address -std=c++20— warnings are gone and behavior is unchanged.Release Management