One unit test fails on FreeBSD using 1.16.0

[diizzyy]

Hi,

One unit test fails on FreeBSD 14.3-RELEASE (amd64)

Output:
----------------------------------------------------------
Randomness seeded to: 2665499192

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
spdlog-utests is a Catch2 v3.11.0 host application.
Run with -? for options

-------------------------------------------------------------------------------
GMT offset
-------------------------------------------------------------------------------
/usr/ports/devel/spdlog/work/spdlog-1.16.0/tests/test_pattern_formatter.cpp:80
...............................................................................

/usr/ports/devel/spdlog/work/spdlog-1.16.0/tests/test_pattern_formatter.cpp:86: FAILED:
  REQUIRE( log_to_str_with_time(yesterday, "Some message", "%z", spdlog::pattern_time_type::utc, "\n") == "+00:00\n" )
with expansion:
  "-24:00
  "
  ==
  "+00:00
  "

[*** LOG ERROR #0001 ***] [2025-11-01 19:31:52] [as] async log: thread pool doesn't exist anymore
[2025-11-01 19:31:53.300] [test] [trace] Test stdout_st
[2025-11-01 19:31:53.300] [test] [debug] Test stdout_mt
[2025-11-01 19:31:53.300] [test] [info] Test stderr_st
[2025-11-01 19:31:53.300] [test] [info] Test stderr_mt
[2025-11-01 19:31:53.300] [test] [warning] Test stderr_mt
[2025-11-01 19:31:53.300] [test] [error] Test stderr_mt
[2025-11-01 19:31:53.300] [test] [critical] Test stderr_mt
[2025-11-01 19:31:53.300] [test] [info] Test stdout_color_st
[2025-11-01 19:31:53.300] [test] [trace] Test stdout_color_mt
[2025-11-01 19:31:53.300] [test] [debug] Test stderr_color_st
[2025-11-01 19:31:53.300] [test] [info] Test stderr_color_mt
[2025-11-01 19:31:53.300] [test] [warning] Test stderr_color_mt
[2025-11-01 19:31:53.300] [test] [error] Test stderr_color_mt
[2025-11-01 19:31:53.300] [test] [critical] Test stderr_color_mt
[*** LOG ERROR #0002 ***] [2025-11-01 19:31:54] [test-error] argument not found
===============================================================================
test cases: 171 | 170 passed | 1 failed
assertions: 579 | 578 passed | 1 failed

<end of output>

Software used:
fmt 12.1.0 (also occurs with 12.0.0)
spdlog 1.16.0
catch2 3.11.0
OS: FreeBSD 14.3-RELEASE (amd64)

Let me know if you need more information

[gabime]

Is it a regression from prev version?

[diizzyy]

No, 1.15.2 also fails.

[gabime]

Help wanted

[tt4g]

The C language specification is ambiguous regarding time-related structures, so it's unclear what's happening.
I think we need to know what FreeBSD is setting in std::tm structure.
@diizzyy, could you run the following code and tell us what gets printed?

#include <ctime>
#include <iostream>

int main()
{
    std::time_t time = ::time(nullptr);
    std::tm tm;
    ::localtime_r(&time, &tm);

    std::cout << "TZ: " << tm.tm_zone << "\n"
              << "GMT Offset: " << tm.tm_gmtoff << "\n"
              << "DST: " << tm.tm_isdst << std::endl;
    return 0;
}

Depending on the results, it might be better to use calculate_gmt_offset(), which produces predictable results, rather than tm.tm_gmtoff, whose behavior is ambiguous.

spdlog/include/spdlog/details/os-inl.h

Lines 267 to 301 in 3f7e502

	#if defined(sun) || defined(__sun) || defined(_AIX) || \
	(defined(__NEWLIB__) && !defined(__TM_GMTOFF)) || \
	(!defined(__APPLE__) && !defined(_BSD_SOURCE) && !defined(_GNU_SOURCE) && \
	(!defined(_POSIX_VERSION) || (_POSIX_VERSION < 202405L)))
	// 'tm_gmtoff' field is BSD extension and it's missing on SunOS/Solaris
	struct helper {
	static long int calculate_gmt_offset(const std::tm &localtm = details::os::localtime(),
	const std::tm &gmtm = details::os::gmtime()) {
	int local_year = localtm.tm_year + (1900 - 1);
	int gmt_year = gmtm.tm_year + (1900 - 1);
	long int days = (
	// difference in day of year
	localtm.tm_yday -
	gmtm.tm_yday
	// + intervening leap days
	+ ((local_year >> 2) - (gmt_year >> 2)) - (local_year / 100 - gmt_year / 100) +
	((local_year / 100 >> 2) - (gmt_year / 100 >> 2))
	// + difference in years * 365 */
	+ static_cast<long int>(local_year - gmt_year) * 365);
	long int hours = (24 * days) + (localtm.tm_hour - gmtm.tm_hour);
	long int mins = (60 * hours) + (localtm.tm_min - gmtm.tm_min);
	long int secs = (60 * mins) + (localtm.tm_sec - gmtm.tm_sec);
	return secs;
	}
	};
	auto offset_seconds = helper::calculate_gmt_offset(tm);
	#else
	auto offset_seconds = tm.tm_gmtoff;
	#endif

[mandree]

@diizzyy don't do that, it's a waste of time.

Note that FreeBSD already uses calculate_gmt_offset(), which is broken.

The C and C++ languages are very clear, and the bugs are also clear once we know where to look.

Bug #1

• The os-inl.h is botched and compares the external gmt-based time passed in from the test's "yesterday" argument vs. a current ::gmtime_r on a fresh ::time() call. They skew especially if debugging...

The issue is that the first field is already passed in from line 298 and only the second is obtained freshly. Call stack:

#0  spdlog::details::os::utc_minutes_offset(tm const&)::helper::calculate_gmt_offset(tm const&, tm const&) (localtm=..., gmtm=...) at /usr/ports/devel/spdlog/work/spdlog-1.16.0/include/spdlog/details/os-inl.h:275
#1  0x0000000800436358 in spdlog::details::os::utc_minutes_offset (tm=...) at /usr/ports/devel/spdlog/work/spdlog-1.16.0/include/spdlog/details/os-inl.h:298
#2  0x00000008004792ff in spdlog::details::z_formatter<spdlog::details::null_scoped_padder>::get_cached_offset (this=0x800e1bc10, msg=..., tm_time=...) at /usr/ports/devel/spdlog/work/spdlog-1.16.0/include/spdlog/pattern_formatter-inl.h:548
#3  0x00000008004791ed in spdlog::details::z_formatter<spdlog::details::null_scoped_padder>::format (this=0x800e1bc10, msg=..., tm_time=..., dest=...) at /usr/ports/devel/spdlog/work/spdlog-1.16.0/include/spdlog/pattern_formatter-inl.h:527

That won't work because one is an external time ...

    const auto now = std::chrono::system_clock::now();
    const auto yesterday = now - 24h;

and the second one is from a ::time() call. Also localtime_r isn't called at all (it's only the default argument, but we have one) and gmtime_r must be called for the very same time_t, not one obtained from two or three separate ::time() calls. If it were, it would be as bogus as time_t because that wouldn't calculate the offset for the time passed in but for the current time. Which may not have the same offset, think DST switch.
And I can expect that "yesterday" had a different GMT offset than "today", for Europe/Berlin, October 25 and 26 in the year 2025 at 12:00 have offsets +02:00 and +01:00 respectively.

(gdb) b calculate_gmt_offset
Breakpoint 2 at 0x80043639c: file /usr/ports/devel/spdlog/work/spdlog-1.16.0/include/spdlog/details/os-inl.h, line 275.
(gdb) r
Starting program: /usr/ports/devel/spdlog/work/.build/tests/spdlog-utests -b GMT\ offset
Filters: "GMT offset"
Randomness seeded to: 892100788

Breakpoint 2, spdlog::details::os::utc_minutes_offset(tm const&)::helper::calculate_gmt_offset(tm const&, tm const&) (localtm=..., gmtm=...) at /usr/ports/devel/spdlog/work/spdlog-1.16.0/include/spdlog/details/os-inl.h:275
275	            int local_year = localtm.tm_year + (1900 - 1);
(gdb) print localtm
$1 = (const tm &) @0x800e3e1c0: {tm_sec = 22, tm_min = 14, tm_hour = 1, tm_mday = 1, tm_mon = 10, tm_year = 125, tm_wday = 6, tm_yday = 304, tm_isdst = 0, tm_gmtoff = 0, tm_zone = 0x800e96a28 "UTC"}
(gdb) print gmtm
$2 = (const tm &) @0x7fffffffc310: {tm_sec = 22, tm_min = 14, tm_hour = 1, tm_mday = 2, tm_mon = 10, tm_year = 125, tm_wday = 0, tm_yday = 305, tm_isdst = 0, tm_gmtoff = 0, tm_zone = 0x800e96a28 "UTC"}

So we see that the localtm is in reality yesterday's argument - and in GMT/UTC already, not the local time. The compiler issued the gmtime() call because that was defaulted, but the localtime isn't called but instead it uses the external argument that gets passed in from the log_to_str_with_time call. With yesterday's GMT from the chrono library. The functions work as expected... but would have to be called with the passed argument's struct tm reassembled to a timestamp to obtain the other.

FreeBSD's libc time functions seem to work:

(gdb) call spdlog::details::os::localtime()
$7 = {tm_sec = 32, tm_min = 15, tm_hour = 2, tm_mday = 2, tm_mon = 10, tm_year = 125, tm_wday = 0, tm_yday = 305, tm_isdst = 0, tm_gmtoff = 3600, tm_zone = 0x800e9d7f1 "CET"}
(gdb) call spdlog::details::os::gmtime()
$8 = {tm_sec = 40, tm_min = 15, tm_hour = 1, tm_mday = 2, tm_mon = 10, tm_year = 125, tm_wday = 0, tm_yday = 305, tm_isdst = 0, tm_gmtoff = 0, tm_zone = 0x800e96a28 "UTC"}

Bug #2 is that the leap day calculation is off and should add 1900 not 1901 nor 1899 else you are one year off and so are your leap year calculations. I don't oversee if they can work that way at all and what happens with offsets either way.

Extra bug, the preprocessor macros are off and check some options that should be #defined by the application programmer, not checked. gmtime_r was moved to Base in Issue 7 of the Open Group Base Specifications already, so the _POSIX_VERSION value to check is 200809L.

[mandree]

I think the leap day calculation also does not take into account if run across Feb 29 or not.

[mandree]

FreeBSD is fixed. The change just replaces that whole botched 4-line #if monster by a simple #if 0 to disable the botched calculate_gmt_offset() which I consider FUBAR - we have tm.tm_gmtoff which we'll use in FreeBSD henceforth. This still needs fixing as above for operating systems that don't have that field.

[tt4g]

@mandree Thank you for your investigation. I had suspected daylight saving time or minute-based time zones might be the cause.

IMO: Your suggested fixes are certainly valid, but to my knowledge, there is no way to accurately calculate the time zone offset on platforms that do not support tm.tm_gmtoff.
I believe it would be more constructive to improve the method for detecting support for tm.tm_gmtoff.

[gabime]

@mandree can you check if adding !defined(__FreeBSD__) fixes this?

#if !defined(__FreeBSD__) && ( \
    defined(sun) || defined(__sun) || defined(_AIX) ||                        \
    (defined(__NEWLIB__) && !defined(__TM_GMTOFF)) ||                         \
    (!defined(__APPLE__) && !defined(_BSD_SOURCE) && !defined(_GNU_SOURCE) && \
     (!defined(_POSIX_VERSION) || (_POSIX_VERSION < 202405L)))                \
)
// ...
#endif

