Skip to content

ABI breaks between debug and non-debug build in generated files. #108918

Open
Open
ABI breaks between debug and non-debug build in generated files.#108918
Assignees
ericsnowcurrently
Labels
interpreter-core(Objects, Python, Grammar, and Parser dirs)type-bugAn unexpected behavior, bug, or error

Description

@markshannon
markshannon
opened on Sep 5, 2023

Bug report

Bug description:

The set of generated objects in the various /Include/internal/pycore_ ... generated.h header files depends on configuration flags.
This causes unnecessary churn and defeats some PR checks.
It also makes it hard to understand what is going on.

Currently we scan the C source code looking for specific patterns, but this is evidently a somewhat flaky way of doing things.
The set of objects should be defined in a checked-in file, to avoid the above issues.

I would suggest a Python file defining the list of strings, possibly with aliases:

name
alias = "not an identifier"

would define the two strings: "name" referenced the symbol _PyPrefix_name and "not an identifier" referenced the symbol _PyPrefix_alias. PyPrefix is a placeholder for the actual prefix.

Activity

  1. markshannon
    added
    type-bugAn unexpected behavior, bug, or error
    on Sep 5, 2023
  2. markshannon
    assigned
    ericsnowcurrently
    on Sep 5, 2023

  3. kumaraditya303 commented on Sep 5, 2023

    @kumaraditya303
    kumaraditya303
    on Sep 5, 2023
    Contributor

    The set of generated objects in the various /Include/internal/pycore_ ... generated.h header files depends on configuration flags.

    What configuration option causes that?

    This causes unnecessary churn and defeats some PR checks.

    Can you provide link to such a case where it failed?

  4. markshannon commented on Sep 5, 2023

    @markshannon
    markshannon
    on Sep 5, 2023
    MemberAuthor

    The ABI checks for 3.12 backports fail when using .configure --with-pydebug as it generates extra strings.
    #108899

  5. iritkatriel
    added
    interpreter-core(Objects, Python, Grammar, and Parser dirs)
    on Dec 1, 2023

  6. markshannon commented on Jan 12, 2024

    @markshannon
    markshannon
    on Jan 12, 2024
    MemberAuthor

    I just got tripped up by this again.
    82acac8
    Note all the unrelated changes to Include/internal/pycore_global_strings.h

    I think we need to have a list of strings that we are going to add to pycore_global_strings.h checked in.
    The tooling can check that the set of required strings in a subset of that list, but generate the full list for consistency.

  7. markshannon commented on Jan 12, 2024

    @markshannon
    markshannon
    on Jan 12, 2024
    MemberAuthor

    This is really blocking me, as I have no way of telling what is the correct set of strings. make regen-all creates some new static strings, but then CI fails because the set is incorrect on a different platform.

  8. markshannon commented on Jan 12, 2024

    @markshannon
    markshannon
    on Jan 12, 2024
    MemberAuthor

    Windows build implicitly regenerate the files, resulting in quite the mess where string are added by make regen-all all, then removed by PCbuild\build.bat -e on another.

  9. markshannon commented on Jan 12, 2024

    @markshannon
    markshannon
    on Jan 12, 2024
    MemberAuthor

    adae6cf
    is a diff from running PCbuild\build.bat -e on Windows, on the code generated by make regen-all on linux.

  10. markshannon commented on Jan 18, 2024

    @markshannon
    markshannon
    on Jan 18, 2024
    MemberAuthor

    Another failure: https://github.com/python/cpython/actions/runs/7572366680/job/20622023781?pr=114247

  11. ericsnowcurrently commented on Jan 22, 2024

    @ericsnowcurrently
    ericsnowcurrently
    on Jan 22, 2024
    Member

    Sorry for the delay. I'll take a look at this tomorrow.

  12. ericsnowcurrently commented on Jan 23, 2024

    @ericsnowcurrently
    ericsnowcurrently
    on Jan 23, 2024
    Member

    Just to make sure we're on the same page...

    • the build toolchain includes a step that generates the list of global strings, which are statically allocated
      1. the build tool (Tools/build/generate_global_objects.py) identifies all global strings
        • a hard-coded list of strings in the script (IDENTIFIERS)
        • uses of _Py_ID() and _Py_DECLARE_STR()
      2. the build tool updates several files with the identified strings:
        • Include/internal/pycore_global_objects_fini_generated.h
        • Include/internal/pycore_global_strings.h
        • Include/internal/pycore_runtime_init_generated.h
        • Include/internal/pycore_unicodeobject_generated.h
    • the generated files are tracked in the repo (at least in part to facilitate bootstrapping, i.e. bootstrap_python)
    • we have a CI check on PRs that runs the tool and checks for differences in the generated files

    The problem is that different builds/platforms may have slightly different sets of global strings. This leads to the following two issues:

    • the generated files in the repo reflect the platform/build on which development happened for the last PR that touched the generated files (thus potentially restricting which platforms may actually bootstrap)
    • the CI check is specific to a single linux build, meaning global strings identified just for some other platform would cause the check to fail

    This is a problem both because it restricts which platforms/builds can properly bootstrap and because it's a pain for anyone developing for a platform with extra generated strings.

    Is that right?

  13. ericsnowcurrently commented on Jan 23, 2024

    @ericsnowcurrently
    ericsnowcurrently
    on Jan 23, 2024
    Member

    Having a separate data file with the list of all supported global strings makes sense. We already do something like that with the C-API and elsewhere.

    In the meantime, one workaround is to add the problematic strings to the IDENTIFIERS list in Tools/build/generate_global_objects.py. That list contains strings that are always made global.

  14. ericsnowcurrently
    added
    topic-subinterpreters
    on Feb 27, 2024
  15. github-project-automation
    added this to Subinterpreterson Feb 27, 2024
  16. github-project-automation
    moved this to Todo in Subinterpreterson Feb 27, 2024
  17. ericsnowcurrently
    removed
    topic-subinterpreters
    on Jul 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

interpreter-core(Objects, Python, Grammar, and Parser dirs)type-bugAn unexpected behavior, bug, or error

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions