Fix cpp/misra/pointer-argument-to-cstring-function-is-invalid (alert #881) - #846
Open
castler wants to merge 1 commit into
Open
Fix cpp/misra/pointer-argument-to-cstring-function-is-invalid (alert #881)#846castler wants to merge 1 commit into
castler wants to merge 1 commit into
Conversation
castler
requested review from
LittleHuba,
bemerybmw,
crimson11,
hoe-jo and
limdor
as code owners
August 5, 2026 10:56
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| // of T with the size argument of another instantiation, producing a false-positive size mismatch. Deriving the | ||
| // source range directly from the same pointer (source_begin / source_end) that is copied keeps the range | ||
| // trivially self-consistent for every instantiation of T. | ||
| const auto* const source_begin = reinterpret_cast<const std::uint8_t*>(&t); |
| // source range directly from the same pointer (source_begin / source_end) that is copied keeps the range | ||
| // trivially self-consistent for every instantiation of T. | ||
| const auto* const source_begin = reinterpret_cast<const std::uint8_t*>(&t); | ||
| const auto* const source_end = source_begin + sizeof(T); |
…#881) Replace the templated memcpy call in SerializeToMessage<T>() with an explicit std::copy over a byte range derived from the same pointer (source_begin/source_end). CodeQL's buffer-size analysis for the memcpy call was conflating the read-buffer size of one instantiation of T (e.g. ElementFqId/pid_t, 8 bytes) with the sizeof(T) size argument of other instantiations (12 and 24 byte payload structs), producing a false-positive size mismatch (alert #881). This is not a real memory-safety bug: for every instantiation, &t and sizeof(T) refer to the same object by construction. The rewrite keeps the source range self-consistent (derived from a single pointer) so static analysis can verify each instantiation independently instead of summarizing across instantiations. Verified with default and --config=tsan builds plus the existing message_passing_service_instance_test and message_passing_service_instance_methods_test suites (all passing). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
2 participants
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.
Summary
Fixes CodeQL alert #881 (
cpp/misra/pointer-argument-to-cstring-function-is-invalid, MISRA RULE-8-7-1) inscore/mw/com/impl/bindings/lola/messaging/message_passing_service_instance.cpp.Root cause (false positive, not a real bug)
SerializeToMessage<T>()is a template used with several different payload types:ElementFqId/pid_t— 8 bytesSubscribeServiceMethodUnserializedPayload/UnsubscribeServiceMethodUnserializedPayload— 12 bytesMethodCallUnserializedPayload— 24 bytesThese are exactly the sizes reported by the alert ("read buffer is 8 bytes, but the size argument is 12/24 bytes"). CodeQL's buffer-size analysis for the
memcpy(&out[1], &t, sizeof(T))call appears to conflate the read-buffer size of one template instantiation (the 8-byte case) with thesizeof(T)size argument of the other instantiations, producing a false mismatch.This is not an actual memory-safety issue: for every instantiation,
&tandsizeof(T)are derived from the exact same objecttof static typeT, so the read is always exactly in bounds. The destinationstd::array<std::uint8_t, sizeof(T) + 1>write (out[1..sizeof(T)]) is also in bounds.Fix
Replace the raw
memcpycall with an explicitstd::copyover a byte range computed from a single pointer (source_begin/source_end = source_begin + sizeof(T)), so the range is self-consistent per instantiation and does not require the analyzer to correlate a separate pointer expression with a separately-computed size argument. This follows the project's established approach of restructuring code so static analysis can prove safety locally, rather than suppressing the finding.Verification
bazel build //score/mw/com/impl/bindings/lola/messaging:message_passing_service_instance— passbazel build --config=tsan //score/mw/com/impl/bindings/lola/messaging:message_passing_service_instance— passbazel test //score/mw/com/impl/bindings/lola/messaging:message_passing_service_instance_test— passbazel test //score/mw/com/impl/bindings/lola/messaging:message_passing_service_instance_methods_test— passNo behavior change; this is a pure serialization-implementation refactor.