tst: Provide Y2038 tests for mktime (tst-mktime4.c)

Message ID 20210125130308.32242-1-lukma@denx.de
State Superseded
Headers
Series tst: Provide Y2038 tests for mktime (tst-mktime4.c) |

Commit Message

Lukasz Majewski Jan. 25, 2021, 1:03 p.m. UTC
  This change adds new test to assess mktime's functionality.

To be more specific - following use cases are checked:
- Pass struct tm as epoch time
- Pass struct tm as value just before Y2038 threshold (returned
  value shall be 0x7FFFFFFF)
- Pass struct tm as the first value after Y2038 threshold
  (expected value - 0x80000000)
---
 time/Makefile      |  2 +-
 time/tst-mktime4.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 83 insertions(+), 1 deletion(-)
 create mode 100644 time/tst-mktime4.c
  

Comments

Andreas Schwab Jan. 25, 2021, 1:37 p.m. UTC | #1
error: tst-mktime4.c:53: *** mktime returned -3600 (expected 0)

error: 1 test failures

Andreas.
  
Lukasz Majewski Jan. 25, 2021, 3:35 p.m. UTC | #2
Hi Andreas,

> error: tst-mktime4.c:53: *** mktime returned -3600 (expected 0)
> 
> error: 1 test failures
> 

Are you sure that this is correct? The -3600 would be from
tst-difftime.c


> Andreas.
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
  
Andreas Schwab Jan. 25, 2021, 3:48 p.m. UTC | #3
On Jan 25 2021, Lukasz Majewski wrote:

> Hi Andreas,
>
>> error: tst-mktime4.c:53: *** mktime returned -3600 (expected 0)
>> 
>> error: 1 test failures
>> 
>
> Are you sure that this is correct? The -3600 would be from
> tst-difftime.c

That's literally the output of your test.

Andreas.
  
Joseph Myers Jan. 25, 2021, 6:13 p.m. UTC | #4
On Mon, 25 Jan 2021, Lukasz Majewski wrote:

> +  /* Check that mktime(1970-01-01 00:00:00) returns 0.  */
> +  test_mktime_helper (&tm0, 0);
> +
> +  /* Check that mktime(2038-01-19 03:14:07) returns 0x7FFFFFFF.  */
> +  test_mktime_helper (&tmY2038, 0x7fffffff);
> +
> +  /* Check that mktime(2038-01-19 03:14:08) returns 0x80000000
> +     (time_t overflow).  */
> +  if (sizeof (time_t) > 4)
> +    {
> +      tmY2038.tm_sec++;
> +      test_mktime_helper (&tmY2038, 0x80000000);
> +    }
> +  else
> +    FAIL_UNSUPPORTED ("32-bit time_t");

FAIL_UNSUPPORTED causes the whole test to report an UNSUPPORTED status.  
If a test has a mixture of assertions that work with 32-bit time and 
assertions that only work for 64-bit time, I think those should be split 
into separate tests so that those tests that make sense with 32-bit time 
still produce results in that case.
  
Paul Eggert Jan. 25, 2021, 7:22 p.m. UTC | #5
On 1/25/21 5:03 AM, Lukasz Majewski wrote:

> +  /* Set time zone for the test.  */
> +  TEST_VERIFY_EXIT (setenv ("TZ", "Europe/Warsaw", 1) == 0);
> +  tzset ();

Your test data assume UTC, so why set the time zone to Warsaw's?

Also, there's no need to call tzset; mktime is supposed to do that.

I suggest also testing 2**32 - 1 and 2**32, when using 64-bit time_t.
  
Florian Weimer Jan. 25, 2021, 8:01 p.m. UTC | #6
* Paul Eggert:

> On 1/25/21 5:03 AM, Lukasz Majewski wrote:
>
>> +  /* Set time zone for the test.  */
>> +  TEST_VERIFY_EXIT (setenv ("TZ", "Europe/Warsaw", 1) == 0);
>> +  tzset ();
>
> Your test data assume UTC, so why set the time zone to Warsaw's?
>
> Also, there's no need to call tzset; mktime is supposed to do that.
>
> I suggest also testing 2**32 - 1 and 2**32, when using 64-bit time_t.

