Segmentation fault in date::format()

gv-me commented 2 years ago

Summary:

We use date::format function to get formatted timestamp in one of our internal libraries. We encountered a segmentation fault in function call to date::format() when %T formatter is used anywhere in the format string.

System details:

compiler gcc 10.3.0
OS: ubuntu 20.04
build system CMake
build type: Release or RelWithDebInfo
date library is built statically, with add_subdirectory() call in cmake

cmake options are following

set( USE_SYSTEM_TZ_DB ON)
set( ENABLE_DATE_TESTING OFF)
set( BUILD_TZ_LIB ON)

Crash details:

The segfault occurs in all commits including and after 9a9a42db74df6320e7896282c9d2fe64cf3a674c
We have observed this behavior in Release and Release with Debug Information build modes using CMake
This could not be reproduced in "Debug" build mode

the crash occurs in this code snippet:

    const auto tp = std::chrono::system_clock::now();
    const auto zoned =  date::make_zoned(date::current_zone(), date::floor<millis>(tp));
    const std::string str = date::format("%F %T", zoned);

Stack trace on the latest commit e6f4aed4d11d4e50687e9aa8630c7f0db7d31dba is given below:

#7  | Source "<path to library>/date/include/date/date.h", line 6294, in to_stream<char, std::char_traits<char>, std::chrono::duration<long int, std::ratio<1, 1000> >, const date::time_zone*>
      Source "<path to library>/date/include/date/tz.h", line 1839, in format<char, std::char_traits<char>, std::allocator<char>, date::zoned_time<std::chrono::duration<long int, std::ratio<1, 1000> >, const date::time_zone*> > [0x557755ddf45b]
#6  | Source "<path to library>/date/include/date/date.h", line 6223, in to_stream<char, std::char_traits<char>, std::chrono::duration<long int, std::ratio<1, 1000> > >
    | Source "<path to library>/date/include/date/date.h", line 5709, in operator<< <char, std::char_traits<char> >
    | Source "<path to library>/date/include/date/date.h", line 4121, in operator<< <char, std::char_traits<char> >
      Source "<path to library>/date/include/date/date.h", line 3994, in to_stream<char, std::char_traits<char>, std::chrono::duration<long int, std::ratio<1, 1000> > > [0x557755e2203a]
#5    Source "<path to library>/date/include/date/date.h", line 4026, in print<char, std::char_traits<char> > [0x557755e1f94d]
#4    Object "<executable name redacted>", at 0x557755e34692, in date::detail::save_ostream<char, std::char_traits<char> >::~save_ostream()
#3    Object "<executable name redacted>", at 0x557755e35188, in date::detail::save_istream<char, std::char_traits<char> >::~save_istream()
#2    Object "/usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.28", at 0x7fc0e1b3d2c7, in std::basic_ios<char, std::char_traits<char> >::imbue(std::locale const&)
#1    Object "/usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.28", at 0x7fc0e1ae3cc1, in std::ios_base::imbue(std::locale const&)
#0    Object "/usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.28", at 0x7fc0e1ae4ac0, in std::locale::operator=(std::locale const&)
Segmentation fault (Address not mapped to object [(nil)])

I am unable to reproduce the bug outside out the build environment and cmake projects that we use.

HowardHinnant commented 2 years ago

Thank you for your efforts to create a quality bug report.

I do not know what is wrong for sure. However the most common cause of crashes like this result from errors in the build process. The typical error is that tz.cpp is compiled with one set of configuration macros and the client of tz.h is compiled with another. For example USE_OS_TZDB may be set to 1 in one translation unit and 0 in another.

Can you explore this possibility? Meanwhile I will be reexamining date's streaming code. Thanks.

gv-me commented 2 years ago

Thanks for your quick response. I am using the following code to include date library:

set( USE_SYSTEM_TZ_DB ON CACHE INTERNAL "")
set( ENABLE_DATE_TESTING OFF CACHE INTERNAL "")
set( BUILD_TZ_LIB ON CACHE INTERNAL "")
add_subdirectory(<path to library>/date date EXCLUDE_FROM_ALL)

This should enable the options across the entire date/CMakeLists.txt. I am even getting the following print in CMake. # date: USE_SYSTEM_TZ_DB ON

HowardHinnant commented 2 years ago

For those clients that include tz.h, do they also set USE_SYSTEM_TZ_DB ON?

gv-me commented 2 years ago

the consumer projects link to a library (which links to date::date-tz) they don't set any compile definitions themselves. Something like this.

# libA/CMakeLists.txt
set( USE_SYSTEM_TZ_DB ON CACHE INTERNAL "")
set( ENABLE_DATE_TESTING OFF CACHE INTERNAL "")
set( BUILD_TZ_LIB ON CACHE INTERNAL "")
add_subdirectory(<path to library>/date date EXCLUDE_FROM_ALL)
add_library(liba ... ... ...)
target_link_libraries(liba date::date-tz)

# execB/CMakeLists.txt
add_subdirectory(<path to library>/libA libA )
add_executable(execb dateuser.cpp ..)
target_link_libraries(execb liba)

// dateuser.cpp
#include "date/date.h"
#include "date/tz.h"
 std::string format_time(const TimePoint tp, const std::string& formatString) {
    const auto zoned =  date::make_zoned(date::current_zone(), date::floor<millis>(tp));
    return date::format(formatString, zoned);
}

Should the execB also have some compile definitions set for this? I imagine this might be changing the way preprocessor directives are including/excluding code in date.h and tz.h

gv-me commented 2 years ago

okay so I just did a quick check and it seems that the tz.h included from the second project does also have USE_OS_TZDB set.

Adding this at the top of tz.h and date.h does not fire an error

#ifndef USE_OS_TZDB
# error "USE_OS_TZDB is not set!"
#endif

HowardHinnant / date