public inbox for newlib@sourceware.org
 help / color / mirror / Atom feed
From: Dimitar Dimitrov <dimitar@dinux.eu>
To: jdoubleu <hi@jdoubleu.de>
Cc: newlib@sourceware.org
Subject: Re: [PATCH] add tests for tzset(3)
Date: Mon, 11 Apr 2022 00:00:23 +0300	[thread overview]
Message-ID: <YlNFZ+JY0OxY+6d3@kendros> (raw)
In-Reply-To: <c8ca181b-294b-2cc0-8b0f-5756ddd99e88@jdoubleu.de>

On Sun, Apr 10, 2022 at 07:55:56PM +0200, jdoubleu wrote:
> Hi Dimitar,
> 
> On 4/10/2022 10:43 AM, Dimitar Dimitrov wrote:
> > I get the following failure on pru-none-elf and arm-none-eabi targets:
> 
> that is interesting! I just checked and they are still passing on
> x86_64-pc-linux-gnu. Which begs the question: What is the difference to your
> environment/target platform?

Here is how I build a cross toolchain for arm:
  https://github.com/dinuxbg/gnupru/blob/master/testing/manual-build-arm.sh
Then I build and execute the test case using qemu:
  $ ./arm-opt/bin/arm-none-eabi-gcc -O2 newlib/newlib/testsuite/newlib.time/tzset.c -o tzset-arm.elf
  $ qemu-arm tzset-arm.elf
Assertion failed! Expected 1647906533 to equal 1647916532. winter time, timezone = "3:33:33"

For PRU target I'm running a daily regression test for Newlib, using the
GNU simulator:
  https://github.com/dinuxbg/gnupru/blob/master/testing/buildbot-pru.sh

> 
> Are you using any additional compilation flags? Have you enabled/disabled
> optimizations? Have you enabled the nano I/O API[1]?
> What compiler (and version) are you using? How/Where are you running the
> tests?

These are the newlib configure options I use for arm:
   --target=arm-none-eabi --enable-newlib-io-long-long --enable-newlib-io-long-double --enable-newlib-io-c99-formats

I use GCC 12 from GIT master branch for both PRU and ARM cross
toolchains.

> 
> > [..] timezone = "<+0123456789ABCDEF>3:33:33"
> 
> The given test case aims to check for tzset's parsing behavior when the name
> (inside the angle brackets) exceeds the TZNAME_MAX var, i.e. is too long to
> be fully stored. The default behavior is to truncate the name and ignore all
> remaining chars[2]. The time should still be parsed.
> 
> > Assertion failed! Expected 1647906533 to equal 1647916532.
> 
> Now, the interesting part is, that the diff is 9,999 seconds. The time is
> off by 6 hours, 20 minutes and 12 seconds. That is 2 hours, 46 minutes and
> 38 seconds too far in the future. I am not sure where this is coming from.

6:20:12 is the timezone string of the previous test case, whose tzset
call was successful. Looking at the current code, this is expected
behaviour. Perhaps TZ should be reset to UTC before the bail out?

      /* quit if no items, too few or too many chars, or no close quote '>' */
      if (sscanf (tzenv, "%10[-+0-9A-Za-z]%n", __tzname_std, &n) <= 0
                || n < TZNAME_MIN || TZNAME_MAX < n || '>' != tzenv[n])
      {
+       _timezone = 0;
+       _daylight = 0;
+       _tzname[0] = "GMT";
+       _tzname[1] = "GMT";
+       free(prev_tzenv);
+       prev_tzenv = NULL;
        return;
      }

> 
> Neither does it look like it bailed early[3], because it couldn't parse the
> name.
> 
> Please try and remove the "<+0123456789ABCDEF>" part from the string in the
> tests[4] and re-run the tests.

With that chunk removed, as shown below:
    {"3:33:33",               IN_SECONDS(3, 33, 33),   NO_TIME},                 // truncates the name (17 + 1)
I still get:
  Assertion failed! Expected 1647906533 to equal 1647916532. winter time, timezone = "3:33:33"