Furthermore, if the test can't use UTC for some reason, please add a
custom time zone for it, so that it does not depend on real tzdata files
for future dates, which can change.

Thanks,
Florian
  
Lukasz Majewski Jan. 25, 2021, 9:31 p.m. UTC | #7
Hi Joseph,

> On Mon, 25 Jan 2021, Lukasz Majewski wrote:
> 
> > +  /* Check that mktime(1970-01-01 00:00:00) returns 0.  */
> > +  test_mktime_helper (&tm0, 0);
> > +
> > +  /* Check that mktime(2038-01-19 03:14:07) returns 0x7FFFFFFF.  */
> > +  test_mktime_helper (&tmY2038, 0x7fffffff);
> > +
> > +  /* Check that mktime(2038-01-19 03:14:08) returns 0x80000000
> > +     (time_t overflow).  */
> > +  if (sizeof (time_t) > 4)
> > +    {
> > +      tmY2038.tm_sec++;
> > +      test_mktime_helper (&tmY2038, 0x80000000);
> > +    }
> > +  else
> > +    FAIL_UNSUPPORTED ("32-bit time_t");  
> 
> FAIL_UNSUPPORTED causes the whole test to report an UNSUPPORTED
> status. If a test has a mixture of assertions that work with 32-bit
> time and assertions that only work for 64-bit time, I think those
> should be split into separate tests so that those tests that make
> sense with 32-bit time still produce results in that case.
> 

The idea behind this FAIL_UNSUPPORTED when sizeof(time_t) <= 4 is to:

1. On 32 bit ports - check correct operation of mktime for 32 bit
time_t. If those checks fail, then we fail the test. Tests which require
64 bit size of time_t will not be supported until -D_TIME_BITS=64 is
supported in glibc.

2. On 64 bit ports - the test will either end without errors or fail.

3. I do have corresponding test - tst-mktime4-y2038.c [1] which only has
following lines:

#define _TIME_BITS 64
#define _FILE_OFFSET_BITS 64

#include "tst-mktime4.c"

Which will either pass or fail when 64 bit time_t support is added on
ports with __WORDSIZE=32 && __TIMESIZE != 64.

However, there is no point to send tst-mktime4-y2038.c until the
above feature is supported.


Links:
[1] - https://github.com/lmajewski/y2038_glibc/commits/y2038_edge


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
  
Joseph Myers Jan. 25, 2021, 10:26 p.m. UTC | #8
On Mon, 25 Jan 2021, Lukasz Majewski wrote:

> 1. On 32 bit ports - check correct operation of mktime for 32 bit
> time_t. If those checks fail, then we fail the test.

But if those checks pass, the test will end with UNSUPPORTED rather than 
PASS.  If there is anything useful to verify on 32-bit ports, the test 
should end with PASS if that verification succeeds and FAIL if that 
verification fails - not with a choice of UNSUPPORTED and FAIL.
  
Lukasz Majewski Jan. 25, 2021, 10:28 p.m. UTC | #9
Hi Florian,

> * Paul Eggert:
> 
> > On 1/25/21 5:03 AM, Lukasz Majewski wrote:
> >  
> >> +  /* Set time zone for the test.  */
> >> +  TEST_VERIFY_EXIT (setenv ("TZ", "Europe/Warsaw", 1) == 0);
> >> +  tzset ();  
> >
> > Your test data assume UTC, so why set the time zone to Warsaw's?

I thought that I do need to explicitly set TZ here (either "TZ=UTC" or
"TZ=Europe/Warsaw").

When I poke on the manuals (like [1]) - it is all written that by
default (when TZ is not set) the UTC is used.

Hence, my question - why cannot I just check if TZ is set and if not
set it to default "TZ=UTC" ? 
In that way I would have reproductible setup no matter if I run this
test on HOST or TARGET (VM) machines.

Another question - why in ./time/tst-y2039.c the 
setenv ("TZ", "PST8PDT,M3.2.0,M11.1.0", 1) is set?
According to [2] the PST8PDT is deprecated. Why the UTC cannot be just
used?

