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

Compiler warnings in ubsan builds #85170

Closed
tiran opened this issue Jun 17, 2020 · 14 comments
Closed

Compiler warnings in ubsan builds #85170

tiran opened this issue Jun 17, 2020 · 14 comments
Labels
3.9 only security fixes 3.10 only security fixes build The build process and cross-build

Comments

@tiran
Copy link
Member

tiran commented Jun 17, 2020

BPO 40998
Nosy @vstinner, @ericvsmith, @tiran, @ericsnowcurrently, @miss-islington
PRs
  • bpo-40998: Address compiler warnings found by ubsan #20929
  • bpo-40998: Fix a refleak in create_filter() #23365
  • [3.9] bpo-40998: Fix a refleak in create_filter() (GH-23365) #23369
  • [3.9] bpo-40998: Address compiler warnings found by ubsan (GH-20929) #23370
  • 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 = None
    created_at = <Date 2020-06-17.07:00:21.597>
    labels = ['build', '3.9', '3.10']
    title = 'Compiler warnings in ubsan builds'
    updated_at = <Date 2020-11-18.17:45:56.990>
    user = 'https://github.com/tiran'

    bugs.python.org fields:

    activity = <Date 2020-11-18.17:45:56.990>
    actor = 'miss-islington'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Build']
    creation = <Date 2020-06-17.07:00:21.597>
    creator = 'christian.heimes'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 40998
    keywords = ['patch']
    message_count = 11.0
    messages = ['371712', '371713', '381332', '381334', '381335', '381336', '381338', '381349', '381353', '381354', '381362']
    nosy_count = 5.0
    nosy_names = ['vstinner', 'eric.smith', 'christian.heimes', 'eric.snow', 'miss-islington']
    pr_nums = ['20929', '23365', '23369', '23370']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'compile error'
    url = 'https://bugs.python.org/issue40998'
    versions = ['Python 3.9', 'Python 3.10']

    @tiran
    Copy link
    Member Author

    tiran commented Jun 17, 2020

    I'm seeing several compiler warnings in ubsan builds:

    $ ./configure --with-address-sanitizer --with-undefined-behavior-sanitizer
    $ make clean
    $ make
    Parser/string_parser.c: In function ‘decode_unicode_with_escapes’:
    Parser/string_parser.c:98:17: warning: null destination pointer [-Wformat-overflow=]
       98 |                 sprintf(p, "\\U%08x", chr);
          |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~
    Parser/string_parser.c:98:17: warning: null destination pointer [-Wformat-overflow=]
    Parser/string_parser.c:98:17: warning: null destination pointer [-Wformat-overflow=]
    In function ‘assemble_lnotab’,
        inlined from ‘assemble_emit’ at Python/compile.c:5697:25,
        inlined from ‘assemble’ at Python/compile.c:6036:18:
    Python/compile.c:5651:19: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
     5651 |         *lnotab++ = k;
          |         ~~~~~~~~~~^~~
    Objects/unicodeobject.c: In function ‘xmlcharrefreplace’:
    Objects/unicodeobject.c:849:16: warning: null destination pointer [-Wformat-overflow=]
      849 |         str += sprintf(str, "&#%d;", PyUnicode_READ(kind, data, i));
          |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    Objects/unicodeobject.c:849:16: warning: null destination pointer [-Wformat-overflow=]
    Objects/unicodeobject.c:849:16: warning: null destination pointer [-Wformat-overflow=]
    Python/pylifecycle.c: In function ‘Py_FinalizeEx’:
    Python/pylifecycle.c:1339:25: warning: unused variable ‘interp’ [-Wunused-variable]
     1339 |     PyInterpreterState *interp = tstate->interp;
          |

    @tiran tiran added 3.9 only security fixes 3.10 only security fixes build The build process and cross-build labels Jun 17, 2020
    @tiran
    Copy link
    Member Author

    tiran commented Jun 17, 2020

    PR 20929 addresses three out of four warnings found by GCC 10's ubsan on Fedora 32.

    @tiran
    Copy link
    Member Author

    tiran commented Nov 18, 2020

    I see more warnings and a new leak detection on latest master:

    gcc -pthread -c -fsanitize=undefined -fsanitize=address -fno-omit-frame-pointer -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -std=c99 -Wextra -Wno-unused-result -Wno-unused-parameter -Wno-missing-field-initializers -Werror=implicit-function-declaration -fvisibility=hidden -I./Include/internal -I. -I./Include -DPy_BUILD_CORE -o Objects/codeobject.o Objects/codeobject.c
    In function ‘emit_pair’,
    inlined from ‘emit_delta’ at Objects/codeobject.c:423:14,
    inlined from ‘code_getlnotab’ at Objects/codeobject.c:462:18:
    Objects/codeobject.c:414:15: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
    414 | *lnotab++ = b;
    | ~~~~~~~~~~^~~
    In function ‘emit_pair’,
    inlined from ‘emit_delta’ at Objects/codeobject.c:436:14,
    inlined from ‘code_getlnotab’ at Objects/codeobject.c:462:18:
    Objects/codeobject.c:414:15: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
    414 | *lnotab++ = b;
    | ~~~~~~~~~~^~~
    In function ‘emit_pair’,
    inlined from ‘emit_delta’ at Objects/codeobject.c:429:14,
    inlined from ‘code_getlnotab’ at Objects/codeobject.c:462:18:
    Objects/codeobject.c:414:15: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
    414 | *lnotab++ = b;
    | ~~~~~~~~~~^~~
    gcc -pthread -c -fsanitize=undefined -fsanitize=address -fno-omit-frame-pointer -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -std=c99 -Wextra -Wno-unused-result -Wno-unused-parameter -Wno-missing-field-initializers -Werror=implicit-function-declaration -fvisibility=hidden -I./Include/internal -I. -I./Include -DPy_BUILD_CORE -o Python/compile.o Python/compile.c
    In function ‘assemble_emit_linetable_pair’,
    inlined from ‘assemble_line_range’ at Python/compile.c:5614:18:
    Python/compile.c:5586:15: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
    5586 | *lnotab++ = ldelta;
    | ~~~~~~~~~~^~~~~~~~
    In function ‘assemble_emit_linetable_pair’,
    inlined from ‘assemble_line_range’ at Python/compile.c:5608:18:
    Python/compile.c:5586:15: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
    5586 | *lnotab++ = ldelta;
    | ~~~~~~~~~~^~~~~~~~
    In function ‘assemble_emit_linetable_pair’,
    inlined from ‘assemble’ at Python/compile.c:6011:10:
    Python/compile.c:5586:15: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
    5586 | *lnotab++ = ldelta;
    | ~~~~~~~~~~^~~~~~~~

    ==669952==ERROR: LeakSanitizer: detected memory leaks

    Direct leak of 57 byte(s) in 1 object(s) allocated from:
    #0 0x7fa6397cc667 in __interceptor_malloc (/lib64/libasan.so.6+0xb0667)
    #1 0x777579 in PyUnicode_New Objects/unicodeobject.c:1459
    #2 0x86aa4e in unicode_decode_utf8 Objects/unicodeobject.c:5129
    #3 0x8b6050 in PyUnicode_DecodeUTF8Stateful Objects/unicodeobject.c:5259
    #4 0x8b6050 in PyUnicode_FromString Objects/unicodeobject.c:2311
    #5 0x8b6050 in PyUnicode_InternFromString Objects/unicodeobject.c:15788
    #6 0x8f1e0b in create_filter Python/_warnings.c:67
    #7 0x8f1e0b in init_filters Python/_warnings.c:95
    #8 0x8f1e0b in _PyWarnings_InitState Python/_warnings.c:123
    #9 0xa52178 in pycore_init_types Python/pylifecycle.c:704
    #10 0xa52178 in pycore_interp_init Python/pylifecycle.c:760
    #11 0xa5eab1 in pyinit_config Python/pylifecycle.c:807
    #12 0xa5eab1 in pyinit_core Python/pylifecycle.c:970
    #13 0xa60037 in Py_InitializeFromConfig Python/pylifecycle.c:1155
    #14 0x47a842 in pymain_init Modules/main.c:66
    #15 0x4802a2 in pymain_main Modules/main.c:698
    #16 0x4802a2 in Py_BytesMain Modules/main.c:731
    #17 0x7fa638a5b041 in __libc_start_main (/lib64/libc.so.6+0x27041)

    @tiran
    Copy link
    Member Author

    tiran commented Nov 18, 2020

    The reference leak was introduced in 86ea581 / #57368 / bpo-36737. PR #65128 fixes it, too.

    @tiran
    Copy link
    Member Author

    tiran commented Nov 18, 2020

    Sorry, I meant to add Eric S. :)

    @ericvsmith
    Copy link
    Member

    Too many Eric S's!

    @vstinner
    Copy link
    Member

    The reference leak was introduced in 86ea581 / #57368 / bpo-36737. PR #65128 fixes it, too.

    Usually, it's not that the subinterpreter work introduces new leak, but makes old leak suddenly visible.

    The create_filter() leak was introduced way earlier:

    commit 9b99747
    Author: Nick Coghlan <ncoghlan@gmail.com>
    Date: Mon Jan 8 12:45:02 2018 +1000

    bpo-31975 (PEP-565): Show DeprecationWarning in __main__ (GH-4458)
    

    It's a minor leak since the create_filter() function is called exactly 5 times at startup. It's a leak of 5 strong references, it's not a big deal :-)

    @miss-islington
    Copy link
    Contributor

    New changeset d1e38d4 by Victor Stinner in branch 'master':
    bpo-40998: Fix a refleak in create_filter() (GH-23365)
    d1e38d4

    @miss-islington
    Copy link
    Contributor

    New changeset 07f2ade by Christian Heimes in branch 'master':
    bpo-40998: Address compiler warnings found by ubsan (GH-20929)
    07f2ade

    @miss-islington
    Copy link
    Contributor

    New changeset 994c68f by Miss Islington (bot) in branch '3.9':
    bpo-40998: Address compiler warnings found by ubsan (GH-20929)
    994c68f

    @miss-islington
    Copy link
    Contributor

    New changeset 35bf8ea by Christian Heimes in branch '3.9':
    [3.9] bpo-40998: Fix a refleak in create_filter() (GH-23365) (GH-23369)
    35bf8ea

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @iritkatriel
    Copy link
    Member

    Is there anything left to do here?

    @iritkatriel iritkatriel added the pending The issue will be closed if no feedback is provided label May 20, 2022
    @tiran
    Copy link
    Member Author

    tiran commented May 20, 2022

    I don't think so.

    @tiran tiran closed this as completed May 20, 2022
    @tiran tiran removed the pending The issue will be closed if no feedback is provided label May 20, 2022
    @vstinner
    Copy link
    Member

    xmlcharrefreplace():

            size = sprintf(str, "&#%d;", PyUnicode_READ(kind, data, i));
            if (size < 0) {
                return NULL;
            }
            str += size;
    

    If sprintf() can return a negative value, a Python exception must be raised when returning NULL. Otherwise, you get a SystemError (return NULL without raising an exception).

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.9 only security fixes 3.10 only security fixes build The build process and cross-build
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants