Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[C API] Py_IS_INFINITY() macro doesn't work in the limited C API if isinf() is not defined #89603

Closed
vstinner opened this issue Oct 11, 2021 · 5 comments
Closed

[C API] Py_IS_INFINITY() macro doesn't work in the limited C API if isinf() is not defined #89603

vstinner opened this issue Oct 11, 2021 · 5 comments
Labels
3.11 only security fixes topic-C-API

Comments

@vstinner
Copy link
Member

BPO 45440
Nosy @vstinner
PRs
  • bpo-45440: Require math.h isinf() to build #28894
  • bpo-45440: Remove pymath.c fallbacks #28977
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = <Date 2021-10-13.21:30:01.697>
created_at = <Date 2021-10-11.23:51:20.093>
labels = ['expert-C-API', '3.11']
title = "[C API] Py_IS_INFINITY() macro doesn't work in the limited C API if isinf() is not defined"
updated_at = <Date 2021-10-15.17:45:37.921>
user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2021-10-15.17:45:37.921>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2021-10-13.21:30:01.697>
closer = 'vstinner'
components = ['C API']
creation = <Date 2021-10-11.23:51:20.093>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 45440
keywords = ['patch']
message_count = 5.0
messages = ['403704', '403706', '403873', '403874', '404041']
nosy_count = 1.0
nosy_names = ['vstinner']
pr_nums = ['28894', '28977']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue45440'
versions = ['Python 3.11']
    @vstinner
    Copy link
    Member Author

    If the HAVE_DECL_ISINF macro is not defined in pyconfig.h, the Py_IS_INFINITY macro is defined as:

    #define Py_IS_INFINITY(X) \
    ((X) && (Py_FORCE_DOUBLE(X)*0.5 == Py_FORCE_DOUBLE(X)))

    Problem: Py_FORCE_DOUBLE() is excluded from the limited C API (and the stable ABI).

    I see different options:

    • Implement Py_IS_INFINITY() as an opaque function if the HAVE_DECL_ISINF macro is not defined. I did something similar in Py_INCREF() to support the limited C API with a debug build of Python: call _Py_IncRef() opaque function.

    • Make Py_FORCE_DOUBLE() private and add it to the limited C API as an implementation detail.

    • Add Py_FORCE_DOUBLE() macro to the limited C API: the current implementation is fragile, it depends on how Python.h is included. Also, I dislike macros in the limited C API.

    I would prefer to *remove* Py_FORCE_DOUBLE() to all APIs, than adding it to the limited C API.

    @vstinner vstinner added 3.11 only security fixes topic-C-API labels Oct 11, 2021
    @vstinner
    Copy link
    Member Author

    The Py_FORCE_DOUBLE() macro was added in bpo-5724 by the change:

    commit e05e840
    Author: Mark Dickinson <dickinsm@gmail.com>
    Date: Mon May 4 13:30:43 2009 +0000

    Issue bpo-5724: Fix cmath failures on Solaris 10.

    @vstinner
    Copy link
    Member Author

    New changeset 194a952 by Victor Stinner in branch 'main':
    bpo-45440: Require math.h isinf() to build (GH-28894)
    194a952

    @vstinner
    Copy link
    Member Author

    [C API] Py_IS_INFINITY() macro doesn't work in the limited C API if isinf() is not defined

    Well, the final fix is to remove the code path when isinf() is not available and require it to build Python ;-)

    @vstinner
    Copy link
    Member Author

    New changeset 00ffc45 by Victor Stinner in branch 'main':
    bpo-45440: Remove pymath.c fallbacks (GH-28977)
    00ffc45

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.11 only security fixes topic-C-API
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    1 participant
    @vstinner