Skip to content

Commit 230ba48

Browse files
liuzicheng1987liuzicheng1987
authored
Fix memory safety issues (#241); should resolve #240 
1 parent 7bf71c6 commit 230ba48

File tree

8 files changed

+60
-23
lines changed

8 files changed

+60
-23
lines changed

‎include/rfl/Timestamp.hpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ class Timestamp {
2525

2626
using ReflectionType = std::string;
2727

28+
Timestamp() : tm_(std::tm{}) {}
29+
2830
Timestamp(const char* _str) : tm_(std::tm{}) {
2931
const auto r = strptime(_str, _format.str().c_str(), &tm_);
3032
if (r == NULL) {

‎include/rfl/Tuple.hpp‎

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@
33

44
#include <algorithm>
55
#include <array>
6+
#include <bit>
67
#include <cstdint>
78
#include <limits>
9+
#include <memory>
810
#include <optional>
911
#include <stdexcept>
1012
#include <tuple>
@@ -55,17 +57,16 @@ class Tuple {
5557

5658
/// Gets an element by index.
5759
template <int _index>
58-
auto& get() {
60+
constexpr auto& get() {
5961
using Type = internal::nth_element_t<_index, Types...>;
60-
return *std::launder(reinterpret_cast<Type*>(data_.data() + pos<_index>()));
62+
return *std::bit_cast<Type*>(data_.data() + pos<_index>());
6163
}
6264

6365
/// Gets an element by index.
6466
template <int _index>
65-
const auto& get() const {
67+
constexpr const auto& get() const {
6668
using Type = internal::nth_element_t<_index, Types...>;
67-
return *std::launder(
68-
reinterpret_cast<const Type*>(data_.data() + pos<_index>()));
69+
return *std::bit_cast<const Type*>(data_.data() + pos<_index>());
6970
}
7071

7172
/// Assigns the underlying object.
@@ -132,7 +133,8 @@ class Tuple {
132133
const auto copy_one = [this]<int _i>(const auto& _other,
133134
std::integral_constant<int, _i>) {
134135
using Type = internal::nth_element_t<_i, Types...>;
135-
new (data_.data() + pos<_i>()) Type(_other.template get<_i>());
136+
::new (static_cast<void*>(data_.data() + pos<_i>()))
137+
Type(_other.template get<_i>());
136138
};
137139
(copy_one(_other, std::integral_constant<int, _is>{}), ...);
138140
}
@@ -143,7 +145,7 @@ class Tuple {
143145
const auto copy_one = [this]<int _i>(const auto& _t,
144146
std::integral_constant<int, _i>) {
145147
using Type = internal::nth_element_t<_i, Types...>;
146-
new (data_.data() + pos<_i>()) Type(_t);
148+
::new (static_cast<void*>(data_.data() + pos<_i>())) Type(_t);
147149
};
148150
(copy_one(_types, std::integral_constant<int, _is>{}), ...);
149151
}
@@ -165,7 +167,8 @@ class Tuple {
165167
const auto move_one = [this]<int _i>(auto&& _other,
166168
std::integral_constant<int, _i>) {
167169
using Type = internal::nth_element_t<_i, Types...>;
168-
new (data_.data() + pos<_i>()) Type(std::move(_other.template get<_i>()));
170+
::new (static_cast<void*>(data_.data() + pos<_i>()))
171+
Type(std::move(_other.template get<_i>()));
169172
};
170173
(move_one(_other, std::integral_constant<int, _is>{}), ...);
171174
}
@@ -175,7 +178,7 @@ class Tuple {
175178
const auto move_one = [this]<int _i>(auto&& _t,
176179
std::integral_constant<int, _i>) {
177180
using Type = internal::nth_element_t<_i, Types...>;
178-
new (data_.data() + pos<_i>()) Type(std::move(_t));
181+
::new (static_cast<void*>(data_.data() + pos<_i>())) Type(std::move(_t));
179182
};
180183
(move_one(std::move(_types), std::integral_constant<int, _is>{}), ...);
181184
}
@@ -192,25 +195,25 @@ class Tuple {
192195

193196
/// Gets an element by index.
194197
template <int _index, class... Types>
195-
auto& get(rfl::Tuple<Types...>& _tup) {
198+
constexpr auto& get(rfl::Tuple<Types...>& _tup) {
196199
return _tup.template get<_index>();
197200
}
198201

199202
/// Gets an element by index.
200203
template <int _index, class... Types>
201-
const auto& get(const rfl::Tuple<Types...>& _tup) {
204+
constexpr const auto& get(const rfl::Tuple<Types...>& _tup) {
202205
return _tup.template get<_index>();
203206
}
204207

205208
/// Gets an element by index.
206209
template <int _index, class... Types>
207-
auto& get(std::tuple<Types...>& _tup) {
210+
constexpr auto& get(std::tuple<Types...>& _tup) {
208211
return std::get<_index>(_tup);
209212
}
210213

211214
/// Gets an element by index.
212215
template <int _index, class... Types>
213-
const auto& get(const std::tuple<Types...>& _tup) {
216+
constexpr const auto& get(const std::tuple<Types...>& _tup) {
214217
return std::get<_index>(_tup);
215218
}
216219

@@ -259,13 +262,13 @@ namespace std {
259262

260263
/// Gets an element by index.
261264
template <int _index, class... Types>
262-
auto& get(rfl::Tuple<Types...>& _tup) {
265+
constexpr auto& get(rfl::Tuple<Types...>& _tup) {
263266
return _tup.template get<_index>();
264267
}
265268

266269
/// Gets an element by index.
267270
template <int _index, class... Types>
268-
const auto& get(const rfl::Tuple<Types...>& _tup) {
271+
constexpr const auto& get(const rfl::Tuple<Types...>& _tup) {
269272
return _tup.template get<_index>();
270273
}
271274

‎include/rfl/generic/Writer.hpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,9 @@ struct Writer {
8585
OutputVarType add_null_to_object(const std::string_view& _name,
8686
OutputObjectType* _parent) const noexcept;
8787

88-
void end_array(OutputArrayType* _arr) const noexcept {}
88+
void end_array(OutputArrayType*) const noexcept {}
8989

90-
void end_object(OutputObjectType* _obj) const noexcept {}
90+
void end_object(OutputObjectType*) const noexcept {}
9191

9292
OutputVarType& root() { return root_; }
9393

‎include/rfl/internal/tuple/calculate_positions.hpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ struct Positions {
1717
};
1818

1919
template <class T, unsigned long _last, unsigned long... _is>
20-
consteval auto operator+(const Positions<_last, _is...>& _sizes,
21-
const PositionWrapper<T>& _w) {
20+
consteval auto operator+(const Positions<_last, _is...>&,
21+
const PositionWrapper<T>&) {
2222
if constexpr (_last % alignof(T) == 0) {
2323
constexpr auto last_new = _last + sizeof(T);
2424
return Positions<last_new, _is..., _last>{};

‎include/rfl/internal/tuple/concat.hpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ auto wrap_tuple(const rfl::Tuple<Types...>& _tuple) {
1919

2020
template <class... Types>
2121
auto wrap_tuple(rfl::Tuple<Types...>&& _tuple) {
22-
return TupleWrapper<Types...>{std::move(_tuple)};
22+
return TupleWrapper<Types...>{std::forward<rfl::Tuple<Types...> >(_tuple)};
2323
}
2424

2525
template <class... Types1, class... Types2, int... _is, int... _js>
@@ -46,7 +46,8 @@ auto concat(const Head& _head, const Tail&... _tail) {
4646

4747
template <class Head, class... Tail>
4848
auto concat(Head&& _head, Tail&&... _tail) {
49-
return (wrap_tuple(std::move(_head)) + ... + wrap_tuple(std::move(_tail)))
49+
return (wrap_tuple(std::forward<Head>(_head)) + ... +
50+
wrap_tuple(std::forward<Tail>(_tail)))
5051
.tuple_;
5152
}
5253

‎include/rfl/tuple_cat.hpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ auto tuple_cat(const Head& _head, const Tail&... _tail) {
1313

1414
template <class Head, class... Tail>
1515
auto tuple_cat(Head&& _head, Tail&&... _tail) {
16-
return internal::tuple::concat(std::move(_head), std::move(_tail)...);
16+
return internal::tuple::concat(std::forward<Head>(_head),
17+
std::forward<Tail>(_tail)...);
1718
}
1819

1920
inline auto tuple_cat() { return rfl::Tuple(); }

‎tests/CMakeLists.txt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -O2")
33
if (MSVC)
44
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std:c++20")
55
else()
6-
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")
6+
#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")
7+
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -O3 -Wall -Wextra -ggdb -ftemplate-backtrace-limit=0")
78
endif()
89

910
if (REFLECTCPP_JSON)

‎tests/json/test_segfault.cpp‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
#include <iostream>
2+
#include <rfl.hpp>
3+
#include <rfl/json.hpp>
4+
#include <string>
5+
#include <vector>
6+
7+
#include "write_and_read.hpp"
8+
9+
// This example had led to a segfault on GCC 12.
10+
// To make sure this won't happen again, we include
11+
// it in our tests.
12+
namespace test_segfault {
13+
14+
struct Data {
15+
std::string prop1;
16+
std::string prop2;
17+
std::string prop3;
18+
std::string prop4;
19+
std::string prop5;
20+
std::string prop6;
21+
};
22+
23+
TEST(json, test_segfault) {
24+
const auto data = Data{};
25+
write_and_read(
26+
data,
27+
R"({"prop1":"","prop2":"","prop3":"","prop4":"","prop5":"","prop6":""})");
28+
}
29+
} // namespace test_segfault

0 commit comments

Comments
 (0)