[scudo] Align objects with alignas

[vitalybuka]

Summary

Operator new must align allocations for types with large alignment.

Before c++17 behavior was implementation defined and both clang and gc++
before 11 ignored alignment. Miss-aligned objects mysteriously crashed
tests on Ubuntu 14.

Alternatives are compile with -std=c++17 or -faligned-new, but they were
discarded as less portable.

[vitalybuka]

typo

[pcc]

I'm not sure that we can do this until all of our minimum compiler versions support C++17. E.g. minimum gcc version is 5.1.0 which was released in 2015, so it presumably doesn't have C++17 support.

[hctim]

In D101874#2737687, @pcc wrote:

I'm not sure that we can do this until all of our minimum compiler versions support C++17. E.g. minimum gcc version is 5.1.0 which was released in 2015, so it presumably doesn't have C++17 support.

Looks like gcc 5.1 has support for -std=c++17, but unfortunately no support for -faligned-new (which I thought would be preferable to bumping to cxx17).

Shouldn't we just mark the individual test rather than bumping the whole allocator?

[vitalybuka]

In D101874#2737809, @hctim wrote:

In D101874#2737687, @pcc wrote:

I'm not sure that we can do this until all of our minimum compiler versions support C++17. E.g. minimum gcc version is 5.1.0 which was released in 2015, so it presumably doesn't have C++17 support.

Looks like gcc 5.1 has support for -std=c++17, but unfortunately no support for -faligned-new (which I thought would be preferable to bumping to cxx17).

I would prefer -std=c++17 as more portable.

Shouldn't we just mark the individual test rather than bumping the whole allocator?

Why? No one complained when you asked for entire compiler-rt.

[vitalybuka]

In D101874#2737687, @pcc wrote:

I'm not sure that we can do this until all of our minimum compiler versions support C++17. E.g. minimum gcc version is 5.1.0 which was released in 2015, so it presumably doesn't have C++17 support.

Probably I should be able to convert tests to allocate aligned stuff on the stack.

[vitalybuka]

switch to new overload

[hctim]

In D101874#2737834, @vitalybuka wrote:

Shouldn't we just mark the individual test rather than bumping the whole allocator?

Why? No one complained when you asked for entire compiler-rt.

I was planning on abandoning that patch after your fix in D101514. It's not clear to me whether there's going to be enough benefit to switch crt to c++17, but these two bugs in the last two weeks are definitely fresh in the mind. Maybe let's bring it up in the next team meeting.

This test fix LGTM.

[vitalybuka]

BTW Allocating them on stack is not an option. TestConfig1 on 64bit defines allocator as .5GB object :)

compiler-rt/lib/scudo/standalone/CMakeLists.txt

 add_compiler_rt_component(scudo_standalone)
 if (COMPILER_RT_HAS_GWP_ASAN)
   add_dependencies(scudo_standalone gwp_asan)
 endif()
 
 include_directories(../.. include)
 
 set(SCUDO_CFLAGS)
 
 list(APPEND SCUDO_CFLAGS
   -Werror=conversion
   -Wall
   -g
+  -std=c++17  # For consistency with unit-tests only.
   -nostdinc++)
 
 # Remove -stdlib= which is unused when passing -nostdinc++.
 string(REGEX REPLACE "-stdlib=[a-zA-Z+]*" "" CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS})
 
 append_list_if(COMPILER_RT_HAS_FVISIBILITY_HIDDEN_FLAG -fvisibility=hidden SCUDO_CFLAGS)
 
 append_list_if(COMPILER_RT_HAS_FNO_EXCEPTIONS_FLAG -fno-exceptions SCUDO_CFLAGS)

compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt

 include_directories(..)
 
 add_custom_target(ScudoUnitTests)
 set_target_properties(ScudoUnitTests PROPERTIES
   FOLDER "Compiler-RT Tests")
 
 set(SCUDO_UNITTEST_CFLAGS
   ${COMPILER_RT_UNITTEST_CFLAGS}
   ${COMPILER_RT_GTEST_CFLAGS}
   -I${COMPILER_RT_SOURCE_DIR}/include
   -I${COMPILER_RT_SOURCE_DIR}/lib
   -I${COMPILER_RT_SOURCE_DIR}/lib/scudo/standalone
   -I${COMPILER_RT_SOURCE_DIR}/lib/scudo/standalone/include
   -DGTEST_HAS_RTTI=0
   -DSCUDO_DEBUG=1
   -g
+  -std=c++17  # Required for "new" for "alignas" types.
   # Extra flags for the C++ tests
   # TODO(kostyak): find a way to make -fsized-deallocation work
   -Wno-mismatched-new-delete)
 
 if(ANDROID)
   list(APPEND SCUDO_UNITTEST_CFLAGS -fno-emulated-tls)
 endif()