Skip to content

Commit a650514

Browse files
JavgilaviJavgilavi
authored
Fix null-dereference error on Ubuntu (#5895)
* Refs #21358: Fix null-deprecated error Signed-off-by: Javier Gil Aviles <[email protected]> * Refs #21358: Change null dereference compilation to test on alternatives build Signed-off-by: Javier Gil Aviles <[email protected]> * Refs #21358: Assert correctiosn fro tests Signed-off-by: Javier Gil Aviles <[email protected]> * Refs #Refs #21358: Correct getLocalParticipantProxyData assert missplace Signed-off-by: Javier Gil Aviles <[email protected]> * Refs #21358: Final corrections Signed-off-by: Javier Gil Aviles <[email protected]> * Refs #21358: Apply review corrections Signed-off-by: Javier Gil Aviles <[email protected]> * Refs 21358: Apply seconds review changes Signed-off-by: Javier Gil Aviles <[email protected]> * Refs #21358: Limit gnu::optimize to Ubuntu Signed-off-by: Javier Gil Aviles <[email protected]> * Refs #21358: Delate assert(false) and add log error Signed-off-by: Javier Gil Aviles <[email protected]> --------- Signed-off-by: Javier Gil Aviles <[email protected]>
1 parent d09c3c3 commit a650514

File tree

12 files changed

+264
-58
lines changed

12 files changed

+264
-58
lines changed

‎.github/workflows/reusable-ubuntu-ci.yml

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -856,6 +856,22 @@ jobs:
856856
cd ${{ github.workspace }}
857857
rm -rf build install log
858858
859+
- name: Null dereference colcon build
860+
uses: eProsima/eProsima-CI/multiplatform/colcon_build@v0
861+
with:
862+
colcon_meta_file: ${{ github.workspace }}/src/fastdds/.github/workflows/config/fastdds_build.meta
863+
colcon_build_args: '${{ inputs.colcon-args }} --packages-up-to fastdds'
864+
cmake_args: '-DEPROSIMA_BUILD_TESTS=OFF -DCMAKE_CXX_FLAGS="-Wnull-dereference" -DSECURITY=ON -DFASTDDS_STATISTICS=ON ${{ inputs.cmake-args }}'
865+
cmake_args_default: ${{ env.colcon-build-default-cmake-args }}
866+
# Force to build with maximum compiler optimization to catch all 'null-dereference' errors
867+
cmake_build_type: Release
868+
workspace: ${{ github.workspace }}
869+
870+
- name: Clean workspace - No shared libs
871+
run: |
872+
cd ${{ github.workspace }}
873+
rm -rf build install log
874+
859875
- name: Vanilla colcon build
860876
uses: eProsima/eProsima-CI/multiplatform/colcon_build@v0
861877
with:

‎src/cpp/fastdds/publisher/DataWriter.cpp

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,24 @@ DataWriter::DataWriter(
4040
DataWriterListener* listener,
4141
const StatusMask& mask)
4242
: DomainEntity(mask)
43-
, impl_(pub->create_datawriter(topic, qos, listener, mask)->impl_)
43+
, impl_(nullptr)
4444
{
45+
if (nullptr == pub)
46+
{
47+
EPROSIMA_LOG_ERROR(DATA_WRITER, "Publisher pointer is null");
48+
}
49+
else
50+
{
51+
DataWriter* dw = pub->create_datawriter(topic, qos, listener, mask);
52+
if (nullptr == dw)
53+
{
54+
EPROSIMA_LOG_ERROR(DATA_WRITER, "Publisher::create_datawriter returned null");
55+
}
56+
else
57+
{
58+
impl_ = dw->impl_;
59+
}
60+
}
4561
}
4662

4763
DataWriter::~DataWriter()

‎src/cpp/fastdds/xtypes/dynamic_types/AnnotationDescriptorImpl.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,11 @@ bool AnnotationDescriptorImpl::is_consistent() noexcept
137137
{
138138
return false;
139139
}
140+
141+
if (!ann_param_type)
142+
{
143+
return false;
144+
}
140145
//}}}
141146

142147
//{{{ Check the parameter value is convertible to its type.

‎src/cpp/fastdds/xtypes/dynamic_types/DynamicDataImpl.cpp

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1585,6 +1585,10 @@ size_t DynamicDataImpl::calculate_key_serialized_size(
15851585
for (auto& member : enclosing_type_->get_all_members())
15861586
{
15871587
auto member_impl {traits<DynamicTypeMember>::narrow<DynamicTypeMemberImpl>(member.second)};
1588+
if (!member_impl)
1589+
{
1590+
continue;
1591+
}
15881592
if (member_impl->get_descriptor().is_key())
15891593
{
15901594
there_is_keyed_member = true;
@@ -1610,6 +1614,10 @@ size_t DynamicDataImpl::calculate_key_serialized_size(
16101614
for (auto& member : enclosing_type_->get_all_members())
16111615
{
16121616
auto member_impl {traits<DynamicTypeMember>::narrow<DynamicTypeMemberImpl>(member.second)};
1617+
if (!member_impl)
1618+
{
1619+
continue;
1620+
}
16131621
auto member_type {traits<DynamicType>::narrow<DynamicTypeImpl>(member_impl->get_descriptor().type())};
16141622

16151623
//TODO(richiware) For the future support of optionals. Optional member cannot be a keyed member.
@@ -1978,6 +1986,10 @@ void DynamicDataImpl::serialize_key(
19781986
for (auto& member : enclosing_type_->get_all_members())
19791987
{
19801988
auto member_impl {traits<DynamicTypeMember>::narrow<DynamicTypeMemberImpl>(member.second)};
1989+
if (!member_impl)
1990+
{
1991+
continue;
1992+
}
19811993
if (member_impl->get_descriptor().is_key())
19821994
{
19831995
there_is_keyed_member = true;
@@ -2002,6 +2014,10 @@ void DynamicDataImpl::serialize_key(
20022014
for (auto& member : enclosing_type_->get_all_members())
20032015
{
20042016
auto member_impl {traits<DynamicTypeMember>::narrow<DynamicTypeMemberImpl>(member.second)};
2017+
if (!member_impl)
2018+
{
2019+
continue;
2020+
}
20052021
auto member_type {traits<DynamicType>::narrow<DynamicTypeImpl>(member_impl->get_descriptor().type())};
20062022

20072023
if (TK_MAP != member_type->resolve_alias_enclosed_type()->get_kind())
@@ -2050,6 +2066,11 @@ void DynamicDataImpl::apply_bitset_mask(
20502066
assert(enclosing_type_->get_all_members().end() != enclosing_type_->get_all_members().find(member_id));
20512067
const auto member_impl {traits<DynamicTypeMember>::narrow<DynamicTypeMemberImpl>(
20522068
enclosing_type_->get_all_members().at(member_id))};
2069+
if (!member_impl)
2070+
{
2071+
EPROSIMA_LOG_ERROR(DYN_TYPES, "Error applying bitset mask. MemberId not found.");
2072+
return;
2073+
}
20532074
const auto member_index {member_impl->get_descriptor().index()};
20542075
const auto bound {enclosing_type_->get_descriptor().bound().at(member_index)};
20552076
uint64_t mask {64 == bound ? 0x0llu : 0XFFFFFFFFFFFFFFFFllu << bound};
@@ -4008,6 +4029,13 @@ void DynamicDataImpl::set_discriminator_value(
40084029
auto m_impl = traits<DynamicTypeMember>::narrow<DynamicTypeMemberImpl>(member);
40094030
int32_t label {0};
40104031

4032+
if (!m_impl)
4033+
{
4034+
EPROSIMA_LOG_ERROR(DYN_TYPES, "MemberId " << id << " not found in enclosing type "
4035+
<< enclosing_type_->get_descriptor().name());
4036+
return;
4037+
}
4038+
40114039
if (m_impl->get_descriptor().is_default_label())
40124040
{
40134041
label = enclosing_type_->default_value();
@@ -4274,6 +4302,72 @@ ReturnCode_t DynamicDataImpl::set_primitive_value<TK_STRING16>(
42744302
return ret_value;
42754303
}
42764304

4305+
template<typename T, typename std::enable_if<std::is_integral<T>::value, bool>::type>
4306+
bool DynamicDataImpl::check_new_discriminator_value(
4307+
const T& value)
4308+
{
4309+
bool ret_value = false;
4310+
4311+
if (MEMBER_ID_INVALID != selected_union_member_) // There is a member selected by current discriminator.
4312+
{
4313+
traits<DynamicTypeMember>::ref_type selected_member;
4314+
enclosing_type_->get_member(selected_member, selected_union_member_);
4315+
auto sm_impl = traits<DynamicTypeMember>::narrow<DynamicTypeMemberImpl>(selected_member);
4316+
4317+
if (!sm_impl)
4318+
{
4319+
EPROSIMA_LOG_ERROR(DYNAMIC_DATA,
4320+
"Null DynamicTypeMemberImpl pointer");
4321+
return false;
4322+
}
4323+
4324+
for (auto label : sm_impl->get_descriptor().label())
4325+
{
4326+
if (static_cast<int32_t>(value) == label)
4327+
{
4328+
ret_value = true;
4329+
break;
4330+
}
4331+
}
4332+
}
4333+
4334+
if (MEMBER_ID_INVALID == selected_union_member_ ||
4335+
(MEMBER_ID_INVALID == enclosing_type_->default_union_member() && !ret_value)) // It is selected the implicit default member.
4336+
{
4337+
ret_value = true;
4338+
4339+
if (enclosing_type_->default_value() != static_cast<int32_t>(value))
4340+
{
4341+
for (auto member : enclosing_type_->get_all_members_by_index())
4342+
{
4343+
auto m_impl = traits<DynamicTypeMember>::narrow<DynamicTypeMemberImpl>(member);
4344+
4345+
if (!m_impl)
4346+
{
4347+
ret_value = false;
4348+
continue;
4349+
}
4350+
4351+
for (auto label : m_impl->get_descriptor().label())
4352+
{
4353+
if (static_cast<int32_t>(value) == label)
4354+
{
4355+
ret_value = false;
4356+
break;
4357+
}
4358+
}
4359+
}
4360+
}
4361+
4362+
if (ret_value)
4363+
{
4364+
selected_union_member_ = MEMBER_ID_INVALID;
4365+
}
4366+
}
4367+
4368+
return ret_value;
4369+
}
4370+
42774371
template<TypeKind TK>
42784372
ReturnCode_t DynamicDataImpl::set_sequence_values(
42794373
MemberId id,
@@ -6406,6 +6500,11 @@ bool DynamicDataImpl::deserialize(
64066500
{
64076501
auto member_impl = traits<DynamicTypeMember>::narrow<DynamicTypeMemberImpl>(member);
64086502
traits<DynamicDataImpl>::ref_type member_data;
6503+
if (!member_impl)
6504+
{
6505+
throw fastcdr::exception::BadParamException(
6506+
"Member not found in DynamicTypeImpl");
6507+
}
64096508
auto it = value_.find(member_impl->get_id());
64106509

64116510
if (it != value_.end())
@@ -6466,6 +6565,12 @@ bool DynamicDataImpl::deserialize(
64666565
auto m_impl {traits<DynamicTypeMember>::narrow<DynamicTypeMemberImpl>(
64676566
member)};
64686567

6568+
if (!m_impl)
6569+
{
6570+
ret_value = false;
6571+
continue;
6572+
}
6573+
64696574
for (auto label : m_impl->get_descriptor().label())
64706575
{
64716576
if (static_cast<int32_t>(discriminator) == label)

‎src/cpp/fastdds/xtypes/dynamic_types/DynamicDataImpl.hpp

Lines changed: 1 addition & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -481,56 +481,7 @@ class DynamicDataImpl : public traits<DynamicData>::base_type
481481
*/
482482
template<typename T, typename std::enable_if<std::is_integral<T>::value, bool>::type = true>
483483
bool check_new_discriminator_value(
484-
const T& value)
485-
{
486-
bool ret_value = false;
487-
488-
if (MEMBER_ID_INVALID != selected_union_member_) // There is a member selected by current discriminator.
489-
{
490-
traits<DynamicTypeMember>::ref_type selected_member;
491-
enclosing_type_->get_member(selected_member, selected_union_member_);
492-
auto sm_impl = traits<DynamicTypeMember>::narrow<DynamicTypeMemberImpl>(selected_member);
493-
494-
for (auto label : sm_impl->get_descriptor().label())
495-
{
496-
if (static_cast<int32_t>(value) == label)
497-
{
498-
ret_value = true;
499-
break;
500-
}
501-
}
502-
}
503-
504-
if (MEMBER_ID_INVALID == selected_union_member_ ||
505-
(MEMBER_ID_INVALID == enclosing_type_->default_union_member() && !ret_value)) // It is selected the implicit default member.
506-
{
507-
ret_value = true;
508-
509-
if (enclosing_type_->default_value() != static_cast<int32_t>(value))
510-
{
511-
for (auto member : enclosing_type_->get_all_members_by_index())
512-
{
513-
auto m_impl = traits<DynamicTypeMember>::narrow<DynamicTypeMemberImpl>(member);
514-
515-
for (auto label : m_impl->get_descriptor().label())
516-
{
517-
if (static_cast<int32_t>(value) == label)
518-
{
519-
ret_value = false;
520-
break;
521-
}
522-
}
523-
}
524-
}
525-
526-
if (ret_value)
527-
{
528-
selected_union_member_ = MEMBER_ID_INVALID;
529-
}
530-
}
531-
532-
return ret_value;
533-
}
484+
const T& value);
534485

535486
template<typename T, typename std::enable_if<!std::is_integral<T>::value, bool>::type = true>
536487
bool check_new_discriminator_value(

‎src/cpp/fastdds/xtypes/dynamic_types/DynamicPubSubType.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,11 @@ bool DynamicPubSubType::serialize(
232232
payload.encapsulation = ser.endianness() == eprosima::fastcdr::Cdr::BIG_ENDIANNESS ? CDR_BE : CDR_LE;
233233

234234
auto type_impl = traits<DynamicType>::narrow<DynamicTypeImpl>(dynamic_type_);
235+
if (!type_impl)
236+
{
237+
EPROSIMA_LOG_ERROR(DYN_TYPES, "DynamicPubSubType cannot serialize data. Unspecified type.");
238+
return false;
239+
}
235240
ser.set_encoding_flag(get_fastcdr_encoding_flag(type_impl->get_descriptor().extensibility_kind(),
236241
fastdds::dds::DataRepresentationId_t::XCDR_DATA_REPRESENTATION == data_representation?
237242
eprosima::fastcdr::CdrVersion:: XCDRv1 :
@@ -317,6 +322,10 @@ void DynamicPubSubType::update_dynamic_type()
317322
for (auto& member : type_impl->get_all_members_by_index())
318323
{
319324
auto member_impl = traits<DynamicTypeMember>::narrow<DynamicTypeMemberImpl>(member);
325+
if (!member_impl)
326+
{
327+
continue;
328+
}
320329
if (member_impl->get_descriptor().is_key())
321330
{
322331
is_compute_key_provided = true;

‎src/cpp/fastdds/xtypes/dynamic_types/DynamicTypeBuilderFactoryImpl.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,12 @@ traits<DynamicTypeBuilder>::ref_type DynamicTypeBuilderFactoryImpl::create_type(
5656
{
5757
auto descriptor_impl = traits<TypeDescriptor>::narrow<TypeDescriptorImpl>(descriptor);
5858

59+
if (!descriptor_impl)
60+
{
61+
EPROSIMA_LOG_ERROR(DYN_TYPES, "Invalid TypeDescriptor provided to create_type.");
62+
return {};
63+
}
64+
5965
if (descriptor_impl->is_consistent())
6066
{
6167
return std::make_shared<DynamicTypeBuilderImpl>(*descriptor_impl);

‎src/cpp/fastdds/xtypes/dynamic_types/DynamicTypeBuilderImpl.cpp

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,16 @@ DynamicTypeBuilderImpl::DynamicTypeBuilderImpl(
5959
{
6060
traits<DynamicTypeMemberImpl>::ref_type member_impl {traits<DynamicTypeMember>::narrow<DynamicTypeMemberImpl>(
6161
members_.back())};
62-
assert(MEMBER_ID_INVALID != member_impl->get_descriptor().id());
63-
next_id_ = member_impl->get_descriptor().id() + 1;
62+
if (!member_impl)
63+
{
64+
EPROSIMA_LOG_ERROR(DYN_TYPES,
65+
"Internal error: last member is not a DynamicTypeMemberImpl.");
66+
}
67+
else
68+
{
69+
assert(MEMBER_ID_INVALID != member_impl->get_descriptor().id());
70+
next_id_ = member_impl->get_descriptor().id() + 1;
71+
}
6472
}
6573

6674
next_index_ = static_cast<uint32_t>(members_.size());
@@ -453,6 +461,11 @@ ReturnCode_t DynamicTypeBuilderImpl::add_member(
453461
{
454462
const MemberId mid {member.first};
455463
const auto member_impl {traits<DynamicTypeMember>::narrow<DynamicTypeMemberImpl>(member.second)};
464+
if (!member_impl)
465+
{
466+
EPROSIMA_LOG_ERROR(DYN_TYPES, "Member with id " << mid << " is not a DynamicTypeMemberImpl");
467+
return RETCODE_BAD_PARAMETER;
468+
}
456469
const auto member_index {member_impl->get_descriptor().index()};
457470

458471
if (mid == new_member_id)
@@ -499,6 +512,12 @@ ReturnCode_t DynamicTypeBuilderImpl::add_member(
499512
{
500513
const auto member_impl = traits<DynamicTypeMember>::narrow<DynamicTypeMemberImpl>(members_.at(0));
501514

515+
if (!member_impl)
516+
{
517+
EPROSIMA_LOG_ERROR(DYN_TYPES, "Member is not a DynamicTypeMemberImpl");
518+
return RETCODE_BAD_PARAMETER;
519+
}
520+
502521
if (member_impl->get_descriptor().type()->get_kind() != descriptor->type()->get_kind())
503522
{
504523
EPROSIMA_LOG_ERROR(DYN_TYPES, "Descriptor type kind differs from the current member types.");
@@ -554,6 +573,11 @@ ReturnCode_t DynamicTypeBuilderImpl::add_member(
554573
for (++it; it != members_.end(); ++it)
555574
{
556575
auto next_member = traits<DynamicTypeMember>::narrow<DynamicTypeMemberImpl>(*it);
576+
if (!next_member)
577+
{
578+
EPROSIMA_LOG_ERROR(DYN_TYPES, "Member is not a DynamicTypeMemberImpl");
579+
return RETCODE_BAD_PARAMETER;
580+
}
557581
next_member->get_descriptor().index(next_member->get_descriptor().index() + 1);
558582
}
559583
++next_index_;

‎src/cpp/rtps/builtin/discovery/endpoint/EDPSimple.cpp

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,8 +157,19 @@ void EDPSimple::processPersistentData(
157157
std::lock_guard<RecursiveTimedMutex> guardW(writer.first->getMutex());
158158
std::lock_guard<std::recursive_mutex> guardP(*mp_PDP->getMutex());
159159

160+
if (nullptr == mp_PDP)
161+
{
162+
EPROSIMA_LOG_ERROR(RTPS_EDP, "Cannot processPersistentData: mp_PDP is null");
163+
return;
164+
}
165+
ParticipantProxyData* local_ppd = mp_PDP->getLocalParticipantProxyData();
166+
if (nullptr == local_ppd)
167+
{
168+
EPROSIMA_LOG_ERROR(RTPS_EDP, "Cannot processPersistentData: LocalParticipantProxyData is null");
169+
return;
170+
}
160171
// own server instance
161-
InstanceHandle_t server_key = mp_PDP->getLocalParticipantProxyData()->m_key;
172+
InstanceHandle_t server_key = local_ppd->m_key;
162173

163174
// reference own references from writer history
164175
std::forward_list<CacheChange_t*> removal;

0 commit comments

Comments
 (0)