Skip to content

Commit

Permalink
[C++] Integrate std::span support for flyweight API
Browse files Browse the repository at this point in the history 
The impetus was a bug that we ran into when writing a string-literal to a fixed-width char field:

```c++
flyweight.putFixedChar("hello");
```

This is unsafe:
- If the field size is less than 6, we overrun the buffer and corrupt it.
- If the field size is more than 6, we don't zero pad the rest of it.

Instead, we build on support for the std::string_view getters and setters, which do length checking.
std::span generalizes this to fixed-width fields of all types. Notably, if the size of the std::span
is knowable at compile time, we pay no runtime cost for the length checking, and we should get
similar performance to the existing API which takes a raw pointer.

Further, we add a sbetool option to disable accepting arrays by raw pointer, which should prevent
memcpy operation without bounds checking. This is off by default to avoid a breaking change.
  • Loading branch information
@mspdt22
mspdt22 committed Nov 20, 2024
1 parent b8c7056 commit 60af6f0
Show file tree
Hide file tree
Showing 3 changed files with 203 additions and 6 deletions.
134 changes: 129 additions & 5 deletions sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/cpp/CppGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ public class CppGenerator implements CodeGenerator
{
private static final boolean DISABLE_IMPLICIT_COPYING = Boolean.parseBoolean(
System.getProperty("sbe.cpp.disable.implicit.copying", "false"));
private static final boolean DISABLE_RAW_ARRAYS = Boolean.parseBoolean(
System.getProperty("sbe.cpp.disable.raw.arrays", "false"));
private static final String BASE_INDENT = "";
private static final String INDENT = " ";
private static final String TWO_INDENT = INDENT + INDENT;
Expand Down Expand Up @@ -1612,6 +1614,23 @@ private static CharSequence generateArrayFieldNotPresentCondition(final int sinc
sinceVersion);
}

private static CharSequence generateSpanFieldNotPresentCondition(
final int sinceVersion, final String indent, final String cppTypeName)
{
if (0 == sinceVersion)
{
return "";
}

return String.format(
indent + " if (m_actingVersion < %1$d)\n" +
indent + " {\n" +
indent + " return std::span<const $2%s>();\n" +
indent + " }\n\n",
sinceVersion,
cppTypeName);
}

