Merge "Fix tzalloc(nullptr) and add a test." am: 0e437c00e0

Original change: https://android-review.googlesource.com/c/platform/bionic/+/2630990

Change-Id: Ia318bfe6774da8ecdb242ff50dcb271e6cfa3c5f
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
This commit is contained in:
Elliott Hughes 2023-06-22 21:54:58 +00:00 committed by Automerger Merge Worker
commit 8e1ea43c79
8 changed files with 130 additions and 58 deletions

View file

@ -134,9 +134,9 @@ libc/
tzcode/
# A modified superset of the IANA tzcode. Most of the modifications relate
# to Android's use of a single file (with corresponding index) to contain
# time zone data.
# timezone data.
zoneinfo/
# Android-format time zone data.
# Android-format timezone data.
# See 'Updating tzdata' later.
```

View file

@ -59,7 +59,7 @@ New libc functions in V (API level 35):
* `timespec_getres` (C23 addition).
* `localtime_rz`, `mktime_z`, `tzalloc`, and `tzfree` (NetBSD
extensions implemented in tzcode, and the "least non-standard"
functions for avoiding $TZ if you need to use multiple time zones in
functions for avoiding $TZ if you need to use multiple timezones in
multi-threaded C).
* `mbsrtowcs_l` and `wcsrtombs_l` aliases for `mbsrtowcs` and `wcsrtombs`.

View file

@ -269,7 +269,7 @@ cc_library_static {
"-DTHREAD_SAFE=1",
// The name of the tm_gmtoff field in our struct tm.
"-DTM_GMTOFF=tm_gmtoff",
// TZDEFAULT is not applicable to Android as there is no one file per time zone mapping
// Android uses a system property instead of /etc/localtime, so make callers crash.
"-DTZDEFAULT=NULL",
// Where we store our tzdata.
"-DTZDIR=\"/system/usr/share/zoneinfo\"",

View file

@ -42,22 +42,22 @@ __BEGIN_DECLS
/* If we just use void* in the typedef, the compiler exposes that in error messages. */
struct __timezone_t;
/** The `timezone_t` type that represents a time zone. */
/** The `timezone_t` type that represents a timezone. */
typedef struct __timezone_t* timezone_t;
/** Divisor to compute seconds from the result of a call to clock(). */
#define CLOCKS_PER_SEC 1000000
/**
* The name of the current time zone's non-daylight savings (`tzname[0]`) and
* The name of the current timezone's non-daylight savings (`tzname[0]`) and
* daylight savings (`tzname[1]`) variants. See tzset().
*/
extern char* _Nonnull tzname[];
/** Whether the current time zone ever uses daylight savings time. See tzset(). */
/** Whether the current timezone ever uses daylight savings time. See tzset(). */
extern int daylight;
/** The difference in seconds between UTC and the current time zone. See tzset(). */
/** The difference in seconds between UTC and the current timezone. See tzset(). */
extern long int timezone;
struct sigevent;
@ -86,7 +86,7 @@ struct tm {
int tm_isdst;
/** Offset from UTC (GMT) in seconds for this time. */
long int tm_gmtoff;
/** Name of the time zone for this time. */
/** Name of the timezone for this time. */
const char* _Nullable tm_zone;
};
@ -145,7 +145,7 @@ double difftime(time_t __lhs, time_t __rhs);
* [mktime(3)](http://man7.org/linux/man-pages/man3/mktime.3p.html) converts
* broken-down time `tm` into the number of seconds since the Unix epoch.
*
* See tzset() for details of how the time zone is set, and mktime_rz()
* See tzset() for details of how the timezone is set, and mktime_rz()
* for an alternative.
*
* Returns the time in seconds on success, and returns -1 and sets `errno` on failure.
@ -154,7 +154,7 @@ time_t mktime(struct tm* _Nonnull __tm);
/**
* mktime_z(3) converts broken-down time `tm` into the number of seconds
* since the Unix epoch, assuming the given time zone.
* since the Unix epoch, assuming the given timezone.
*
* Returns the time in seconds on success, and returns -1 and sets `errno` on failure.
*
@ -178,7 +178,7 @@ struct tm* _Nullable localtime(const time_t* _Nonnull __t);
* the number of seconds since the Unix epoch in `t` to a broken-down time.
* That broken-down time will be written to the given struct `tm`.
*
* See tzset() for details of how the time zone is set, and localtime_rz()
* See tzset() for details of how the timezone is set, and localtime_rz()
* for an alternative.
*
* Returns a pointer to a broken-down time on success, and returns null and sets `errno` on failure.
@ -187,7 +187,7 @@ struct tm* _Nullable localtime_r(const time_t* _Nonnull __t, struct tm* _Nonnull
/**
* localtime_rz(3) converts the number of seconds since the Unix epoch in
* `t` to a broken-down time, assuming the given time zone. That broken-down
* `t` to a broken-down time, assuming the given timezone. That broken-down
* time will be written to the given struct `tm`.
*
* Returns a pointer to a broken-down time on success, and returns null and sets `errno` on failure.
@ -278,29 +278,36 @@ char* _Nullable ctime_r(const time_t* _Nonnull __t, char* _Nonnull __buf);
/**
* [tzset(3)](http://man7.org/linux/man-pages/man3/tzset.3.html) tells
* libc that the time zone has changed.
* libc that the timezone has changed.
*
* Android looks at both the system property `persist.sys.timezone` and the
* environment variable `TZ`. The former is the device's current time zone
* as shown in Settings, while the latter is usually unset but can be used
* to override the global setting. This is a bad idea outside of unit tests
* or single-threaded programs because it's inherently thread-unsafe.
* See tzalloc(), localtime_rz(), mktime_z(), and tzfree() for an
* alternative.
* tzset() on Android looks at both the system property
* `persist.sys.timezone` and the environment variable `TZ`. The former is
* the device's current timezone as shown in Settings, while the latter is
* usually unset but can be used to override the global setting. This is a
* bad idea outside of unit tests or single-threaded programs because it's
* inherently thread-unsafe. See tzalloc(), localtime_rz(), mktime_z(),
* and tzfree() for an alternative.
*/
void tzset(void);
/**
* tzalloc(3) allocates a time zone corresponding to the given Olson id.
* tzalloc(3) allocates a timezone corresponding to the given Olson ID.
*
* Returns a time zone object on success, and returns NULL and sets `errno` on failure.
* A null `id` returns the system timezone (as seen in Settings) from
* the system property `persist.sys.timezone`, ignoring `$TZ`. Although
* tzset() honors `$TZ`, callers of tzalloc() can use `$TZ` themselves if
* that's the (thread-unsafe) behavior they want, but by ignoring `$TZ`
* tzalloc() is thread safe (though obviously the system timezone can
* change, especially if your mobile device is actually mobile!).
*
* Returns a timezone object on success, and returns NULL and sets `errno` on failure.
*
* Available since API level 35.
*/
timezone_t _Nullable tzalloc(const char* _Nullable __id) __INTRODUCED_IN(35);
/**
* tzfree(3) frees a time zone object returned by tzalloc().
* tzfree(3) frees a timezone object returned by tzalloc().
*
* Available since API level 35.
*/
@ -320,7 +327,7 @@ clock_t clock(void);
/**
* [clock_getcpuclockid(3)](http://man7.org/linux/man-pages/man3/clock_getcpuclockid.3.html)
* gets the clock id of the cpu-time clock for the given `pid`.
* gets the clock ID of the cpu-time clock for the given `pid`.
*
* Returns 0 on success, and returns -1 and returns an error number on failure.
*/

View file

@ -321,7 +321,7 @@ persist.odm. u:object_r:vendor_default_prop:s0
persist.vendor. u:object_r:vendor_default_prop:s0
vendor. u:object_r:vendor_default_prop:s0
# Properties that relate to time / time zone detection behavior.
# Properties that relate to time / timezone detection behavior.
persist.time. u:object_r:time_prop:s0
# Properties that relate to server configurable flags

View file

@ -36,10 +36,45 @@
#include "private/CachedProperty.h"
extern "C" void tzset_unlocked(void);
extern "C" void __bionic_get_system_tz(char* buf, size_t n);
extern "C" int __bionic_open_tzdata(const char*, int32_t*);
extern "C" void tzsetlcl(char const*);
void __bionic_get_system_tz(char* buf, size_t n) {
static CachedProperty persist_sys_timezone("persist.sys.timezone");
const char* name = persist_sys_timezone.Get();
// If the system property is not set, perhaps because this is called
// before the default value has been set (the recovery image being a
// classic example), fall back to GMT.
if (name == nullptr) name = "GMT";
strlcpy(buf, name, n);
if (!strcmp(buf, "GMT")) {
// Typically we'll set the system property to an Olson ID, but
// java.util.TimeZone also supports the "GMT+xxxx" style, and at
// least historically (see http://b/25463955) some Android-based set
// top boxes would get the timezone from the TV network in this format
// and use it directly in the system property. This caused trouble
// for native code because POSIX and Java disagree about the sign in
// a timezone string. For POSIX, "GMT+3" means "3 hours west/behind",
// but for Java it means "3 hours east/ahead". Since (a) Java is the
// one that matches human expectations and (b) this system property is
// used directly by Java, we flip the sign here to translate from Java
// to POSIX. We only need to worry about the "GMT+xxxx" case because
// the expectation is that these are valid java.util.TimeZone ids,
// not general POSIX custom timezone specifications (which is why this
// code only applies to the system property, and not to the environment
// variable).
char sign = buf[3];
if (sign == '-' || sign == '+') {
buf[3] = (sign == '-') ? '+' : '-';
}
}
}
void tzset_unlocked() {
// The TZ environment variable is meant to override the system-wide setting.
const char* name = getenv("TZ");
@ -47,26 +82,10 @@ void tzset_unlocked() {
// If that's not set, look at the "persist.sys.timezone" system property.
if (name == nullptr) {
static CachedProperty persist_sys_timezone("persist.sys.timezone");
if ((name = persist_sys_timezone.Get()) != nullptr && strlen(name) > 3) {
// POSIX and Java disagree about the sign in a timezone string. For POSIX, "GMT+3" means
// "3 hours west/behind", but for Java it means "3 hours east/ahead". Since (a) Java is
// the one that matches human expectations and (b) this system property is used directly
// by Java, we flip the sign here to translate from Java to POSIX. http://b/25463955.
char sign = name[3];
if (sign == '-' || sign == '+') {
strlcpy(buf, name, sizeof(buf));
buf[3] = (sign == '-') ? '+' : '-';
name = buf;
}
}
__bionic_get_system_tz(buf, sizeof(buf));
name = buf;
}
// If the system property is also not available (because you're running AOSP on a WiFi-only
// device, say), fall back to GMT.
if (name == nullptr) name = "GMT";
tzsetlcl(name);
}
@ -192,14 +211,14 @@ static int __bionic_open_tzdata_path(const char* path,
close(fd);
// This file descriptor (-1) is passed to localtime.c. In invalid fd case
// upstream passes errno value around methods and having 0 there will
// indicate that time zone was found and read successfully and localtime's
// indicate that timezone was found and read successfully and localtime's
// internal state was properly initialized (which wasn't as we couldn't find
// requested time zone in the tzdata file).
// requested timezone in the tzdata file).
// If we reached this point errno is unlikely to be touched. It is only
// close(fd) which can do it, but that is very unlikely to happen. And
// even if it happens we can't extract any useful insights from it.
// We are overriding it to ENOENT as it matches upstream expectations -
// time zone is absent in the tzdata file == there is no TZif file in
// timezone is absent in the tzdata file == there is no TZif file in
// /usr/share/zoneinfo.
errno = ENOENT;
return -1;
@ -219,7 +238,7 @@ int __bionic_open_tzdata(const char* olson_id, int32_t* entry_length) {
int fd;
// Try the two locations for the tzdata file in a strict order:
// 1: The time zone data module which contains the main copy. This is the
// 1: The timezone data module which contains the main copy. This is the
// common case for current devices.
// 2: The ultimate fallback: the non-updatable copy in /system.

View file

@ -374,8 +374,11 @@ union input_buffer {
+ 4 * TZ_MAX_TIMES];
};
// Android-removed: There is no directory with file-per-time zone on Android.
#ifndef __BIONIC__
#if defined(__BIONIC__)
// Android: there is no directory with one file per timezone on Android,
// but we do have a system property instead.
#include <sys/system_properties.h>
#else
/* TZDIR with a trailing '/' rather than a trailing '\0'. */
static char const tzdirslash[sizeof TZDIR] = TZDIR "/";
#endif
@ -415,13 +418,20 @@ tzloadbody(char const *name, struct state *sp, bool doextend,
#endif
register union input_buffer *up = &lsp->u.u;
register int tzheadsize = sizeof(struct tzhead);
char system_tz_name[PROP_VALUE_MAX];
sp->goback = sp->goahead = false;
if (! name) {
#if defined(__BIONIC__)
extern void __bionic_get_system_tz(char* , size_t);
__bionic_get_system_tz(system_tz_name, sizeof(system_tz_name));
name = system_tz_name;
#else
name = TZDEFAULT;
if (! name)
return EINVAL;
#endif
}
#if defined(__BIONIC__)

View file

@ -97,7 +97,7 @@ TEST(time, mktime_TZ_as_UTC_and_offset) {
static void* gmtime_no_stack_overflow_14313703_fn(void*) {
const char* original_tz = getenv("TZ");
// Ensure we'll actually have to enter tzload by using a time zone that doesn't exist.
// Ensure we'll actually have to enter tzload by using a timezone that doesn't exist.
setenv("TZ", "gmtime_stack_overflow_14313703", 1);
tzset();
if (original_tz != nullptr) {
@ -324,7 +324,7 @@ TEST(time, strftime_Z_null_tm_zone) {
// According to C language specification the only tm struct field needed to
// find out replacement for %z and %Z in strftime is tm_isdst. Which is
// wrong, as time zones change their standard offset and even DST savings.
// wrong, as timezones change their standard offset and even DST savings.
// tzcode deviates from C language specification and requires tm struct either
// to be output of localtime-like functions or to be modified by mktime call
// before passing to strftime. See tz mailing discussion for more details
@ -560,7 +560,7 @@ TEST(time, strptime_Z) {
EXPECT_EQ(1, tm.tm_isdst);
EXPECT_EQ(3600, tm.tm_gmtoff);
// And as long as we're in Europe/Berlin, those are the only time zone
// And as long as we're in Europe/Berlin, those are the only timezone
// abbreviations that are recognized.
tm = {};
ASSERT_TRUE(strptime("PDT", "%Z", &tm) == nullptr);
@ -1118,7 +1118,7 @@ TEST(time, bug_31938693) {
// Actual underlying bug (the code change, not the tzdata upgrade that first exposed the bug):
// http://b/31848040
// This isn't a great test, because very few time zones were actually affected, and there's
// This isn't a great test, because very few timezones were actually affected, and there's
// no real logic to which ones were affected: it was just a coincidence of the data that came
// after them in the tzdata file.
@ -1159,10 +1159,10 @@ TEST(time, bug_31938693) {
TEST(time, bug_31339449) {
// POSIX says localtime acts as if it calls tzset.
// tzset does two things:
// 1. it sets the time zone ctime/localtime/mktime/strftime will use.
// 1. it sets the timezone ctime/localtime/mktime/strftime will use.
// 2. it sets the global `tzname`.
// POSIX says localtime_r need not set `tzname` (2).
// Q: should localtime_r set the time zone (1)?
// Q: should localtime_r set the timezone (1)?
// Upstream tzcode (and glibc) answer "no", everyone else answers "yes".
// Pick a time, any time...
@ -1361,7 +1361,7 @@ TEST(time, localtime_rz) {
ASSERT_EQ(&tm, localtime_rz(seoul, &t, &tm));
AssertTmEq(tm, 17);
// Just check that mktime()'s time zone didn't change.
// Just check that mktime()'s timezone didn't change.
tm = {};
ASSERT_EQ(&tm, localtime_r(&t, &tm));
ASSERT_EQ(0, tm.tm_hour);
@ -1403,7 +1403,7 @@ TEST(time, mktime_z) {
tm = {.tm_year = 93, .tm_mday = 1};
ASSERT_EQ(725814000, mktime_z(seoul, &tm));
// Just check that mktime()'s time zone didn't change.
// Just check that mktime()'s timezone didn't change.
tm = {.tm_year = 93, .tm_mday = 1};
ASSERT_EQ(725875200, mktime(&tm));
@ -1416,3 +1416,39 @@ TEST(time, mktime_z) {
GTEST_SKIP() << "glibc doesn't have timezone_t";
#endif
}
TEST(time, tzalloc_nullptr) {
#if __BIONIC__
// tzalloc(nullptr) returns the system timezone.
timezone_t default_tz = tzalloc(nullptr);
ASSERT_NE(nullptr, default_tz);
// Check that mktime_z() with the default timezone matches mktime().
// This assumes that the system timezone doesn't change during the test,
// but that should be unlikely, and we don't have much choice if we
// want to write a test at all.
// We unset $TZ before calling mktime() because mktime() honors $TZ.
unsetenv("TZ");
struct tm tm = {.tm_year = 93, .tm_mday = 1};
time_t t = mktime(&tm);
ASSERT_EQ(t, mktime_z(default_tz, &tm));
// Check that changing $TZ doesn't affect the tzalloc() default in
// the same way it would the mktime() default.
setenv("TZ", "America/Los_Angeles", 1);
tzset();
ASSERT_EQ(t, mktime_z(default_tz, &tm));
setenv("TZ", "Europe/London", 1);
tzset();
ASSERT_EQ(t, mktime_z(default_tz, &tm));
setenv("TZ", "Asia/Seoul", 1);
tzset();
ASSERT_EQ(t, mktime_z(default_tz, &tm));
tzfree(default_tz);
#else
GTEST_SKIP() << "glibc doesn't have timezone_t";
#endif
}