[mandree]

No thanks. We have a bulletproof (#if 0) solution in place for FreeBSD and I don't want to spend time on an entangled preprocessor stuff that I will not ever want to use.
We can't risk the broken calculate_gmt_offset() reappearing on FreeBSD some day.

I'd be happy to test an improved workaround function and/or cmake or meson checks to detect tm_gmtoff presence though.

[gabime]

No thanks. We have a bulletproof (#if 0)

Happy for you, but we still need to fix it here.

@diizzyy Could you see if the above fixes your test?

I'd be happy to test an improved workaround function

@mandree here’s a better (i hope) fallback that computes a serial Gregorian day count for each tm and subtracts (handles leap years/centuries and day/year rollovers):

static long int calculate_gmt_offset(
    const std::tm& localtm = details::os::localtime(),
    const std::tm& gmtm    = details::os::gmtime())
{
    const auto y_local = 1900LL + localtm.tm_year;
    const auto y_gmt   = 1900LL + gmtm.tm_year;

    // Serial day numbers; leap years handled by the Gregorian formula y/4 - y/100 + y/400.
    const auto dl = 365LL * y_local + y_local / 4 - y_local / 100 + y_local / 400 + localtm.tm_yday;
    const auto dg = 365LL * y_gmt   + y_gmt   / 4 - y_gmt   / 100 + y_gmt   / 400 + gmtm.tm_yday;

    // Turn serial days + time-of-day into seconds
    const auto sec_local = ((dl * 24 + localtm.tm_hour) * 60 + localtm.tm_min) * 60 + localtm.tm_sec;
    const auto sec_gmt = ((dg * 24 + gmtm.tm_hour) * 60 + gmtm.tm_min) * 60 + gmtm.tm_sec;

    return static_cast<long>(sec_local - sec_gmt);
}

It would be great if you could help test it.

cmake or meson checks to detect tm_gmtoff presence though

A flag to cmake to explicity enable this workaround seems reasonable approach. Something like

option(SPDLOG_USE_FALLBACK_GMT_OFFSET "Use fallback to compute GMT offset instead of tm.tm_gmtoff" OFF)

if(SPDLOG_USE_FALLBACK_GMT_OFFSET)
  add_compile_definitions(SPDLOG_NO_TM_GMTOFF=1)
endif()

and then

#if SPDLOG_NO_TM_GMTOFF
     const long off = calculate_gmt_offset(localtm, gmtm);    
#else
    const long off = static_cast<long>(localtm.tm_gmtoff);
#endif

Another possibility is to use SFINAE to detect the absence of tm_gmtoff but it bit cryptic and might not work on older compilers.