private static CharSequence generateStringNotPresentCondition(final int sinceVersion, final String indent)
{
if (0 == sinceVersion)
Expand Down Expand Up @@ -1681,10 +1700,16 @@ private CharSequence generateFileHeader(
"#if __cplusplus >= 201703L\n" +
"# include <string_view>\n" +
"# define SBE_NODISCARD [[nodiscard]]\n" +
"# define SBE_USE_STRING_VIEW 1\n" +
"#else\n" +
"# define SBE_NODISCARD\n" +
"#endif\n\n" +

"#if __cplusplus >= 202002L\n" +
"# include <span>\n" +
"# define SBE_USE_SPAN 1\n" +
"#endif\n\n" +

"#if !defined(__STDC_LIMIT_MACROS)\n" +
"# define __STDC_LIMIT_MACROS 1\n" +
"#endif\n\n" +
Expand Down Expand Up @@ -2236,12 +2261,38 @@ private void generateArrayProperty(
accessOrderListenerCall);

new Formatter(sb).format("\n" +
indent + " %1$s &put%2$s(const char *const src)%7$s\n" +
indent + " #ifdef SBE_USE_SPAN\n" +
indent + " SBE_NODISCARD std::span<const %5$s> get%1$sAsSpan() const%7$s\n" +
indent + " {\n" +
"%3$s" +
"%6$s" +
indent + " const char *buffer = m_buffer + m_offset + %4$d;\n" +
indent + " return std::span<const %5$s>(reinterpret_cast<const %5$s*>(buffer), %2$d);\n" +
indent + " }\n" +
indent + " #endif\n",
toUpperFirstChar(propertyName),
arrayLength,
generateSpanFieldNotPresentCondition(propertyToken.version(), indent, cppTypeName),
offset,
cppTypeName,
accessOrderListenerCall,
noexceptDeclaration);

new Formatter(sb).format("\n" +
indent + " #ifdef SBE_USE_SPAN\n" +
indent + " template <std::size_t N>\n" +
indent + " %1$s &put%2$s(std::span<const %4$s, N> src)%7$s\n" +
indent + " {\n" +
indent + " static_assert(N <= %5$d, \"array too large for put%2$s\");\n\n" +
"%6$s" +
indent + " std::memcpy(m_buffer + m_offset + %3$d, src, sizeof(%4$s) * %5$d);\n" +
indent + " std::memcpy(m_buffer + m_offset + %3$d, src.data(), sizeof(%4$s) * N);\n" +
indent + " for (std::size_t start = N; start < %5$d; ++start)\n" +
indent + " {\n" +
indent + " m_buffer[m_offset + %3$d + start] = 0;\n" +
indent + " }\n\n" +
indent + " return *this;\n" +
indent + " }\n",
indent + " }\n" +
indent + " #endif\n",
containingClassName,
toUpperFirstChar(propertyName),
offset,
Expand All @@ -2250,6 +2301,79 @@ private void generateArrayProperty(
accessOrderListenerCall,
noexceptDeclaration);

if (encodingToken.encoding().primitiveType() != PrimitiveType.CHAR)
{
new Formatter(sb).format("\n" +
indent + " #ifdef SBE_USE_SPAN\n" +
indent + " %1$s &put%2$s(std::span<const %4$s> src)\n" +
indent + " {\n" +
indent + " const std::size_t srcLength = src.size();\n" +
indent + " if (srcLength > %5$d)\n" +
indent + " {\n" +
indent + " throw std::runtime_error(\"array too large for put%2$s [E106]\");\n" +
indent + " }\n\n" +

"%6$s" +
indent + " std::memcpy(m_buffer + m_offset + %3$d, src.data(), sizeof(%4$s) * srcLength);\n" +
indent + " for (std::size_t start = srcLength; start < %5$d; ++start)\n" +
indent + " {\n" +
indent + " m_buffer[m_offset + %3$d + start] = 0;\n" +
indent + " }\n\n" +
indent + " return *this;\n" +
indent + " }\n" +
indent + " #endif\n",
containingClassName,
toUpperFirstChar(propertyName),
offset,
cppTypeName,
arrayLength,
accessOrderListenerCall);
}

if (!DISABLE_RAW_ARRAYS)
{
new Formatter(sb).format("\n" +
indent + " #ifdef SBE_USE_SPAN\n" +
indent + " template <typename T>\n" +
// If std::span is available, redirect string literals to the std::span-accepting overload,
// where we can do compile-time bounds checking.
indent + " %1$s &put%2$s(T&& src) %7$s requires\n" +
indent + " (std::is_pointer_v<std::remove_reference_t<T>> &&\n" +
indent + " !std::is_array_v<std::remove_reference_t<T>>)\n" +
indent + " #else\n" +
indent + " %1$s &put%2$s(const char *const src)%7$s\n" +
indent + " #endif\n" +
indent + " {\n" +
"%6$s" +
indent + " std::memcpy(m_buffer + m_offset + %3$d, src, sizeof(%4$s) * %5$d);\n" +
indent + " return *this;\n" +
indent + " }\n",
containingClassName,
toUpperFirstChar(propertyName),
offset,
cppTypeName,
arrayLength,
accessOrderListenerCall,
noexceptDeclaration);

}
if (encodingToken.encoding().primitiveType() == PrimitiveType.CHAR)
{
// Resolve ambiguity of string literal arguments, which match both
// std::span<const char, N> and std::string_view overloads.
new Formatter(sb).format("\n" +
indent + " #ifdef SBE_USE_SPAN\n" +
indent + " template <std::size_t N>\n" +
indent + " %1$s &put%2$s(const char (&src)[N])%3$s\n" +
indent + " {\n" +
indent + " return put%2$s(std::span<const char, N>(src));\n" +
indent + " }\n" +
indent + " #endif\n",
containingClassName,
toUpperFirstChar(propertyName),
noexceptDeclaration);
}

if (arrayLength > 1 && arrayLength <= 4)
{
sb.append("\n").append(indent).append(" ")
Expand Down Expand Up @@ -2310,7 +2434,7 @@ private void generateArrayProperty(
generateJsonEscapedStringGetter(sb, encodingToken, indent, propertyName, accessOrderListenerCall);

new Formatter(sb).format("\n" +
indent + " #if __cplusplus >= 201703L\n" +
indent + " #ifdef SBE_USE_STRING_VIEW\n" +
indent + " SBE_NODISCARD std::string_view get%1$sAsStringView() const%6$s\n" +
indent + " {\n" +
"%4$s" +
Expand All @@ -2332,7 +2456,7 @@ private void generateArrayProperty(
noexceptDeclaration);

new Formatter(sb).format("\n" +
indent + " #if __cplusplus >= 201703L\n" +
indent + " #ifdef SBE_USE_STRING_VIEW\n" +
indent + " %1$s &put%2$s(const std::string_view str)\n" +
indent + " {\n" +
indent + " const std::size_t srcLength = str.length();\n" +
Expand Down
13 changes: 12 additions & 1 deletion sbe-tool/src/test/cpp/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,11 @@ if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
sbe_test(DtoTest codecs)
target_compile_features(DtoTest PRIVATE cxx_std_17)
endif()

# Check if the GCC version supports C++20
if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL "11.0")
target_compile_features(CodeGenTest PRIVATE cxx_std_20)
endif()
endif()

if (CMAKE_CXX_COMPILER_ID STREQUAL "CLang")
Expand All @@ -111,12 +116,18 @@ if (CMAKE_CXX_COMPILER_ID STREQUAL "CLang")
sbe_test(DtoTest codecs)
target_compile_features(DtoTest PRIVATE cxx_std_17)
endif()

# Check if CLang version supports C++20
if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL "10.0")
target_compile_features(CodeGenTest PRIVATE cxx_std_20)
endif()
endif()

if (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC")
# Check if MSVC version supports C++17
# Check if MSVC version supports C++17 / C++20
if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL "19.14")
sbe_test(DtoTest codecs)
target_compile_options(DtoTest PRIVATE /std:c++17)
target_compile_options(CodeGenTest PRIVATE /std:c++20)
endif()
endif()
62 changes: 62 additions & 0 deletions sbe-tool/src/test/cpp/CodeGenTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1083,3 +1083,65 @@ TEST_F(CodeGenTest, shouldAllowForMultipleIterations)
std::string passFive = walkCar(carDecoder);
EXPECT_EQ(passOne, passFive);
}

#ifdef SBE_USE_STRING_VIEW
TEST_F(CodeGenTest, shouldBeAbleToUseStdStringViewMethods)
{
std::string vehicleCode(VEHICLE_CODE, Car::vehicleCodeLength());

char buffer[BUFFER_LEN] = {};
std::uint64_t baseOffset = MessageHeader::encodedLength();
Car car;
car.wrapForEncode(buffer, baseOffset, sizeof(buffer));
car.putVehicleCode(std::string_view(vehicleCode));

car.sbeRewind();
EXPECT_EQ(car.getVehicleCodeAsStringView(), vehicleCode);
}
#endif

#ifdef SBE_USE_SPAN
TEST_F(CodeGenTest, shouldBeAbleToUseStdSpanViewMethods)
{
char buffer[BUFFER_LEN] = {};

std::uint64_t baseOffset = MessageHeader::encodedLength();
Car car;
car.wrapForEncode(buffer, baseOffset, sizeof(buffer));

{
std::array<std::int32_t, Car::someNumbersLength()> numbers;
for (std::uint64_t i = 0; i < Car::someNumbersLength(); i++)
{
numbers[i] = static_cast<std::int32_t>(i);
}
car.putSomeNumbers(numbers);
}

car.sbeRewind();

{
std::span<const std::int32_t> numbers = car.getSomeNumbersAsSpan();
ASSERT_EQ(numbers.size(), Car::someNumbersLength());
for (std::uint64_t i = 0; i < numbers.size(); i++)
{
EXPECT_EQ(numbers[i], static_cast<std::int32_t>(i));
}
}
}
#endif

#ifdef SBE_USE_SPAN
TEST_F(CodeGenTest, shouldBeAbleToResolveStringLiterals)
{
char buffer[BUFFER_LEN] = {};

std::uint64_t baseOffset = MessageHeader::encodedLength();
Car car;
car.wrapForEncode(buffer, baseOffset, sizeof(buffer));
car.putVehicleCode("ABCDE");

car.sbeRewind();
EXPECT_EQ(car.getVehicleCodeAsStringView(), "ABCDE");
}
#endif

0 comments on commit 60af6f0

Please sign in to comment.