[Bug time/23789] New: mktime does not set errno on failure

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[Bug time/23789] New: mktime does not set errno on failure

cvs-commit at gcc dot gnu.org
https://sourceware.org/bugzilla/show_bug.cgi?id=23789

            Bug ID: 23789
           Summary: mktime does not set errno on failure
           Product: glibc
           Version: 2.28
            Status: NEW
          Severity: normal
          Priority: P2
         Component: time
          Assignee: unassigned at sourceware dot org
          Reporter: eggert at cs dot ucla.edu
  Target Milestone: ---

Created attachment 11343
  --> https://sourceware.org/bugzilla/attachment.cgi?id=11343&action=edit
test program that fails on 32-bit glibc

A while ago, POSIX added a requirement to 'mktime' that it must set errno on
failure. Probably this was around 2001. However, glibc mktime never got updated
to set errno and this should be fixed. I'll attach a program to illustrate the
bug. Compile and run it with 'gcc -m32 mktime-test.c'. It should exit with
status 0, but exits with status 2 because mktime does not set errno when it
fails.

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug time/23789] mktime does not set errno on failure

cvs-commit at gcc dot gnu.org
https://sourceware.org/bugzilla/show_bug.cgi?id=23789

--- Comment #1 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, aaribaud/bugzilla/23789/v2 has been created
        at  821e26d45f35922dd00e8fc604ef3fe9f3a654ab (commit)

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=821e26d45f35922dd00e8fc604ef3fe9f3a654ab

commit 821e26d45f35922dd00e8fc604ef3fe9f3a654ab
Author: Albert ARIBAUD (3ADEV) <[hidden email]>
Date:   Wed Oct 24 14:43:06 2018 +0200

    Ensure mktime sets errno on error (bug 23789)

    Posix mandates that mktime set errno to EOVERFLOW
    on error, that it, upon retuning -1, but the glibc
    mktime does not so far on 32-bit architectures.

    Fix this and add a test to prevent regressions.

    The test was run through 'make check' on i686-linux-gnu,
    then the fix was added and 'make check' run again.

        * time/mktime.c
        (mktime): Set errno to EOVERFLOW on error.
        * time/bug-mktime4.c: New file.
            * time/Makefile: Add bug-mktime4.

-----------------------------------------------------------------------

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug time/23789] mktime does not set errno on failure

cvs-commit at gcc dot gnu.org
In reply to this post by cvs-commit at gcc dot gnu.org
https://sourceware.org/bugzilla/show_bug.cgi?id=23789

--- Comment #2 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, aaribaud/bugzilla/23789/v2 has been deleted
       was  821e26d45f35922dd00e8fc604ef3fe9f3a654ab

- Log -----------------------------------------------------------------
821e26d45f35922dd00e8fc604ef3fe9f3a654ab Ensure mktime sets errno on error (bug
23789)
-----------------------------------------------------------------------

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug time/23789] mktime does not set errno on failure

cvs-commit at gcc dot gnu.org
In reply to this post by cvs-commit at gcc dot gnu.org
https://sourceware.org/bugzilla/show_bug.cgi?id=23789

--- Comment #3 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, aaribaud/bugzilla/23789/v2 has been created
        at  91ac988a6b8662cd1e13770cdc550c2609c42178 (commit)

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=91ac988a6b8662cd1e13770cdc550c2609c42178

commit 91ac988a6b8662cd1e13770cdc550c2609c42178
Author: Albert ARIBAUD (3ADEV) <[hidden email]>
Date:   Wed Oct 24 14:43:06 2018 +0200

    Ensure mktime sets errno on error (bug 23789)

    Posix mandates that mktime set errno to EOVERFLOW
    on error, that it, upon retuning -1, but the glibc
    mktime does not so far on 32-bit architectures.

    Fix this and add a test to prevent regressions.

    The test was run through 'make check' on i686-linux-gnu,
    then the fix was added and 'make check' run again.

        * time/mktime.c
        (mktime): Set errno to EOVERFLOW on error.
        * time/bug-mktime4.c: New file.
            * time/Makefile: Add bug-mktime4.

-----------------------------------------------------------------------

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug time/23789] mktime does not set errno on failure

cvs-commit at gcc dot gnu.org
In reply to this post by cvs-commit at gcc dot gnu.org
https://sourceware.org/bugzilla/show_bug.cgi?id=23789

--- Comment #4 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, aaribaud/bugzilla/23789/v4 has been created
        at  134ae5ca82d3c5b5b9bab0ad145ffe8b1919a1ae (commit)

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=134ae5ca82d3c5b5b9bab0ad145ffe8b1919a1ae

commit 134ae5ca82d3c5b5b9bab0ad145ffe8b1919a1ae
Author: Albert ARIBAUD (3ADEV) <[hidden email]>
Date:   Wed Oct 24 14:43:06 2018 +0200

    Ensure mktime sets errno on error [BZ  #23789]

    Posix mandates that mktime set errno to EOVERFLOW
    on error, but the glibc mktime wasn't doing it so
    far.

    Fix this and add a test to prevent regressions.
    The fix also fixes the same issue in timegm.

    Tested with 'make check' on x86-linux-gnu and
    i686-linux-gnu.

            * time/Makefile: Add bug-mktime4.
        * time/bug-mktime4.c: New file.
        * time/mktime.c
        (__mktime_internal): Set errno to EOVERFLOW on error.
        (mktime): Move call to __tzset inside conditional.

-----------------------------------------------------------------------

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug time/23789] mktime does not set errno on failure