Test passes with the following modifcation:
     {"<+00>3:33:33",               IN_SECONDS(3, 33, 33),   NO_TIME},                 // truncates the name (17 + 1)

> 
> Could you maybe also post the assembly listing and intermediate output of
> the compiler? If you're using gcc, just (re)configure newlib and rebuild
> libc:
> 
> 	CFLAGS=-save-temps=obj ../newlib/configure
> 	make clean # get rid of stamp files
> 	make
> 
> then copy and attach the following files:
> 	
> 	libc/time/libc_a-tzset_r.i
> 	libc/time/libc_a-tzset_r.s

Sure. Give me a day or two to spin the builds.

> 
> 
> [1]: https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=newlib/README;h=97890b9d20ca0ac9f206d7a2334d55a3d7052289;hb=HEAD#l403
> [2]: https://sourceware.org/pipermail/newlib/2022/019535.html
> [3]: https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=newlib/libc/time/tzset_r.c;h=9cb30b188f989f65ec9eb6417f5d74020f8c72e9;hb=HEAD#l57
> [4]: https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=newlib/testsuite/newlib.time/tzset.c;h=0e5b196c629bcbecae6148c09bbeba82079c5367;hb=HEAD#l96
> 
> 
> Cheers
> ---
> 🙎🏻‍♂️ jdoubleu

  reply	other threads:[~2022-04-10 21:00 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-07 15:58 jdoubleu
2022-04-08 21:21 ` Jeff Johnston
2022-04-10  8:43 ` Dimitar Dimitrov
2022-04-10 17:55   ` jdoubleu
2022-04-10 21:00     ` Dimitar Dimitrov [this message]
2022-04-11 11:17       ` jdoubleu
2022-04-11 17:27         ` Dimitar Dimitrov
2022-04-12 11:19           ` jdoubleu
2022-04-12 18:33             ` Brian Inglis
2022-04-07 23:34               ` [PATCH v2 0/2] add tzset/_r POSIX angle bracket <> support in TZ env var Brian Inglis
2022-04-07 23:34                 ` [PATCH v2 1/2] newlib/libc/time/tzset.c: doc update POSIX angle bracket <> support Brian Inglis
2022-04-07 23:34                 ` [PATCH v2 2/2] newlib/libc/time/tzset_r.c(_tzset_unlocked_r): " Brian Inglis
2022-04-08 19:11                 ` [PATCH v2 0/2] add tzset/_r POSIX angle bracket <> support in TZ env var Jeff Johnston
2022-04-13 17:53                 ` [PATCH] add tests for tzset(3) Brian Inglis
2022-04-13 20:33                   ` Jeff Johnston
2022-04-13 22:19                     ` Brian Inglis
2022-04-14  8:59                       ` jdoubleu
2022-04-14 16:31                         ` Brian Inglis
2022-04-14 19:23                           ` Jeff Johnston
2022-04-15 10:10                             ` jdoubleu
2022-04-27 19:30                               ` Jeff Johnston
2022-05-14 14:39                         ` jdoubleu
2022-05-16 16:05                           ` Dimitar Dimitrov
2022-05-16 17:38                             ` Jeff Johnston
2022-05-17  8:45                           ` [PATCH] update tzset tests jdoubleu
2022-05-18 18:48                             ` Dimitar Dimitrov
2022-05-18 20:56                               ` Keith Packard
2022-05-19  8:47                                 ` jdoubleu
2022-05-22  9:51                                   ` [PATCH v2] " jdoubleu
2022-05-22 21:02                                     ` Dimitar Dimitrov
2022-05-27 15:46                                       ` Jeff Johnston
2022-04-13 22:21               ` [PATCH] add tests for tzset(3) Brian Inglis
2022-04-29 15:46 Jeff Johnston
2022-05-12 18:35 ` jdoubleu
2022-05-16 17:49   ` Jeff Johnston

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YlNFZ+JY0OxY+6d3@kendros \
    --to=dimitar@dinux.eu \
    --cc=hi@jdoubleu.de \
    --cc=newlib@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).