> >
> > Also, there's no need to call tzset; mktime is supposed to do that.

Yes. Correct __tzset() is called in __mktime64() implementation.

> >
> > I suggest also testing 2**32 - 1 and 2**32, when using 64-bit
> > time_t.

Ok.

> 
> Furthermore, if the test can't use UTC for some reason, please add a
> custom time zone for it, so that it does not depend on real tzdata
> files for future dates, which can change.

Sorry, but I'm confused - how shall I evaluate if I cannot use UTC for
some reason? As fair as I can tell the UTC is a default fallback (and
is assumed) for several time related calls [2].

Could you point me to some test case, which I could use as an example?

For example in ./time/tst-y2039.c the TZ is set to some arbitrary value
to have some non UTC (i.e. default) TZ, so the test will be more
reliable (detached from default TZ).

Is there any issue with using "TZ=UTC" for Y2038 related tests?

Links:
[1] - https://linux.die.net/man/3/mktime
[2] - https://en.wikipedia.org/wiki/List_of_tz_database_time_zones

> 
> Thanks,
> Florian




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
  
Lukasz Majewski Jan. 25, 2021, 10:51 p.m. UTC | #10
Hi Joseph,

> On Mon, 25 Jan 2021, Lukasz Majewski wrote:
> 
> > 1. On 32 bit ports - check correct operation of mktime for 32 bit
> > time_t. If those checks fail, then we fail the test.  
> 
> But if those checks pass, the test will end with UNSUPPORTED rather
> than PASS.  If there is anything useful to verify on 32-bit ports,
> the test should end with PASS if that verification succeeds and FAIL
> if that verification fails - not with a choice of UNSUPPORTED and
> FAIL.
> 

Please correct me if I'm wrong.
Then the mktime4 test shall be as follows:

1. tst-mktime4-32time.c
/* mktime tests which can be safely run with 32 bit time_t */
test_mktime_helper (&tm0, 0);
test_mktime_helper (&tmY2038, 0x7fffffff);


2. tst-mktime4-64time.c

if (sizeof (time_t) > 4)
{

Reuse code from tst-mktime4-32time.c
and add extra:
test_mktime_helper (&tmY2038, 0x80000000);

} else
FAIL_UNSUPPORTED ("32-bit time_t");

3. tst-mktime4-y2038.c

#define _TIME_BITS 64
#define _FILE_OFFSET_BITS 64

#include "tst-mktime4-64time.c"

When we got -D_TIME_BITS=64 support in glibc accepted.


Above use cases shall cover all use cases.

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
  
Paul Eggert Jan. 25, 2021, 11:41 p.m. UTC | #11
On 1/25/21 2:28 PM, Lukasz Majewski wrote:

> I thought that I do need to explicitly set TZ here (either "TZ=UTC" or
> "TZ=Europe/Warsaw").

You do if you want reproducible results with mktime, yes. For glibc, 
perhaps the GNU extension "TZ=:" is the best, as it's the only way in 
glibc to guarantee that you're using UTC without leap seconds. (This 
whole area is a bit messy and undocumented in glibc, unfortunately.)

