From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 250993858C2C for ; Wed, 13 Apr 2022 20:33:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 250993858C2C Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1649882014; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=MEekPp/ErWvDG+zkg3fb6RcmuYCpi8tcjH/p/5q9KZo=; b=Bnvcu4Imkv1wZK6+3MrV1wUvDutnRUpttpDI5HPEoaOjDCELjxVo2V0zF9Mg5OAv7o0G7t kmPQ3gABBOhr6uqTyItOLvX8+WQRZjgBe4yZEbI3s6JSg8Ot3++Cm+QFmaT2u2Ej3BPkPl kaz6Wtu7R0LcpKsxjy6KoDLDx+v9+FQ= Received: from mail-yb1-f197.google.com (mail-yb1-f197.google.com [209.85.219.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-327-f8NuC0D-N_qqc628OKnksg-1; Wed, 13 Apr 2022 16:33:31 -0400 X-MC-Unique: f8NuC0D-N_qqc628OKnksg-1 Received: by mail-yb1-f197.google.com with SMTP id h82-20020a25d055000000b00641d2fd5f3fso2270511ybg.11 for ; Wed, 13 Apr 2022 13:33:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=MEekPp/ErWvDG+zkg3fb6RcmuYCpi8tcjH/p/5q9KZo=; b=wohY2rpprMqqLN1shEi1ZlpPiVRKtYhUVDF0Ve2TkU1ZdoERJGHBvaNqG8nSENNGR/ Tjai1vDv6+KdankDXMv17yUvNksSkX6D1rJuNSqVUI3DRJNAejXvxcr0rs28mMadrwAE u/y9QKqhEXlj+sNpxAkgolZl0Kz9Bl1yKkXRbdFut0EU7i/AT/rfKcprt3f6noSDPNNC HeEqV98ljicoO/IA0+VzfyhyBYwUJd6le8/Nw5uMQDdmISGkb5xA+8QXmAQqpfWhk3XM +Apv70Eyos0KcG8mYQ/bVpp61nP6k8IopSuzXefMIBJlcyCZwjCM1ATxOxtyrkBz6JLp eS4g== X-Gm-Message-State: AOAM5332UMThPzshPt8ULCCiw83/whKnfkbp2OK3mhaQJIq+5Wb0+ewy d7ZlR/MkAXfMu3dbVJueD5y8dzNnhr9IgnWD0BI3sRCfKM5PJ+/++K7BH7x+4H6hC1zOgv9IT1j 2RGd3BIUqkRJk2fEEQpDOT/dlJoMjNDI= X-Received: by 2002:a81:7757:0:b0:2ec:3671:86a0 with SMTP id s84-20020a817757000000b002ec367186a0mr616697ywc.165.1649882010874; Wed, 13 Apr 2022 13:33:30 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwXHksBBAYgGvlRcL4DOmj7bZDCuzoz7MlrMMc8MQYpiIrH9QQyX4JtOxCjV0g0B6pgGjPX2SleV5ez/wNW8sw= X-Received: by 2002:a81:7757:0:b0:2ec:3671:86a0 with SMTP id s84-20020a817757000000b002ec367186a0mr616681ywc.165.1649882010634; Wed, 13 Apr 2022 13:33:30 -0700 (PDT) MIME-Version: 1.0 References: <822e81a0-ed9f-200e-3318-0495456ad67e@SystematicSw.ab.ca> <20220407233425.2012-1-Brian.Inglis@SystematicSW.ab.ca> <9603e3aa-bd7d-6740-c710-27ace1d80397@SystematicSw.ab.ca> In-Reply-To: <9603e3aa-bd7d-6740-c710-27ace1d80397@SystematicSw.ab.ca> From: Jeff Johnston Date: Wed, 13 Apr 2022 16:33:19 -0400 Message-ID: Subject: Re: [PATCH] add tests for tzset(3) To: Newlib Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=jjohnstn@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-4.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, HTML_MESSAGE, KAM_SHORT, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: newlib@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Newlib mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 13 Apr 2022 20:33:37 -0000 Looking at the glibc tzset code I have locally (not latest/greatest, but does support angle brackets): If there any parse failures, UTC is defaulted. Extraneous characters inside brackets or less than 3 characters is a parse failure. Glibc parses the tz name string char by char and allocates space for the name strings so there is no max size. I am fine if you want to mandate a maximum, but if you do, then too many chars should be treated as a failure. If you aren't certain of the limit, make the limit higher than you expect. If people run into max limit with reasonable timezone format strings, then we can up the limit. -- Jeff J. On Wed, Apr 13, 2022 at 1:53 PM Brian Inglis < Brian.Inglis@systematicsw.ab.ca> wrote: > On 2022-04-12 12:33, Brian Inglis wrote: > > On 2022-04-12 05:19, jdoubleu wrote: > >> On 4/11/2022 7:27 PM, Dimitar Dimitrov wrote: > >>> On Mon, Apr 11, 2022 at 01:17:16PM +0200, jdoubleu wrote: > >>>> looks like I'm running the testsuite against glibc and not newlib > >>>> (for target x86_64-pc-linux-gnu). I'm not even sure whether > >>>> there's a backend for linux. > >>>> I'm currently trying to run only the tzset code against the test > >>>> vectors (like Brian Inglis did[1]). > >>>> At least it show, that the newlib implementation differs from glibc. > >>>> Maybe the test case is flawed and it should indeed fail. > > > >>>>> 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. > > > >>>> Okay. Looks like the condition[2] fails. The question is, which > >>>> part of it does? > >>> I believe it is the ('>' != tzenv[n]) check that fails, because the > >>> maximum parsed string limit of 10 has been reached and n=10. > > > >>>> I've appended a patch, which prints all variables when the condition > >>>> fails. > >>>> Could you please apply the patch and then recompile and re-run the > >>>> tests > >>>> again? > > > >>> Here is the result for arm-none-eabi: > >>> parsing name: tzenv="+0123456789ABCDEF>3:33:33", res=1, n=10, > >>> tzenv[n] = 9 > >>> Assertion failed! Expected 1647906533 to equal 1647916532. winter > >>> time, timezone = "<+0123456789ABCDEF>3:33:33" > >>> I assume you no longer need assembly output from compiler? > > > >>>> I've previously noticed something with the sscanf format[3]. > >>>>> Perhaps TZ should be reset to UTC before the bail out? > > > >>>> I don't think the implementation should fall back to UTC > >>>> whenever parsing failed. It apparently doesn't in glibc. I'm not > >>>> sure if the behavior is specified somewhere. > > > >>> The tzset manual page says that UTC is used if TZ cannot be parsed: > >>> https://man7.org/linux/man-pages/man3/tzset.3.html > > > >>>> Maybe resetting it before each test case is a good idea, though. > >>>> That makes it clearer, why the test case failed. > >>>>> 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" > > > >>>> My bad; "3:33:33" isn't a valid timezone string. It has to be > >>>> prefixed by a > >>>> name e.g. "MESZ" or "<+00>", as you tried. That explains why it is > also > >>>> failing. > > > >>>> [1]: https://sourceware.org/pipermail/newlib/2022/019529.html > >>>> [2]: > >>>> > https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=newlib/libc/time/tzset_r.c;h=9cb30b188f989f65ec9eb6417f5d74020f8c72e9;hb=HEAD#l57 > >>>> > >>>> [3]: https://sourceware.org/pipermail/newlib/2022/019535.html > > > >>> I believe it is the ('>' != tzenv[n]) check that fails, because the > >>> maximum parsed string limit of 10 has been reached and n=10. > > > >> Okay, so the newlib implementation actually fails, when the name is > >> too long. > >> The POSIX standard leaves it up to the implementation how to handle > >> names[1]: > > > >>> The interpretation of these fields is unspecified if either field is > >>> less than three bytes (except for the case when dst is missing), more > >>> than {TZNAME_MAX} bytes, or if they contain characters other than > >>> those specified. > > > >> I'm not sure where to go with this. In both cases, the current > >> implementation needs a fix. Looping in Brian Inglis, the writer of the > >> current implementation and Jeff Johnston, a maintainer. > >> As I see it, there are two solutions: > >> 1. Treat long names as an error case. The failed condition[2] needs to > >> apply UTC as current time, like Dimitar Dimitrov previously suggested. > >> The test needs to be changed to check that it actually fails. > >> 2. Mimic glibc's behavior and ignore the remaining characters from the > >> name. In that case, the condition[2] and following code needs to be > >> updated as well. > >> I am happy to provide a patch, when we agreed on one solution. > > > >>> I assume you no longer need assembly output from compiler? > > > >> No, thanks. We already found the issue. It would only be helpful, if > >> different configurations generate different code (through macros, > >> optimization, etc.). > >> [1]: > https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_03 > >> [2]: > https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=newlib/libc/time/tzset_r.c;hb=HEAD#l57 > > > Does anyone know what glibc or the BSDs do on short < 3 char > > abbreviations, or the BSDs do on > LIMIT abbreviations? > > What do they do on errors - nothing or revert to UTC? > > > > Should tzset_r allow 1...2 char abbreviations or treat them as errors? > > > > Should tzset_r allow > 10 char abbreviations, ignore and skip extra > > chars, or treat them as errors? > > Should tzset_r accept characters within quotes not in the allowed > character set, ignore and skip them, or reject such abbreviations? > > > If it ignores and skips extra or accepts too few characters, then > > presumably it should process the remaining fields if present? > > > > POSIX tzset_r says only that it overrides the default time zone, so is > > there a spec for the default time zone, or is it implementation defined > > (e.g. /etc/timezone /etc/localtime)? > > > > Should tzset_r revert to UTC on errors, or just not change the TZ, and > > leave it up to the application to handle the error? > > -- > Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada > > This email may be disturbing to some readers as it contains > too much technical detail. Reader discretion is advised. > [Data in binary units and prefixes, physical quantities in SI.] > >