cvs-commit at gcc dot gnu.org
In reply to this post by cvs-commit at gcc dot gnu.org
https://sourceware.org/bugzilla/show_bug.cgi?id=23789

--- Comment #5 from eggert at cs dot ucla.edu ---
Created attachment 11377
  --> https://sourceware.org/bugzilla/attachment.cgi?id=11377&action=edit
proposed simpler patch

As discussed in https://www.sourceware.org/ml/libc-alpha/2018-11/msg00062.html
and https://www.sourceware.org/ml/libc-alpha/2018-11/msg00068.html this is a
simpler patch.

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug time/23789] mktime does not set errno on failure

cvs-commit at gcc dot gnu.org
In reply to this post by cvs-commit at gcc dot gnu.org
https://sourceware.org/bugzilla/show_bug.cgi?id=23789

--- Comment #6 from eggert at cs dot ucla.edu ---
Created attachment 11378
  --> https://sourceware.org/bugzilla/attachment.cgi?id=11378&action=edit
proposed test case

As discussed in https://www.sourceware.org/ml/libc-alpha/2018-11/msg00068.html
here is a proposed test case.

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug time/23789] mktime does not set errno on failure

cvs-commit at gcc dot gnu.org
In reply to this post by cvs-commit at gcc dot gnu.org
https://sourceware.org/bugzilla/show_bug.cgi?id=23789

Albert ARIBAUD <albert.aribaud at 3adev dot fr> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |albert.aribaud at 3adev dot fr
         Resolution|---                         |FIXED
   Target Milestone|---                         |2.29

--- Comment #7 from Albert ARIBAUD <albert.aribaud at 3adev dot fr> ---
Fixed by the following 7-commit series:

commit 5d8af1566bd98aa9c2728f8f63f7ba43cfa84d09
Author: Paul Eggert <[hidden email]>
Date:   Thu Nov 15 22:59:33 2018 +0100

    mktime: DEBUG_MKTIME cleanup

    The DEBUG_MKTIME code no longer works in glibc or in Gnulib.
    And it’s no longer needed now that glibc and Gnulib both have
    their own testing mechanisms for mktime.
    * time/mktime.c (DEBUG_MKTIME): Remove.  All uses removed.

commit 86aece3bfbd44538ba4fdc947872c81d4c5e6e61
Author: Paul Eggert <[hidden email]>
Date:   Thu Nov 15 22:59:33 2018 +0100

    mktime: fix non-EOVERFLOW errno handling

    [BZ#23789]
    mktime was not properly reporting failures when the underlying
    localtime_r fails with errno != EOVERFLOW; it incorrectly treated
    them like EOVERFLOW failures, and set errno to EOVERFLOW.
    The problem could happen on non-glibc platforms, with Gnulib.
    * time/mktime.c (guess_time_tm): Remove, replacing with ...
    (tm_diff): ... this simpler function, which does not change errno.
    All callers changed to deal with errno themselves.
    (ranged_convert, __mktime_internal): Return failure immediately if
    the underlying function reports any failure other than EOVERFLOW.
    (__mktime_internal): Set errno to EOVERFLOW if the spring-forward
    gap code fails.

commit f6b3331bbae638d1bb50813fceb429d3b3dc0eb9
Author: Paul Eggert <[hidden email]>
Date:   Thu Nov 15 22:59:33 2018 +0100

    mktime: fix bug with Y2038 DST transition

    [BZ#23789]
    * time/mktime.c (ranged_convert): On 32-bit platforms, don’t
    mishandle a DST transition that jumps over the Y2038 boundary.
    No such DST transitions are known so this is only a theoretical
    bug, but we might as well do things right.

commit efbdddc381cfea5bfa9527e86fa3078257e5d91b
Author: Paul Eggert <[hidden email]>
Date:   Thu Nov 15 22:59:33 2018 +0100

    mktime: make more room for overflow

    [BZ#23789]
    * time/mktime.c (long_int): Now 4⨯ int, not just 3⨯.
    This is so that we can add tm_diff results to a previous guess,
    which will be useful in a later patch.

commit 6c90d759f613761de7ac435bbabcc373092cf8bc
Author: Paul Eggert <[hidden email]>
Date:   Thu Nov 15 22:59:33 2018 +0100

    mktime: simplify offset guess

    [BZ#23789]
    * time/mktime.c (__mktime_internal): Omit excess precision.

commit 32c12f3f7ac9736d9ca2f0e074f1a3d02e973a35
Author: Paul Eggert <[hidden email]>
Date:   Thu Nov 15 22:59:33 2018 +0100

    mktime: new test for mktime failure

    [BZ#23789]
    Based on a test suggested by Albert Aribaud in:
    https://www.sourceware.org/ml/libc-alpha/2018-10/msg00662.html
    * time/Makefile (tests): Add bug-mktime4.
    * time/bug-mktime4.c: New file.

commit de20b81a038fe1c2060ce28125eec3838de5bdc5
Author: Paul Eggert <[hidden email]>
Date:   Thu Nov 15 22:59:33 2018 +0100

    mktime: fix EOVERFLOW bug

    [BZ#23789]
    * time/mktime.c [!_LIBC && !DEBUG_MKTIME]:
    Include libc-config.h, not config.h, for __set_errno.
    (guess_time_tm, __mktime_internal): Set errno to EOVERFLOW on overflow.

--
You are receiving this mail because:
You are on the CC list for the bug.