Bug 2152868 - Using libdate-tz crashes without -DUSE_OS_TZDB=1
Summary: Using libdate-tz crashes without -DUSE_OS_TZDB=1
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: date
Version: 36
Hardware: All
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Aleksei Bavshin
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2022-12-13 11:02 UTC by Jonathan Wakely
Modified: 2022-12-24 01:18 UTC (History)
1 user (show)

Fixed In Version: date-3.0.1^20210518git052eeba-5.fc37 date-3.0.1^20210518git052eeba-5.fc36
Clone Of:
Environment:
Last Closed: 2022-12-24 01:09:51 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)
Patch to change the default value of USE_OS_TZDB (2.28 KB, patch)
2022-12-13 11:12 UTC, Jonathan Wakely
no flags Details | Diff

Description Jonathan Wakely 2022-12-13 11:02:00 UTC
Description of problem:

Fedora's libdate-tz.so library is built with USE_OS_TZDB=1 defined, so that must also be defined when users include it. That isn't obvious to users.

Version-Release number of selected component (if applicable):

date-devel-3.0.1^20210518git052eeba-3.fc36.x86_64
date-devel-3.0.1^20210518git052eeba-4.fc37.x86_64

How reproducible:

Always.

Steps to Reproduce:

Compile this as 'g++ -g leap.cc -ldate-tz' and run it:

#include <date/date.h>
#include <date/tz.h>
#include <iostream>
#include <chrono>

using namespace std::literals::chrono_literals;

int main()
{
    auto t = date::sys_days{date::July/1/2015} - 500ms;
    std::cout << t.time_since_epoch().count() << '\n';
    auto u = date::clock_cast<date::utc_clock>(t);
    std::cout << u.time_since_epoch().count() << '\n';
    auto [is_leap, elapsed] = date::is_leap_second(u);
    std::cout << is_leap << ' ' << elapsed.count() << '\n';
}




Actual results:

$ g++ -g leap.cc -ldate-tz 
$ ./a.out
1435708799500
Segmentation fault (core dumped)



Expected results:

With the macro defined the program runs correctly:

$ g++ -g leap.cc -ldate-tz -DUSE_OS_TZDB=1
$ ./a.out
1435708799500
1435708824500
0 25


Additional info:

The date.pc pkg-config file has the correct macro definitions:

Cflags: -I${includedir} -DUSE_OS_TZDB=1 -DONLY_C_LOCALE=0

And it looks like the cmake files do too.

But users are not required to use pkg-config or cmake, so the simplest way to use this library fails at runtime with a mysterious crash.

Could the tz.h file be patched during the RPM build to set it correctly:

#ifndef USE_OS_TZDB
# define USE_OS_TZDB 1
#endif

so that the header automatically matches how the fedora package was built, and can be used out of the box?

Another option would be to just use #warning to tell the user they should define USE_OS_TZDB appropriately, but I think it should Just Work automatically.

If that's not acceptable, then there should be clear documentation in an easy to find location saying that USE_OS_TZDB=1 must be defined before including date/tz.h

Maybe it could be added to the %description of the libdate-tz sub-package:

--- a/date.spec
+++ b/date.spec
@@ -58,6 +58,8 @@ an easy way to access all of the data in this database, using the types
 from "date.h" and <chrono>. The IANA database also includes data on leap
 seconds, and this library provides utilities to compute with that
 information as well.
+This library is compiled with -DUSE_OS_TZDB=1 so you must use the same
+macro definition before including date/tz.h in your own code.
 
 
 %package        devel

Comment 1 Jonathan Wakely 2022-12-13 11:12:13 UTC
Created attachment 1932320 [details]
Patch to change the default value of USE_OS_TZDB

Comment 2 Aleksei Bavshin 2022-12-13 17:03:33 UTC
Thanks for a detailed report!

The patch makes sense, applied. I'll check if I need to pick any upstream fixes since the last release before publishing the updated build.

Comment 3 Jonathan Wakely 2022-12-13 17:29:00 UTC
Great, thanks!

I also checked with Howard Hinnant, and he agrees that it makes sense for downstream packages to adjust defaults if appropriate for those packages.

Comment 4 Fedora Update System 2022-12-15 16:35:18 UTC
FEDORA-2022-bd53348685 has been submitted as an update to Fedora 37. https://bodhi.fedoraproject.org/updates/FEDORA-2022-bd53348685

Comment 5 Fedora Update System 2022-12-15 16:35:54 UTC
FEDORA-2022-718dc08d38 has been submitted as an update to Fedora 36. https://bodhi.fedoraproject.org/updates/FEDORA-2022-718dc08d38

Comment 6 Fedora Update System 2022-12-16 02:02:27 UTC
FEDORA-2022-bd53348685 has been pushed to the Fedora 37 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf upgrade --enablerepo=updates-testing --refresh --advisory=FEDORA-2022-bd53348685`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2022-bd53348685

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 7 Fedora Update System 2022-12-16 02:38:05 UTC
FEDORA-2022-718dc08d38 has been pushed to the Fedora 36 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf upgrade --enablerepo=updates-testing --refresh --advisory=FEDORA-2022-718dc08d38`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2022-718dc08d38

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 8 Fedora Update System 2022-12-24 01:09:51 UTC
FEDORA-2022-bd53348685 has been pushed to the Fedora 37 stable repository.
If problem still persists, please make note of it in this bug report.

Comment 9 Fedora Update System 2022-12-24 01:18:42 UTC
FEDORA-2022-718dc08d38 has been pushed to the Fedora 36 stable repository.
If problem still persists, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.