Skip to content

Commit dff5a82

Browse files
Revert ABI extension to protect UBSan error (#6420)
* Refs #24365: Avoid UBSan with statistics narrow operation Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com> * Revert "Refs #24365: Fix UB in statistics::DomainParticipant" This reverts commit ff298bdf63e574e20435485a1c21b4828ea839e0. Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com> --------- Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com>
1 parent 25a43a7 commit dff5a82

5 files changed

Lines changed: 29 additions & 25 deletions

File tree

include/fastdds/statistics/dds/domain/DomainParticipant.hpp

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,7 @@
2626
#include <fastdds/dds/builtin/topic/PublicationBuiltinTopicData.hpp>
2727
#include <fastdds/dds/builtin/topic/SubscriptionBuiltinTopicData.hpp>
2828
#include <fastdds/dds/core/ReturnCode.hpp>
29-
#include <fastdds/dds/core/status/StatusMask.hpp>
3029
#include <fastdds/dds/domain/DomainParticipant.hpp>
31-
#include <fastdds/dds/domain/DomainParticipantFactory.hpp>
3230
#include <fastdds/dds/publisher/qos/DataWriterQos.hpp>
3331
#include <fastdds/fastdds_dll.hpp>
3432

@@ -49,13 +47,6 @@ class DomainParticipant : public eprosima::fastdds::dds::DomainParticipant
4947
{
5048
DomainParticipant() = delete;
5149

52-
protected:
53-
54-
DomainParticipant(
55-
const eprosima::fastdds::dds::StatusMask& mask);
56-
57-
friend class eprosima::fastdds::dds::DomainParticipantFactory;
58-
5950
public:
6051

6152
/**

src/cpp/fastdds/domain/DomainParticipantFactory.cpp

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,6 @@
3939
#include <xmlparser/XMLEndpointParser.h>
4040
#include <xmlparser/XMLProfileManager.h>
4141

42-
#ifdef FASTDDS_STATISTICS
43-
#include <fastdds/statistics/dds/domain/DomainParticipant.hpp>
44-
#endif // ifdef FASTDDS_STATISTICS
45-
4642
using namespace eprosima::fastdds::xmlparser;
4743

4844
using eprosima::fastdds::rtps::RTPSDomain;
@@ -161,13 +157,12 @@ DomainParticipant* DomainParticipantFactory::create_participant(
161157

162158
const DomainParticipantQos& pqos = (&qos == &PARTICIPANT_QOS_DEFAULT) ? default_participant_qos_ : qos;
163159

164-
#ifndef FASTDDS_STATISTICS
165160
DomainParticipant* dom_part = new DomainParticipant(mask);
161+
#ifndef FASTDDS_STATISTICS
166162
DomainParticipantImpl* dom_part_impl = new DomainParticipantImpl(dom_part, did, pqos, listener);
167163
#else
168-
statistics::dds::DomainParticipant* dom_part = new statistics::dds::DomainParticipant(mask);
169-
statistics::dds::DomainParticipantImpl* dom_part_impl =
170-
new statistics::dds::DomainParticipantImpl(dom_part, did, pqos, listener);
164+
statistics::dds::DomainParticipantImpl* dom_part_impl = new statistics::dds::DomainParticipantImpl(dom_part, did,
165+
pqos, listener);
171166
#endif // FASTDDS_STATISTICS
172167

173168
if (fastdds::rtps::GUID_t::unknown() != dom_part_impl->guid())

src/cpp/statistics/fastdds/domain/DomainParticipant.cpp

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,6 @@ namespace fastdds {
3131
namespace statistics {
3232
namespace dds {
3333

34-
DomainParticipant::DomainParticipant(
35-
const eprosima::fastdds::dds::StatusMask& mask)
36-
: eprosima::fastdds::dds::DomainParticipant(mask)
37-
{
38-
}
39-
4034
fastdds::dds::ReturnCode_t DomainParticipant::enable_statistics_datawriter(
4135
const std::string& topic_name,
4236
const eprosima::fastdds::dds::DataWriterQos& dwqos)
@@ -82,7 +76,7 @@ DomainParticipant* DomainParticipant::narrow(
8276
eprosima::fastdds::dds::DomainParticipant* domain_participant)
8377
{
8478
#ifdef FASTDDS_STATISTICS
85-
return static_cast<DomainParticipant*>(domain_participant);
79+
return reinterpret_cast<DomainParticipant*>(domain_participant);
8680
#else
8781
(void)domain_participant;
8882
return nullptr;
@@ -93,7 +87,7 @@ const DomainParticipant* DomainParticipant::narrow(
9387
const eprosima::fastdds::dds::DomainParticipant* domain_participant)
9488
{
9589
#ifdef FASTDDS_STATISTICS
96-
return static_cast<const DomainParticipant*>(domain_participant);
90+
return reinterpret_cast<const DomainParticipant*>(domain_participant);
9791
#else
9892
(void)domain_participant;
9993
return nullptr;

test/blackbox/CMakeLists.txt

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,17 @@ set(DDS_BLACKBOXTESTS_SOURCE
143143
${CMAKE_CURRENT_SOURCE_DIR}/../../src/cpp/rtps/messages/CDRMessage.cpp
144144
)
145145

146+
# These statistics tests narrow a base DomainParticipant to statistics::dds::DomainParticipant
147+
# and call members on it. That is intentional UB (the object is never constructed as the derived
148+
# type, to avoid emitting its vtable and breaking ABI), so disable the vptr (bad-downcast) check
149+
# for just these translation units
150+
if(SANITIZER STREQUAL "UNDEFINED")
151+
set_source_files_properties(
152+
${CMAKE_CURRENT_SOURCE_DIR}/common/DDSBlackboxTestsStatistics.cpp
153+
${CMAKE_CURRENT_SOURCE_DIR}/common/DDSBlackboxTestsMonitorService.cpp
154+
PROPERTIES COMPILE_OPTIONS "-fno-sanitize=vptr")
155+
endif()
156+
146157
# Prepare static discovery xml file for blackbox tests.
147158
string(RANDOM LENGTH 4 ALPHABET 0123456789 TOPIC_RANDOM_NUMBER)
148159
math(EXPR TOPIC_RANDOM_NUMBER "${TOPIC_RANDOM_NUMBER} + 0") # Remove extra leading 0s.

test/unittest/statistics/dds/CMakeLists.txt

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,19 @@ if(TINYXML2_INCLUDE_DIR)
2222
include_directories(${TINYXML2_INCLUDE_DIR})
2323
endif(TINYXML2_INCLUDE_DIR)
2424

25+
# These tests narrow a base DomainParticipant to statistics::dds::DomainParticipant and call members
26+
# on it. That is intentional UB (the object is never constructed as the derived type, to avoid
27+
# emitting its vtable and breaking ABI), so disable the vptr (bad-downcast) check for just these
28+
# translation units
29+
if(SANITIZER STREQUAL "UNDEFINED")
30+
set_source_files_properties(
31+
StatisticsDomainParticipantTests.cpp
32+
StatisticsQosTests.cpp
33+
StatisticsDomainParticipantStatusQueryableTests.cpp
34+
StatisticsDomainParticipantMockTests.cpp
35+
PROPERTIES COMPILE_OPTIONS "-fno-sanitize=vptr")
36+
endif()
37+
2538
## StatisticsDomainParticipantTests
2639
set(STATISTICS_DOMAINPARTICIPANT_TESTS_SOURCE
2740
StatisticsDomainParticipantTests.cpp

0 commit comments

Comments
 (0)