freemint / m68k-atari-mint-gcc

Fork of GNU's gcc with support for the m68k-atari-mint target
https://github.com/freemint/m68k-atari-mint-gcc/wiki
Other
26 stars 7 forks source link

Embedding static zoneinfo in libstdc++ #19

Open th-otto opened 1 year ago

th-otto commented 1 year ago

Something that i recently noticed while grepping through config.log:

configure:71667: zoneinfo data directory: none
configure:71682: static tzdata.zi file will be compiled into the library

I haven't checked yes, but i would bet that this has been the case already for quite some time. tzdata.zi is ~109k, and compiling it statically into the library seems a bad idea to me.

This could be fixed by using --with-libstdcxx-zoneinfo=no in configure scripts.

mikrosk commented 1 year ago

Wouldn't this be fixed also by pointing the script to installed mintlib's timezone database? What files does it check for?

th-otto commented 1 year ago

The file is just a text file, and you could also use the hosts version theoretically. It is located in /usr/share/zoneinfo, both on linux and with mintlib. But i just want to avoid compiling in 109k of data (requiring to parse it at runtime), when that information can be taken from already precompiled, much smaller files, by using the already existing functions in mintlib.

Edit: have to check it, but it is possible that cross-compiled binaries are not affected by this (unless you point gcc to some existing file). But it would affect binaries generated by native compilers.

mikrosk commented 1 year ago

Definitely, that's pointless. I'm just worried whether --with-libstdcxx-zoneinfo=no doesn't disable something in libstdc++ for good, i.e. whether all the time-related functions will still work as expected, just relying on underlying libc.

th-otto commented 10 months ago

Initial support for this was added in https://github.com/freemint/m68k-atari-mint-gcc/commit/559993b85744ae09d33eedb1cb062392ac482f94 so it is only present in the gcc-13 branch. There were also numerous patches for this afterwards.

Looks like when you use --with-libstdcxx-zoneinfo=no, instead of compiling the file in, it will be read at runtime, and still parsed. However it seems that only std::chrono::tzdb is affected by this, not the other time functions.

I still think that compiling in a static version is a bad idea, since that would make it impossible to replace it by a newer version.

Edit: --with-libstdcxx-zoneinfo=no is also wrong, that would disable the functionality completely. We have to use --with-libstdcxx-zoneinfo=/usr/share/zoneinfo. Problem with this is that this only works for linux, because then the paths for the cross-compiler and for mint happens to be the same, but not when building a cross-compiler on darwin or mingw, because /usr/share/zoneinfo either does not exist or does not contain a tzdata.zi file.

mikrosk commented 10 months ago

Not to mention if somebody runs the binary on plain TOS. Conversion between time and zones isn't something which only unix-like apps are supposed to do properly. :)

th-otto commented 10 months ago

We could also use --with-libstdcxx-zoneinfo=/usr/share/zoneinfo,static. That should use the directory if it exists (and has a tzdata.zi file), but use the embedded version as a fallback.

OTOH, the usual time functions (that read the binary files from /usr/share/zoneinfo) are also affected by this. So i'm still not sure whats the better approach.

BTW, the static version is compiled in from a tzdata.zi file that is part of the c++ library (in libstdc++v3/src/c++20/tzdata.zi), and not taken from the host.

mikrosk commented 9 months ago

Have you checked how much is executable's file size increased when linking the database statically?

th-otto commented 9 months ago

No, i didn't do any actual tests, because it is only used by the filesystem code (a rather rarely used C++20 feature). But it will increase at least by the size of the file (109k), plus some additional code.

I think it would be best to actually compile it in, for compatibility, and because it might be needed on TOS when the file is not available. But i still have to check how to configure it in MacOS/MinGW.

th-otto commented 3 months ago

Just for info: in recent builds i decided to use --with-libstdcxx-zoneinfo=/usr/share/zoneinfo. The code is only used by the c++ library, and only for new programs compiled with --std=c2x or newer. Such code will not work very well on plain tos anyway because it expects a lot of other posix things, which are only rudimentary emulated by mintlib, so we may as well assume that this should be run on mint. And it will be more consistent with the usual time() functions, which also need access to the timezone database. And btw, on recent linux distros gcc is also configured with that switch.

mikrosk commented 3 months ago

Actually I have a program of my own which does use this timezone stuff (literally I'm asking for some timezone offsets etc there) but haven't tried it to compile / run it on Atari yet.

The code is only used by the c++ library, and only for new programs compiled with --std=c2x or newer. Such code will not work very well on plain tos anyway because it expects a lot of other posix things, which are only rudimentary emulated by mintlib

I guess you mean -std=c++20 ? However I wouldn't be so sure about this statement, one can always just use the newer standard without using any of its new/advanced features. It would be worth checking what happens with a simple C++ program which does not use timezones etc, whether it a) is still relatively small b) it runs at all

th-otto commented 3 months ago

Yes, i mean any option that adds c++ support for C++ >= 2020 (there are many of them)

That zoneinfo stuff is only referenced if you actually use it, by using functions like std::chrono::time_zone::get_info(). Just compiling your program with -std=c++20 without using it features won't make a difference of course.

Note that we only speak about any differences when you run your program an real hardware and plain TOS, and don't have the zoneinfo stuff installed (or it is not not usable because of filesystem restrictions). As noted above, standard functions like time() and gettimeofday() will then also have problems to get the timezone offset. I think it is better to be consistent then, it is not unusual that c++ and standard posix stuff is mixed in an application.

Also i'm not sure how the c++ library actually behaves if you compile the database in. After all, that is only the database, there is still no information about the timezone that should be actually be used (typically taken from /etc/localtime or /usr/share/zoneinfo/localtime, which are symlinks to the actual file).

But feel free to configure compilers with and without that option, and see how it goes.