fix remaining clang-tidy findings in the C sources (#353)#362
Open
ericsson-kuma wants to merge 1 commit into
Open
fix remaining clang-tidy findings in the C sources (#353)#362ericsson-kuma wants to merge 1 commit into
ericsson-kuma wants to merge 1 commit into
Conversation
Fix the two findings left in issue GrapheneOS#353: h_malloc.c:432 clang-analyzer-security.ArrayBound: false positive. slots is limited to MAX_SLAB_SLOT_COUNT (256) via size_class_slots, so (slots - 1) / U64_WIDTH <= 3 and i always stays within the 4 bitmap words; random_index is bounded by slots as well. The analyzer can't see either bound across translation units (slots is loaded from a u16 field and get_random_u16_uniform is defined in random.c), so suppress the report with a targeted NOLINT instead of disabling the check. util.h:51 bugprone-narrowing-conversions: make the conversion explicit with a cast. int64_t rather than long long since casting to the higher rank long long trips bugprone-misplaced-widening-cast. The same bits reach __builtin_ffsll as before and the built library is unchanged.
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
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.
Fixes the two findings that were left in #353.
h_malloc.c:432
clang-analyzer-security.ArrayBoundFalse positive.
slotsis limited to MAX_SLAB_SLOT_COUNT (256) viasize_class_slots, so(slots - 1) / U64_WIDTH <= 3andialways stays within the 4 bitmap words;random_indexis bounded byslotsas well. The analyzer can't see either bound across translation units (slotsis loaded from a u16 field andget_random_u16_uniformis defined in random.c), and I couldn't find a behavior-preserving restructure that expresses them locally, so this uses a targeted NOLINT with a comment rather than disabling the check for the whole project.util.h:51
bugprone-narrowing-conversionsMade the conversion explicit with a cast as suggested.
int64_trather thanlong longbecause casting to the higher-ranklong longtripsbugprone-misplaced-widening-cast(u64isunsigned longon LP64). The same bits reach__builtin_ffsllas before.Verification (clang-tidy 21.1.8):
make tidyfails with the h_malloc.c:432 ArrayBound error, andclang-tidy -header-filter='.*' h_malloc.c util.cadditionally reports the util.h:51 narrowing errormake tidypasses and the-header-filter='.*'run is cleanmake && make testpasses before and after; the rebuilt libhardened_malloc.so is byte-identical to the pre-change build (comments and an explicit cast only)Note: with the C sources passing,
make tidynow reaches its second clang-tidy invocation and surfaces 8 pre-existing findings in new.cc (misc-use-anonymous-namespace x4, misc-const-correctness x4 - the latter's pointer analysis is new in clang-tidy 21) that were previously hidden by the earlier failure. Those look like a style/config decision so I left them out of scope here.Refs #353