Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
40 changes: 40 additions & 0 deletions quality/static_analysis/coding-standards.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,46 @@ deviations:
code-identifier: "shared-memory-align-const-cast"
scope: "Applies only to the specific const_cast marked with this code identifier in score/memory/shared/shared_memory_resource.cpp. This code-identifier must not be used anywhere else in the codebase."
justification: "`do_allocation_algorithm` uses `std::align`, whose `void*&` out-parameter cannot bind to the `const void*` input pointer. `std::align` only reads and arithmetically adjusts the pointer value to compute an aligned address within the given buffer; it never writes through the pointer to modify the pointee (https://timsong-cpp.github.io/cppwp/n4659/ptr.align#lib:align). The const qualification of `alloc_start` is therefore not violated at runtime, and the `const_cast` is required purely to satisfy `std::align`'s non-const parameter type."
- rule-id: "RULE-15-1-3"
query-id: "cpp/misra/non-explicit-conversion-member"
code-identifier: "offset-ptr-implicit-pointer-constructor"
scope: "Applies only to the single-argument `pointer`-taking constructor of `OffsetPtr` marked with this code identifier in score/memory/shared/offset_ptr.h. This code-identifier must not be used anywhere else in the codebase."
justification: "OffsetPtr is a drop-in replacement for a raw pointer in shared memory, and existing code across the codebase relies on being able to implicitly construct an OffsetPtr from a raw pointer (e.g. via assignment or as a default-initialized member/parameter). Requiring `explicit` here would force every one of these call sites to add redundant, purely mechanical `OffsetPtr{...}` wrapping without preventing any real defect, since a pointer-to-OffsetPtr conversion is exactly the intended, single well-defined semantics of this type."
- rule-id: "RULE-15-1-3"
query-id: "cpp/misra/non-explicit-conversion-member"
code-identifier: "offset-ptr-implicit-converting-constructor"
scope: "Applies only to the templated converting constructor of `OffsetPtr` taking `const OffsetPtr<OtherPointedType>&` marked with this code identifier in score/memory/shared/offset_ptr.h. This code-identifier must not be used anywhere else in the codebase."
justification: "This constructor mirrors the standard-library convention (e.g. `std::shared_ptr`'s converting constructor) allowing an `OffsetPtr<Derived>` to be implicitly used wherever an `OffsetPtr<Base>` is expected, exactly as a raw `Derived*` may be implicitly converted to `Base*`. Marking it `explicit` would break this pointer-like substitutability that OffsetPtr is designed to preserve, without eliminating any ambiguity risk beyond what already exists for ordinary pointer conversions."
- rule-id: "RULE-15-1-3"
query-id: "cpp/misra/non-explicit-conversion-member"
code-identifier: "offset-ptr-implicit-pointer-conversion"
scope: "Applies only to the `operator pointer() const` conversion operator of `OffsetPtr` marked with this code identifier in score/memory/shared/offset_ptr.h. This code-identifier must not be used anywhere else in the codebase."
justification: "OffsetPtr is used as the pointer type of allocator-aware standard containers (e.g. `basic_string`, `vector`) living in shared memory. These containers' implementations perform implicit conversions from the container's pointer type to the underlying raw pointer type in numerous internal code paths that cannot be modified. Making this conversion operator `explicit` would break compatibility with these standard-library container implementations, which is the entire purpose of this conversion operator."
- rule-id: "RULE-15-1-3"
query-id: "cpp/misra/non-explicit-conversion-member"
code-identifier: "polymorphic-offset-ptr-allocator-resource-constructor"
scope: "Applies only to the single-argument `ManagedMemoryResource&`-taking constructor of `PolymorphicOffsetPtrAllocator` marked with this code identifier in score/memory/shared/polymorphic_offset_ptr_allocator.h. This code-identifier must not be used anywhere else in the codebase."
justification: "This constructor exists specifically to allow a `ManagedMemoryResource&` to be implicitly used wherever the C++ Allocator named requirements expect an allocator object (e.g. when constructing an allocator-aware container in-place from a memory resource), mirroring the standard library's own `polymorphic_allocator` convention of implicit construction from a `memory_resource*`. Requiring `explicit` here would break this Allocator-requirements-driven usage pattern that is the entire purpose of the constructor."
- rule-id: "RULE-15-1-3"
query-id: "cpp/misra/non-explicit-conversion-member"
code-identifier: "polymorphic-offset-ptr-allocator-rebind-constructor"
scope: "Applies only to the templated `PolymorphicOffsetPtrAllocator<U>`-taking constructor of `PolymorphicOffsetPtrAllocator` marked with this code identifier in score/memory/shared/polymorphic_offset_ptr_allocator.h. This code-identifier must not be used anywhere else in the codebase."
justification: "This constructor implements the C++ Allocator named requirements' rebind-conversion contract, which requires that an allocator of one value type be implicitly constructible from an allocator of another value type (this is how standard containers rebind their allocator to internal node/element types). This is a copy-like construction, not an unrelated-type conversion, and marking it `explicit` would violate the Allocator named requirements and break rebind support in standard containers, which is the entire purpose of this constructor."
- rule-id: "RULE-15-1-3"
query-id: "cpp/misra/non-explicit-conversion-member"
code-identifier: "copyable-atomic-implicit-conversion"
scope: "Applies only to the `operator T() const noexcept` conversion operator of `CopyableAtomic` marked with this code identifier in score/mw/com/impl/util/copyable_atomic.h. This code-identifier must not be used anywhere else in the codebase."
justification: "CopyableAtomic is a thin wrapper intended to behave exactly like `std::atomic<T>` except for being copyable, and `std::atomic<T>` itself provides an implicit, non-explicit `operator T()` conversion. Marking this wrapper's conversion operator `explicit` would make it behave differently from the type it wraps and break existing code that relies on `std::atomic`-compatible implicit-conversion semantics, which is the entire purpose of this wrapper."
- rule-id: "RULE-15-1-3"
query-id: "cpp/misra/non-explicit-conversion-member"
code-identifier: "sample-ptr-implicit-nullptr-constructor"
scope: "Applies only to the `std::nullptr_t`-taking constructor of `SamplePtr` marked with this code identifier in score/mw/com/impl/plumbing/sample_ptr.h. This code-identifier must not be used anywhere else in the codebase."
justification: "This constructor exists specifically to allow `nullptr` to be used as a default argument value for `SamplePtr<T>` function parameters (e.g. `void TestFunc(SamplePtr<T> ptr = nullptr)`), matching the ergonomics of a raw pointer default argument. Requiring `explicit` would prevent this default-argument idiom and force every such call site to spell out `SamplePtr<T>{nullptr}`, without preventing any real defect, since `nullptr` unambiguously identifies the empty/null state of this type."
- rule-id: "RULE-15-1-3"
query-id: "cpp/misra/non-explicit-conversion-member"
code-identifier: "lambda-closure-function-pointer-conversion"
scope: "Applies only to the compiler-generated conversion operator of non-capturing lambda closure types, as they appear when such a lambda is assigned to, or returned as, a function pointer / function reference (e.g. `score::cpp::overload(...)` visitor patterns and factory functions that build and return callback function pointers from lambda literals) throughout the codebase."
justification: "Per the C++ standard ([expr.prim.lambda.closure]), a non-capturing lambda's closure type has an implicitly-declared, non-explicit conversion function to a pointer (or reference) to function with a matching signature. This conversion function is entirely compiler-generated: there is no source location associated with it where a user could write the `explicit` specifier, since the lambda expression itself declares no conversion operator at all. It is therefore structurally impossible to satisfy this rule for these findings without changing the language feature being used (i.e. rewriting a lambda literal as a free function or explicit functor class purely to silence the tool), which would be a purely cosmetic change with no functional or safety benefit."
guideline-recategorizations:
- rule-id: "RULE-0-1-1"
category: "disapplied"
Expand Down
1 change: 1 addition & 0 deletions score/memory/shared/new_delete_delegate_resource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ void* NewDeleteDelegateMemoryResource::do_allocate(const std::size_t bytes, std:
// result in a memory buffer large enough to align start_remaining_allocatable_memory in the worst case. If the
// pointer is worst-case aligned, then it would require (alignment - 1) bytes of padding.
auto max_required_padding_result =
// Deviation of MISRA RULE-15-1-3: codeql::misra_deviation_next_line(lambda-closure-function-pointer-conversion)
safe_math::Add(bytes, alignment).and_then([](const auto max_required_padding) noexcept {
return safe_math::Subtract(max_required_padding, 1U);
});
Expand Down
5 changes: 5 additions & 0 deletions score/memory/shared/offset_ptr.h
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ class OffsetPtr
// Rationale : Non-explicit constructor is needed for implicit conversion
// NOLINTBEGIN(google-explicit-constructor): needed implicit conversion
// coverity[autosar_cpp14_a12_1_4_violation]
// Deviation of MISRA RULE-15-1-3: codeql::misra_deviation_next_line(offset-ptr-implicit-pointer-constructor)
OffsetPtr(pointer ptr = nullptr) noexcept;
// NOLINTEND(google-explicit-constructor): see above

Expand All @@ -173,6 +174,7 @@ class OffsetPtr
// NOLINTBEGIN(google-explicit-constructor): needed implicit conversion
// coverity[autosar_cpp14_a12_1_4_violation]
template <typename OtherPointedType>
// Deviation of MISRA RULE-15-1-3: codeql::misra_deviation_next_line(offset-ptr-implicit-converting-constructor)
OffsetPtr(const OffsetPtr<OtherPointedType>& other);
// NOLINTEND(google-explicit-constructor): see above

Expand Down Expand Up @@ -243,6 +245,7 @@ class OffsetPtr
// Rationale: Using an offset pointer in a basic_string requires this conversion operator to be implicit.
// NOLINTBEGIN(google-explicit-constructor): requires conversion operator to be implicit
// coverity[autosar_cpp14_a13_5_2_violation]
// Deviation of MISRA RULE-15-1-3: codeql::misra_deviation_next_line(offset-ptr-implicit-pointer-conversion)
operator pointer() const
{
// NOLINTNEXTLINE(score-banned-function) See justification above class.
Expand Down Expand Up @@ -395,6 +398,7 @@ template <typename PointedType>
// identical between in-class declaration and definition.
// coverity[autosar_cpp14_m8_4_2_violation : FALSE]
// coverity[autosar_cpp14_m3_9_1_violation : FALSE]
// Deviation of MISRA RULE-15-1-3: codeql::misra_deviation_next_line(offset-ptr-implicit-pointer-constructor)
OffsetPtr<PointedType>::OffsetPtr(pointer ptr) noexcept
: offset_{CalculateOffsetFromPointer(this, ptr)}, memory_bounds_{}
{
Expand All @@ -416,6 +420,7 @@ OffsetPtr<PointedType>::OffsetPtr(const OffsetPtr<PointedType>& other) : offset_

template <typename PointedType>
template <typename OtherPointedType>
// Deviation of MISRA RULE-15-1-3: codeql::misra_deviation_next_line(offset-ptr-implicit-converting-constructor)
OffsetPtr<PointedType>::OffsetPtr(const OffsetPtr<OtherPointedType>& other) : offset_{}, memory_bounds_{}
{
std::tie(offset_, memory_bounds_) = CopyFrom<OtherPointedType>(other, *this);
Expand Down
2 changes: 2 additions & 0 deletions score/memory/shared/polymorphic_offset_ptr_allocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ class PolymorphicOffsetPtrAllocator

// Non-explicit constructor is good enough for maintaining required implicit conversion
// NOLINTNEXTLINE(google-explicit-constructor): Tolerated, discard explicit.
// Deviation of MISRA RULE-15-1-3: codeql::misra_deviation_next_line(polymorphic-offset-ptr-allocator-resource-constructor)
PolymorphicOffsetPtrAllocator(ManagedMemoryResource& resource) noexcept : proxy_{resource.getMemoryResourceProxy()}
{
}
Expand All @@ -52,6 +53,7 @@ class PolymorphicOffsetPtrAllocator
// Non-explicit constructor is good enough for maintaining required implicit conversion.
// In addition semantically is a copy constructor.
// NOLINTNEXTLINE(google-explicit-constructor): Tolerated, discard explicit.
// Deviation of MISRA RULE-15-1-3: codeql::misra_deviation_next_line(polymorphic-offset-ptr-allocator-rebind-constructor)
PolymorphicOffsetPtrAllocator(const PolymorphicOffsetPtrAllocator<U>& rhs) : proxy_(rhs.getMemoryResourceProxy())
{
}
Expand Down
1 change: 1 addition & 0 deletions score/message_passing/log/logging_callback.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ class FixedBufferStreamBuf final : public std::streambuf

LoggingCallback GetCerrLogger()
{
// Deviation of MISRA RULE-15-1-3: codeql::misra_deviation_next_line(lambda-closure-function-pointer-conversion)
return [](LogSeverity /*severity*/, LogItems items) -> void {
std::array<char, 1024U> buffer{};
FixedBufferStreamBuf stream_buffer{buffer.data(), buffer.size()};
Expand Down
1 change: 1 addition & 0 deletions score/message_passing/unix_domain/unix_domain_engine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ void UnixDomainEngine::RegisterPosixEndpoint(PosixEndpointEntry& endpoint) noexc
// TODO: not used/not supported yet
events |= POLLOUT;
}
// Deviation of MISRA RULE-15-1-3: codeql::misra_deviation_next_line(lambda-closure-function-pointer-conversion)
const auto found = std::find_if(poll_fds_.begin(), poll_fds_.end(), [](pollfd& poll) noexcept {
return poll.fd < 0;
});
Expand Down
4 changes: 2 additions & 2 deletions score/message_passing/unix_domain/unix_domain_engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ class UnixDomainEngine final : public ISharedResourceEngine
score::cpp::pmr::unique_ptr<score::os::Unistd> unistd{};
};

UnixDomainEngine(score::cpp::pmr::memory_resource* memory_resource,
LoggingCallback logger = GetCerrLogger()) noexcept;
explicit UnixDomainEngine(score::cpp::pmr::memory_resource* memory_resource,
LoggingCallback logger = GetCerrLogger()) noexcept;
~UnixDomainEngine() noexcept override;

UnixDomainEngine(const UnixDomainEngine&) = delete;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class ConsumerEventDataControlLocalView final
public:
using LocalEventControlSlots = score::cpp::span<ControlSlotType>;

ConsumerEventDataControlLocalView(EventDataControl& event_data_control_shared);
explicit ConsumerEventDataControlLocalView(EventDataControl& event_data_control_shared);

/// Test-only constructor which allows to directly set the TransactionLogLocalView. This avoids having to
/// inject the TransactionLogLocalView via the production code path which would require creating a
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ class ConsumerEventDataControlLocalViewFixture : public ::testing::Test
{
auto& transaction_log = transaction_log_.emplace(max_slots, memory_);
event_data_control_ = std::make_unique<EventDataControl>(max_slots, memory_);
unit_ = std::make_unique<ConsumerEventDataControlLocalView<>>(*event_data_control_, transaction_log);
unit_ = std::make_unique<ConsumerEventDataControlLocalView<>>(*event_data_control_, TransactionLogLocalView{transaction_log});
provider_event_data_control_local_ =
std::make_unique<ProviderEventDataControlLocalView<>>(*event_data_control_);
return *this;
Expand All @@ -111,9 +111,8 @@ class ConsumerEventDataControlLocalViewFixture : public ::testing::Test
atomic_indirector_mock_guard_ =
std::make_unique<AtomicIndirectorMockGuard<EventSlotStatus::value_type>>(*atomic_mock_);

unit_with_mock_atomics_ =
std::make_unique<ConsumerEventDataControlLocalView<concurrency::AtomicIndirectorMock>>(*event_data_control_,
transaction_log);
unit_with_mock_atomics_ = std::make_unique<ConsumerEventDataControlLocalView<concurrency::AtomicIndirectorMock>>(
*event_data_control_, TransactionLogLocalView{transaction_log});
provider_event_data_control_local_ =
std::make_unique<ProviderEventDataControlLocalView<>>(*event_data_control_);

Expand Down Expand Up @@ -419,7 +418,7 @@ TEST_P(MultiSenderMultiReceiverTest, MultiSenderMultiReceiver)
// In the real code, each ProxyEvent has its own ConsumerEventDataControlLocalView and TransactionLog. So we
// replicate that here by creating one of each per receiver thread.
TransactionLog transaction_log{GetParam().num_slots, memory_};
ConsumerEventDataControlLocalView<> consumer_event_data_control_local{event_data_control_, transaction_log};
ConsumerEventDataControlLocalView<> consumer_event_data_control_local{event_data_control_, TransactionLogLocalView{transaction_log}};
std::vector<SlotIndexType> used_slots{};
EventSlotStatus::EventTimeStamp start_ts{1};

Expand Down Expand Up @@ -499,7 +498,7 @@ TEST_P(MultiSenderMultiReceiverTest, DISABLED_MultiSenderMultiReceiverMaxReceive
// In the real code, each ProxyEvent has its own ConsumerEventDataControlLocalView and TransactionLog. So we
// replicate that here by creating one of each per receiver thread.
TransactionLog transaction_log{GetParam().num_slots, memory_};
ConsumerEventDataControlLocalView<> consumer_event_data_control_local{event_data_control_, transaction_log};
ConsumerEventDataControlLocalView<> consumer_event_data_control_local{event_data_control_, TransactionLogLocalView{transaction_log}};
std::vector<SlotIndexType> used_slots{};
EventSlotStatus::EventTimeStamp start_ts{0};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ class EventDataControlCompositeFixture : public ::testing::Test
auto& transaction_log_qm = transaction_log_qm_.emplace(kSlotCount, memory_);
auto& transaction_log_asil = transaction_log_asil_.emplace(kSlotCount, memory_);

proxy_qm_local_.emplace(*qm_, transaction_log_qm);
proxy_asil_local_.emplace(*asil_, transaction_log_asil);
proxy_qm_local_.emplace(*qm_, TransactionLogLocalView{transaction_log_qm});
proxy_asil_local_.emplace(*asil_, TransactionLogLocalView{transaction_log_asil});

unit_ =
std::make_unique<EventDataControlComposite<>>(skeleton_qm_local_.value(), &skeleton_asil_local_.value());
Expand Down Expand Up @@ -134,7 +134,7 @@ class EventDataControlCompositeFixture : public ::testing::Test
skeleton_qm_local_.emplace(*qm_);

auto& transaction_log_qm = transaction_log_qm_.emplace(kSlotCount, memory_);
proxy_qm_local_.emplace(*qm_, transaction_log_qm);
proxy_qm_local_.emplace(*qm_, TransactionLogLocalView{transaction_log_qm});

unit_ = std::make_unique<EventDataControlComposite<>>(skeleton_qm_local_.value(), nullptr);

Expand Down Expand Up @@ -702,8 +702,8 @@ TEST(EventDataControlCompositeTest, DISABLED_fuzz)
auto receiver = [&last_send_time_stamp, &qm, &asil, &memory]() {
TransactionLog transaction_log_qm{MAX_SLOTS, memory};
TransactionLog transaction_log_asil{MAX_SLOTS, memory};
ConsumerEventDataControlLocalView proxy_asil_local{asil, transaction_log_asil};
ConsumerEventDataControlLocalView proxy_qm_local{qm, transaction_log_qm};
ConsumerEventDataControlLocalView proxy_asil_local{asil, TransactionLogLocalView{transaction_log_asil}};
ConsumerEventDataControlLocalView proxy_qm_local{qm, TransactionLogLocalView{transaction_log_qm}};
std::set<SlotIndexType> used_slots_qm{};
std::set<SlotIndexType> used_slots_asil{};

Expand Down
Loading
Loading