Skip to content

ASAN false positives due to vfork() with GCC 9 #103726

Closed
Closed
ASAN false positives due to vfork() with GCC 9#103726
Labels
buildThe build process and cross-buildtype-bugAn unexpected behavior, bug, or error

Description

@colesbury
colesbury
opened on Apr 23, 2023

Older versions of ASAN don't properly support vfork(). Python uses vfork() when available in _posixsubprocess.c. This can lead to false positive reports from ASAN. I believe this issue exists with GCC 9 (and older), but not GCC 10 (and newer). The CPython GitHub actions ASAN runner currently uses GCC 9.4.0, so it does not properly handle vfork().

I've seen these false positives in my nogil-3.12 fork. I'm unsure if you've run into them upstream.

See google/sanitizers#925

cpython/Modules/_posixsubprocess.c

Lines 43 to 49 in 5041c2b

#if defined(__linux__) && defined(HAVE_VFORK) && defined(HAVE_SIGNAL_H) && \
defined(HAVE_PTHREAD_SIGMASK) && !defined(HAVE_BROKEN_PTHREAD_SIGMASK)
/* If this is ever expanded to non-Linux platforms, verify what calls are
* allowed after vfork(). Ex: setsid() may be disallowed on macOS? */
# include <signal.h>
# define VFORK_USABLE 1
#endif

Linked PRs

Activity

  1. colesbury
    added
    type-bugAn unexpected behavior, bug, or error
    on Apr 23, 2023

  2. corona10 commented on Apr 23, 2023

    @corona10
    corona10
    on Apr 23, 2023
    Member

    @colesbury Can I take a look at it?
    (Or if you prefer to submit the patch, it will be great)

  3. gpshead commented on Apr 23, 2023

    @gpshead
    gpshead
    on Apr 23, 2023
    Member

    bonus points if this can be done only under compiler versions known too old to ASAN the vfork code path so that we retain coverage of what actually gets used when possible.

  4. corona10 commented on Apr 23, 2023

    @corona10
    corona10
    on Apr 23, 2023
    Member

    The CPython GitHub actions ASAN runner currently uses GCC 9.4.0, so it does not properly handle vfork().

    But if I understand the issue correctly, what about just upgrading the GCC version by using the apt command for the ASAN tests?

  5. corona10 commented on Apr 23, 2023

    @corona10
    corona10
    on Apr 23, 2023
    Member

    Ah okay, now I understood what the title meant Disable vfork() when running under ASAN.
    It will be great if we can only disable vfork() under ASAN for old gcc versions.

  6. colesbury commented on Apr 23, 2023

    @colesbury
    colesbury
    on Apr 23, 2023
    ContributorAuthor

    Can I take a look at it?

    @corona10 - that would be great.

    bonus points if this can be done only under compiler versions known too old to ASAN the vfork code path

    We might also want to check the Clang version. I believe that Clang 9.0 and newer support vfork() properly:

    But if I understand the issue correctly, what about just upgrading the GCC version by using the apt command for the ASAN tests?

    That should work, but that means that if someone is running ASAN with an older compiler locally they may run into false positives. (That might be OK -- we could instruct people to use a newer compiler). I'm not sure which approach is better.

  7. gpshead commented on Apr 23, 2023

    @gpshead
    gpshead
    on Apr 23, 2023
    Member

    i'm not too worried about people using old toolchains for modern features such as ASAN. Our advice can simply be "upgrade". In my potentially biased experience people who understand what sanitizers are want the recent features and fixes for those to get more accurate results. So if just upgrading our CI and bot configs to use a more recent version is all it takes, sounds good to me.

  8. corona10 commented on Apr 23, 2023

    @corona10
    corona10
    on Apr 23, 2023
    Member

    So if just upgrading our CI and bot configs to use a more recent version is all it takes, sounds good to me.

    +1, I will submit the PR for it

  9. corona10
    added 2 commits that reference this issue on Apr 23, 2023

    pythongh-103726: Set up gcc-10 for ASAN

    7fc090f

    pythongh-103726: Set up gcc-10 for ASAN

    37c0927
  10. arhadthedev
    added
    extension-modulesC modules in the Modules dir
    on Apr 24, 2023
  11. corona10
    added
    buildThe build process and cross-build
    and removed
    extension-modulesC modules in the Modules dir
    on Apr 24, 2023

  12. corona10 commented on Apr 24, 2023

    @corona10
    corona10
    on Apr 24, 2023
    Member

    @arhadthedev I think that this is highly related to build rather than extension modules.

  13. colesbury
    changed the title [-]Disable vfork() when running under ASAN[/-] [+]ASAN false positives due to vfork() with GCC 9[/+] on Apr 24, 2023

  14. colesbury commented on Apr 24, 2023

    @colesbury
    colesbury
    on Apr 24, 2023
    ContributorAuthor

    I edited the issue title - I hope this is more clear (and doesn't confuse things).

  15. corona10
    added 3 commits that reference this issue on Apr 25, 2023

    pythongh-103726: Set up gcc-10 for ASAN

    b4b2e75

    Merge branch 'main' into pythongh-103726

    70b93d9

    gh-103726: Set up gcc-10 for ASAN (gh-103728)

    8330580

  16. corona10 commented on Apr 25, 2023

    @corona10
    corona10
    on Apr 25, 2023
    Member

    Now we can close the issue. Thanks for the report @colesbury

  17. corona10
    closed this as completedon Apr 26, 2023
  18. miss-islington
    added a commit that references this issue on May 23, 2023

    pythongh-103726: Set up gcc-10 for ASAN (pythongh-103728)

    8944408
  19. gpshead
    added a commit that references this issue on May 23, 2023

    [3.11] gh-103726: Set up gcc-10 for ASAN (gh-103728) (#104794)

    ac12a6b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    buildThe build process and cross-buildtype-bugAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions