[issue33312] ubsan undefined behavior sanitizer flags struct _dictkeysobject (PyDictKeysObj)

Ned Deily added the comment: This issue does not seem to me to be a security issue so would not meet the criteria for backporting to 3.6. -- ___ Python tracker ___

[issue33312] ubsan undefined behavior sanitizer flags struct _dictkeysobject (PyDictKeysObj)

Gregory P. Smith added the comment: automated backport of the gdb/libpython change fails on 3.6, if this is needed there (I haven't looked into the code on that branch), it's a tiny change to apply. i'll leave that to the 3.6 RM to decide if at all. -- resolution: -> fixed stage:

[issue33312] ubsan undefined behavior sanitizer flags struct _dictkeysobject (PyDictKeysObj)

Gregory P. Smith added the comment: Sorry for the breakage. I'm glad the fix was that greppable and easy given I had no idea where to start otherwise. :) This is in for 3.7. The PR for 3.6 is pending, i want to let it out in the next 3.7 builds for a while before i merge

[issue33312] ubsan undefined behavior sanitizer flags struct _dictkeysobject (PyDictKeysObj)

Ned Deily added the comment: Thanks, Greg, looking better! -- ___ Python tracker ___ ___

[issue33312] ubsan undefined behavior sanitizer flags struct _dictkeysobject (PyDictKeysObj)

Gregory P. Smith added the comment: New changeset 11659d00b9185c8f02ea6b642fa475a80e21f1a9 by Gregory P. Smith (Miss Islington (bot)) in branch '3.7': bpo-33312: update Tools/gdb/libpython.py to match. (GH-6549)

[issue33312] ubsan undefined behavior sanitizer flags struct _dictkeysobject (PyDictKeysObj)

Change by miss-islington : -- pull_requests: +6246 ___ Python tracker ___

[issue33312] ubsan undefined behavior sanitizer flags struct _dictkeysobject (PyDictKeysObj)

Gregory P. Smith added the comment: New changeset 53f67d401df40486fd0fb8fbcf9da725cd37290c by Gregory P. Smith in branch 'master': bpo-33312: update Tools/gdb/libpython.py to match. (GH-6548) https://github.com/python/cpython/commit/53f67d401df40486fd0fb8fbcf9da725cd37290c

[issue33312] ubsan undefined behavior sanitizer flags struct _dictkeysobject (PyDictKeysObj)

Change by Gregory P. Smith : -- pull_requests: +6245 ___ Python tracker ___ ___

[issue33312] ubsan undefined behavior sanitizer flags struct _dictkeysobject (PyDictKeysObj)

Gregory P. Smith added the comment: I see one line in Tools/gdb/libpython.py that may be related. i'll try changing that. the only way i have to _test_ it is to merge it into master. -- ___ Python tracker

[issue33312] ubsan undefined behavior sanitizer flags struct _dictkeysobject (PyDictKeysObj)

Gregory P. Smith added the comment: ned: yes, it supposedly has. test_gdb. in a manner i cannot reproduce or debug without error output that indicates anything about the problem. test_gdb passes on my systems. -- ___ Python

[issue33312] ubsan undefined behavior sanitizer flags struct _dictkeysobject (PyDictKeysObj)

Gregory P. Smith added the comment: New changeset 392520bd78cd18639a27e5d2803c2e1c2bd593a8 by Gregory P. Smith (Miss Islington (bot)) in branch '3.7': bpo-33312: Fix clang ubsan out of bounds warnings in dict. (GH-6537) (GH-6543)

[issue33312] ubsan undefined behavior sanitizer flags struct _dictkeysobject (PyDictKeysObj)

Ned Deily added the comment: Greg, this change has broken some buildbots: http://buildbot.python.org/all/#/builders/85/builds/934 http://buildbot.python.org/all/#/builders/21/builds/951 -- nosy: +ned.deily ___ Python tracker

[issue33312] ubsan undefined behavior sanitizer flags struct _dictkeysobject (PyDictKeysObj)

Gregory P. Smith added the comment: If you think this should be written differently, please propose it in a PR so we can see what you are suggesting. An unbounded member at the end of a struct is quite a common practice in C. ex: PyBytesObject --

[issue33312] ubsan undefined behavior sanitizer flags struct _dictkeysobject (PyDictKeysObj)

Florian Weimer added the comment: Why does the code even need the flexible struct member? If you use the surrounding backing store directly, the aliasing issue disappears if the backing store is untyped memory (if not, you have the aliasing problem with the rest of the

[issue33312] ubsan undefined behavior sanitizer flags struct _dictkeysobject (PyDictKeysObj)

Change by miss-islington : -- pull_requests: +6240 ___ Python tracker ___

[issue33312] ubsan undefined behavior sanitizer flags struct _dictkeysobject (PyDictKeysObj)

Change by miss-islington : -- pull_requests: +6239 ___ Python tracker ___

[issue33312] ubsan undefined behavior sanitizer flags struct _dictkeysobject (PyDictKeysObj)

Gregory P. Smith added the comment: New changeset 397f1b28c4a12e3b3ed59a89599eabc457412649 by Gregory P. Smith in branch 'master': bpo-33312: Fix clang ubsan out of bounds warnings in dict. (GH-6537)

[issue33312] ubsan undefined behavior sanitizer flags struct _dictkeysobject (PyDictKeysObj)

Benjamin Peterson added the comment: I was fixing strict aliasing problems in the code and I thought the union was cleaner than a bunch of casts. -- ___ Python tracker

[issue33312] ubsan undefined behavior sanitizer flags struct _dictkeysobject (PyDictKeysObj)

Gregory P. Smith added the comment: My PR is almost a revert of https://github.com/python/cpython/commit/186122ead26f3ae4c2bc9f6715d2a29d339fdc5a so I'm curious what your motivation behind that change to use the union was? -- ___

[issue33312] ubsan undefined behavior sanitizer flags struct _dictkeysobject (PyDictKeysObj)

Gregory P. Smith added the comment: I'm going to see what appveyor says with the VLA code on the PR. I've updated it to use char[]. -- ___ Python tracker

[issue33312] ubsan undefined behavior sanitizer flags struct _dictkeysobject (PyDictKeysObj)

Gregory P. Smith added the comment: By my reading on how strict aliasing works, I think just changing the int64_t[1] or int32_t[1] in my PR to char[1] will work as char is always assumed to alias? the clang ubsan i was testing my PR against wasn't warning me about strict

[issue33312] ubsan undefined behavior sanitizer flags struct _dictkeysobject (PyDictKeysObj)

Benjamin Peterson added the comment: Your PR is basically what we did prior to 186122ead26f3ae4c2bc9f6715d2a29d339fdc5a. The problem is that may run afoul of different UB, namely strict aliasing. (Though, I suppose we could probably also avoid that by making dk_indices

[issue33312] ubsan undefined behavior sanitizer flags struct _dictkeysobject (PyDictKeysObj)

Change by Gregory P. Smith : -- pull_requests: +6233 stage: needs patch -> patch review ___ Python tracker ___

[issue33312] ubsan undefined behavior sanitizer flags struct _dictkeysobject (PyDictKeysObj)

Gregory P. Smith added the comment: notably, C99 variable length arrays syntax is not mentioned as allowed in https://www.python.org/dev/peps/pep-0007/. If we want to use VLAs, that should be clarified. But both our solutions should work with [1] instead of [] which is

[issue33312] ubsan undefined behavior sanitizer flags struct _dictkeysobject (PyDictKeysObj)

Change by INADA Naoki : -- nosy: +inada.naoki ___ Python tracker ___ ___

[issue33312] ubsan undefined behavior sanitizer flags struct _dictkeysobject (PyDictKeysObj)

Gregory P. Smith added the comment: I think it is worth getting such a change in. its arguable that the compiler should be able to see through the union for this use case but I assume such a fix would only land in a recent clang version. i'm attaching a variant on your patch

[issue33312] ubsan undefined behavior sanitizer flags struct _dictkeysobject (PyDictKeysObj)

Benjamin Peterson added the comment: Yeah, I've run into this before. The "correct" thing to do is use C99 VLAs. Unfortunately, that doesn't work for PyDictKeysObject because it really wants a union of VLAs but that isn't supported. The best I could do is making a struct

[issue33312] ubsan undefined behavior sanitizer flags struct _dictkeysobject (PyDictKeysObj)

Gregory P. Smith added the comment: related: https://ssl.icu-project.org/trac/ticket/13503 -- ___ Python tracker ___

[issue33312] ubsan undefined behavior sanitizer flags struct _dictkeysobject (PyDictKeysObj)

Change by Gregory P. Smith : -- nosy: +twouters ___ Python tracker ___ ___ Python-bugs-list

[issue33312] ubsan undefined behavior sanitizer flags struct _dictkeysobject (PyDictKeysObj)

Change by Gregory P. Smith : -- nosy: +benjamin.peterson, fweimer ___ Python tracker ___

[issue33312] ubsan undefined behavior sanitizer flags struct _dictkeysobject (PyDictKeysObj)

New submission from Gregory P. Smith : Build CPython (master in this case - though I originally noticed the problem when building a 3.6 tree) as follows with clang installed: build$ LD=clang-5.0 LDFLAGS=-fsanitize=undefined CC=clang-5.0 CXX=clang-5.0