From 20cd91dd0533a52e365198903bf88ab0bb5ea1c9 Mon Sep 17 00:00:00 2001 From: Patrick Urbanke Date: Wed, 30 Oct 2024 22:06:37 +0100 Subject: [PATCH 1/8] Additional test based on #240 --- tests/json/test_segfault.cpp | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 tests/json/test_segfault.cpp diff --git a/tests/json/test_segfault.cpp b/tests/json/test_segfault.cpp new file mode 100644 index 00000000..cd2ce656 --- /dev/null +++ b/tests/json/test_segfault.cpp @@ -0,0 +1,29 @@ +#include +#include +#include +#include +#include + +#include "write_and_read.hpp" + +// This example had led to a segfault on GCC 12. +// To make sure this won't happen again, we include +// it in our tests. +namespace test_segfault { + +struct Data { + std::string prop1; + std::string prop2; + std::string prop3; + std::string prop4; + std::string prop5; + std::string prop6; +}; + +TEST(json, test_segfault) { + const auto data = Data{}; + write_and_read( + data, + R"({"prop1":"","prop2":"","prop3":"","prop4":"","prop5":"","prop6":""})"); +} +} // namespace test_segfault From 6460121e5065e391f9ca2c2909142419d526c9c8 Mon Sep 17 00:00:00 2001 From: Patrick Urbanke Date: Wed, 30 Oct 2024 22:06:51 +0100 Subject: [PATCH 2/8] Increased optimization level to O3; see #240 --- tests/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index edcc9808..531dfc63 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -3,7 +3,7 @@ set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -O2") if (MSVC) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std:c++20") else() - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++20 -Wall -Wno-sign-compare -Wno-missing-braces -Wno-psabi -pthread -fno-strict-aliasing -fwrapv -O2 -ftemplate-backtrace-limit=0 -fsanitize=undefined") + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++20 -Wall -Wno-sign-compare -Wno-missing-braces -Wno-psabi -pthread -fno-strict-aliasing -fwrapv -O3 -ftemplate-backtrace-limit=0 -fsanitize=undefined") endif() if (REFLECTCPP_JSON) From 33fcddd557dabf1942937f42d6f9d5ba6fce4840 Mon Sep 17 00:00:00 2001 From: Patrick Urbanke Date: Wed, 30 Oct 2024 22:16:36 +0100 Subject: [PATCH 3/8] Remove -fsanitize=undefined --- tests/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 531dfc63..0e362249 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -3,7 +3,7 @@ set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -O2") if (MSVC) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std:c++20") else() - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++20 -Wall -Wno-sign-compare -Wno-missing-braces -Wno-psabi -pthread -fno-strict-aliasing -fwrapv -O3 -ftemplate-backtrace-limit=0 -fsanitize=undefined") + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++20 -Wall -Wno-sign-compare -Wno-missing-braces -Wno-psabi -pthread -fno-strict-aliasing -fwrapv -O3 -ftemplate-backtrace-limit=0") endif() if (REFLECTCPP_JSON) From de416b0bcad39a0c7e38eba1a6b1bdad3b97c529 Mon Sep 17 00:00:00 2001 From: Patrick Urbanke Date: Wed, 30 Oct 2024 22:35:51 +0100 Subject: [PATCH 4/8] Use std::forward instead of std::move --- include/rfl/internal/tuple/concat.hpp | 5 +++-- include/rfl/tuple_cat.hpp | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/include/rfl/internal/tuple/concat.hpp b/include/rfl/internal/tuple/concat.hpp index 7d441ad3..3020a000 100644 --- a/include/rfl/internal/tuple/concat.hpp +++ b/include/rfl/internal/tuple/concat.hpp @@ -19,7 +19,7 @@ auto wrap_tuple(const rfl::Tuple& _tuple) { template auto wrap_tuple(rfl::Tuple&& _tuple) { - return TupleWrapper{std::move(_tuple)}; + return TupleWrapper{std::forward >(_tuple)}; } template @@ -46,7 +46,8 @@ auto concat(const Head& _head, const Tail&... _tail) { template auto concat(Head&& _head, Tail&&... _tail) { - return (wrap_tuple(std::move(_head)) + ... + wrap_tuple(std::move(_tail))) + return (wrap_tuple(std::forward(_head)) + ... + + wrap_tuple(std::forward(_tail))) .tuple_; } diff --git a/include/rfl/tuple_cat.hpp b/include/rfl/tuple_cat.hpp index 8c0ab2cd..da558f54 100644 --- a/include/rfl/tuple_cat.hpp +++ b/include/rfl/tuple_cat.hpp @@ -13,7 +13,8 @@ auto tuple_cat(const Head& _head, const Tail&... _tail) { template auto tuple_cat(Head&& _head, Tail&&... _tail) { - return internal::tuple::concat(std::move(_head), std::move(_tail)...); + return internal::tuple::concat(std::forward(_head), + std::forward(_tail)...); } inline auto tuple_cat() { return rfl::Tuple(); } From 19fe734c1db0260b33362f4496229aa8dfb12aa1 Mon Sep 17 00:00:00 2001 From: Patrick Urbanke Date: Thu, 31 Oct 2024 12:40:50 +0100 Subject: [PATCH 5/8] Use the exact same compiler settings proposed in #240 --- tests/CMakeLists.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 0e362249..ef908c0c 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -3,7 +3,8 @@ set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -O2") if (MSVC) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std:c++20") else() - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++20 -Wall -Wno-sign-compare -Wno-missing-braces -Wno-psabi -pthread -fno-strict-aliasing -fwrapv -O3 -ftemplate-backtrace-limit=0") + #set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++20 -Wall -Wno-sign-compare -Wno-missing-braces -Wno-psabi -pthread -fno-strict-aliasing -fwrapv -O3 -ftemplate-backtrace-limit=0") + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -O3 -Wall -Wextra -ggdb -ftemplate-backtrace-limit=0") endif() if (REFLECTCPP_JSON) From 11bf303ead88d9818a290edbc9bafd8da9b5f724 Mon Sep 17 00:00:00 2001 From: Patrick Urbanke Date: Thu, 31 Oct 2024 12:41:02 +0100 Subject: [PATCH 6/8] Fixed some warnings --- include/rfl/Timestamp.hpp | 2 ++ include/rfl/generic/Writer.hpp | 4 ++-- include/rfl/internal/tuple/calculate_positions.hpp | 4 ++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/include/rfl/Timestamp.hpp b/include/rfl/Timestamp.hpp index fd9dcfa6..16f53f44 100644 --- a/include/rfl/Timestamp.hpp +++ b/include/rfl/Timestamp.hpp @@ -25,6 +25,8 @@ class Timestamp { using ReflectionType = std::string; + Timestamp() : tm_(std::tm{}) {} + Timestamp(const char* _str) : tm_(std::tm{}) { const auto r = strptime(_str, _format.str().c_str(), &tm_); if (r == NULL) { diff --git a/include/rfl/generic/Writer.hpp b/include/rfl/generic/Writer.hpp index 98ffabd7..b4e72027 100644 --- a/include/rfl/generic/Writer.hpp +++ b/include/rfl/generic/Writer.hpp @@ -85,9 +85,9 @@ struct Writer { OutputVarType add_null_to_object(const std::string_view& _name, OutputObjectType* _parent) const noexcept; - void end_array(OutputArrayType* _arr) const noexcept {} + void end_array(OutputArrayType*) const noexcept {} - void end_object(OutputObjectType* _obj) const noexcept {} + void end_object(OutputObjectType*) const noexcept {} OutputVarType& root() { return root_; } diff --git a/include/rfl/internal/tuple/calculate_positions.hpp b/include/rfl/internal/tuple/calculate_positions.hpp index 1b884336..20e375b9 100644 --- a/include/rfl/internal/tuple/calculate_positions.hpp +++ b/include/rfl/internal/tuple/calculate_positions.hpp @@ -17,8 +17,8 @@ struct Positions { }; template -consteval auto operator+(const Positions<_last, _is...>& _sizes, - const PositionWrapper& _w) { +consteval auto operator+(const Positions<_last, _is...>&, + const PositionWrapper&) { if constexpr (_last % alignof(T) == 0) { constexpr auto last_new = _last + sizeof(T); return Positions{}; From fdb1d2ed56ba7f61fcc02476b52051c62b9dbd06 Mon Sep 17 00:00:00 2001 From: Patrick Urbanke Date: Thu, 31 Oct 2024 12:41:13 +0100 Subject: [PATCH 7/8] Use std::bit_cast instead of reinterpret_cast --- include/rfl/Tuple.hpp | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/include/rfl/Tuple.hpp b/include/rfl/Tuple.hpp index 43ab91d4..1e08b261 100644 --- a/include/rfl/Tuple.hpp +++ b/include/rfl/Tuple.hpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -55,17 +56,16 @@ class Tuple { /// Gets an element by index. template - auto& get() { + constexpr auto& get() { using Type = internal::nth_element_t<_index, Types...>; - return *std::launder(reinterpret_cast(data_.data() + pos<_index>())); + return *std::bit_cast(data_.data() + pos<_index>()); } /// Gets an element by index. template - const auto& get() const { + constexpr const auto& get() const { using Type = internal::nth_element_t<_index, Types...>; - return *std::launder( - reinterpret_cast(data_.data() + pos<_index>())); + return *std::bit_cast(data_.data() + pos<_index>()); } /// Assigns the underlying object. @@ -132,7 +132,8 @@ class Tuple { const auto copy_one = [this](const auto& _other, std::integral_constant) { using Type = internal::nth_element_t<_i, Types...>; - new (data_.data() + pos<_i>()) Type(_other.template get<_i>()); + ::new (static_cast(data_.data() + pos<_i>())) + Type(_other.template get<_i>()); }; (copy_one(_other, std::integral_constant{}), ...); } @@ -143,7 +144,7 @@ class Tuple { const auto copy_one = [this](const auto& _t, std::integral_constant) { using Type = internal::nth_element_t<_i, Types...>; - new (data_.data() + pos<_i>()) Type(_t); + ::new (static_cast(data_.data() + pos<_i>())) Type(_t); }; (copy_one(_types, std::integral_constant{}), ...); } @@ -165,7 +166,8 @@ class Tuple { const auto move_one = [this](auto&& _other, std::integral_constant) { using Type = internal::nth_element_t<_i, Types...>; - new (data_.data() + pos<_i>()) Type(std::move(_other.template get<_i>())); + ::new (static_cast(data_.data() + pos<_i>())) + Type(std::move(_other.template get<_i>())); }; (move_one(_other, std::integral_constant{}), ...); } @@ -175,7 +177,7 @@ class Tuple { const auto move_one = [this](auto&& _t, std::integral_constant) { using Type = internal::nth_element_t<_i, Types...>; - new (data_.data() + pos<_i>()) Type(std::move(_t)); + ::new (static_cast(data_.data() + pos<_i>())) Type(std::move(_t)); }; (move_one(std::move(_types), std::integral_constant{}), ...); } @@ -192,25 +194,25 @@ class Tuple { /// Gets an element by index. template -auto& get(rfl::Tuple& _tup) { +constexpr auto& get(rfl::Tuple& _tup) { return _tup.template get<_index>(); } /// Gets an element by index. template -const auto& get(const rfl::Tuple& _tup) { +constexpr const auto& get(const rfl::Tuple& _tup) { return _tup.template get<_index>(); } /// Gets an element by index. template -auto& get(std::tuple& _tup) { +constexpr auto& get(std::tuple& _tup) { return std::get<_index>(_tup); } /// Gets an element by index. template -const auto& get(const std::tuple& _tup) { +constexpr const auto& get(const std::tuple& _tup) { return std::get<_index>(_tup); } @@ -259,13 +261,13 @@ namespace std { /// Gets an element by index. template -auto& get(rfl::Tuple& _tup) { +constexpr auto& get(rfl::Tuple& _tup) { return _tup.template get<_index>(); } /// Gets an element by index. template -const auto& get(const rfl::Tuple& _tup) { +constexpr const auto& get(const rfl::Tuple& _tup) { return _tup.template get<_index>(); } From 034d2cc092b541c22a0d497c19132b8f4380e71a Mon Sep 17 00:00:00 2001 From: Patrick Urbanke Date: Thu, 31 Oct 2024 12:48:53 +0100 Subject: [PATCH 8/8] Added missing header --- include/rfl/Tuple.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/include/rfl/Tuple.hpp b/include/rfl/Tuple.hpp index 1e08b261..85e38bc4 100644 --- a/include/rfl/Tuple.hpp +++ b/include/rfl/Tuple.hpp @@ -3,6 +3,7 @@ #include #include +#include #include #include #include