If the glibc test environment is invariably set up without leap-second 
support (is it? I don't know) you needn't worry about leap seconds and 
so can use the POSIX-standard "TZ=UTC0". The trailing "0" is required.

> When I poke on the manuals (like [1]) - it is all written that by
> default (when TZ is not set) the UTC is used.

I don't see where [1] says that. Anyway, that's incorrect; if TZ is 
unset, a system-specific default is used.

> Hence, my question - why cannot I just check if TZ is set and if not
> set it to default "TZ=UTC" ?

"TZ=UTC" assumes tzdb is installed and that leap seconds are not in use. 
Are these assumptions for testing glibc? And an unset TZ means use the 
system-supplied default; can we assume that's UTC when testing glibc?

> Another question - why in ./time/tst-y2039.c the
> setenv ("TZ", "PST8PDT,M3.2.0,M11.1.0", 1) is set?
> According to [2] the PST8PDT is deprecated. Why the UTC cannot be just
> used?

setenv ("TZ", "PST8PDT", 1) is deprecated but setenv ("TZ", 
"PST8PDT,M3.2.0,M11.1.0", 1) is not; it's standardized by POSIX. See:

https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_03

and look for "TZ".

Also, I would not rely on Wikipedia for this stuff; it has too many 
errors. I don't edit Wikipedia and probably wouldn't have time to fix 
its timezone problems even if I did.

> For example in ./time/tst-y2039.c the TZ is set to some arbitrary value
> to have some non UTC (i.e. default) TZ, so the test will be more
> reliable (detached from default TZ).

If memory serves, that particular bug occurred only in non-UTC 
environments, which is why that test had to use something other than UTC.

> [1] - https://linux.die.net/man/3/mktime
> [2] - https://en.wikipedia.org/wiki/List_of_tz_database_time_zones
  

Patch

diff --git a/time/Makefile b/time/Makefile
index 7de2ce0196..f214d001f5 100644
--- a/time/Makefile
+++ b/time/Makefile
@@ -51,7 +51,7 @@  tests	:= test_time clocktest tst-posixtz tst-strptime tst_wcsftime \
 	   tst-clock tst-clock2 tst-clock_nanosleep tst-cpuclock1 \
 	   tst-adjtime tst-clock-y2038 tst-clock2-y2038 \
 	   tst-cpuclock1-y2038 tst-clock_nanosleep-y2038 tst-clock_settime \
-	   tst-clock_adjtime tst-ctime tst-difftime
+	   tst-clock_adjtime tst-ctime tst-difftime tst-mktime4
 
 include ../Rules
 
diff --git a/time/tst-mktime4.c b/time/tst-mktime4.c
new file mode 100644
index 0000000000..468ca24174
--- /dev/null
+++ b/time/tst-mktime4.c
@@ -0,0 +1,82 @@ 
+/* Test for mktime
+   Copyright (C) 2021 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <time.h>
+#include <stdlib.h>
+#include <support/check.h>
+
+struct tm tm0 = {
+                 .tm_year = 70,
+                 .tm_mon = 0,
+                 .tm_mday = 1,
+                 .tm_hour = 0,
+                 .tm_min = 0,
+                 .tm_sec = 0,
+                 .tm_wday = 4,
+                 .tm_yday = 0
+};
+
+struct tm tmY2038 = {
+                     .tm_year = 138,
+                     .tm_mon = 0,
+                     .tm_mday = 19,
+                     .tm_hour = 3,
+                     .tm_min = 14,
+                     .tm_sec = 7,
+                     .tm_wday = 2,
+                     .tm_yday = 18
+};
+
+static
+void test_mktime_helper (struct tm *tm, time_t exp_val)
+{
+  time_t result = mktime (tm);
+  if (result == -1)
+    FAIL_EXIT1 ("*** mktime failed: %m");
+
+  if (result != exp_val)
+    FAIL_EXIT1 ("*** mktime returned %ld (expected %ld)\n", result, exp_val);
+}
+
+static int
+do_test (void)
+{
+  /* Set time zone for the test.  */
+  TEST_VERIFY_EXIT (setenv ("TZ", "Europe/Warsaw", 1) == 0);
+  tzset ();
+
+  /* Check that mktime(1970-01-01 00:00:00) returns 0.  */
+  test_mktime_helper (&tm0, 0);
+
+  /* Check that mktime(2038-01-19 03:14:07) returns 0x7FFFFFFF.  */
+  test_mktime_helper (&tmY2038, 0x7fffffff);
+
+  /* Check that mktime(2038-01-19 03:14:08) returns 0x80000000
+     (time_t overflow).  */
+  if (sizeof (time_t) > 4)
+    {
+      tmY2038.tm_sec++;
+      test_mktime_helper (&tmY2038, 0x80000000);
+    }
+  else
+    FAIL_UNSUPPORTED ("32-bit time_t");
+
+  return 0;
+}
+
+#include <support/test-driver.c>