[issue39689] test_struct failure on s390x Fedora Clang buildbot

Benjamin Peterson added the comment: On Wed, Mar 11, 2020, at 07:41, Petr Viktorin wrote: > > > maybe we should be raising an error if the bytes are not a valid platform > > _Bool pattern? > > That's quite hard to test for. How so? We just make the same assumption you're making that true =

[issue39689] test_struct failure on s390x Fedora Clang buildbot

Change by Petr Viktorin : -- keywords: +patch pull_requests: +18277 stage: needs patch -> patch review pull_request: https://github.com/python/cpython/pull/18925 ___ Python tracker

[issue39689] test_struct failure on s390x Fedora Clang buildbot

Petr Viktorin added the comment: > our unittest assuming that b'\xf0' should be true when interpreted as a bool > is wrong. > just get rid of that value from the loop in the test? That could be the proper thing to do, but it does make it easy to invoke C undefined behavior from Python code.

[issue39689] test_struct failure on s390x Fedora Clang buildbot

Gregory P. Smith added the comment: FYI - I updated the ubsan buildbot to clang 9 so this one shows up in there now: /var/lib/buildbot/workers/clang-ubsan/3.x.gps-clang-ubsan.clang-ubsan/build/Modules/_struct.c:487:28: runtime error: load of value 116, which is not a valid value for type

[issue39689] test_struct failure on s390x Fedora Clang buildbot

STINNER Victor added the comment: Using memcpy() to write a value different than 0 or 1 into a _Bool is clearly an undefined behavior. Example with clang UBSan. bool.c: --- #include #include int main() { char ch = 42; _Bool x; memcpy(, , 1); return x == true; } --- $

[issue39689] test_struct failure on s390x Fedora Clang buildbot

Ammar Askar added the comment: Nothing too interesting, here's the stack trace: /src/cpython3/Modules/_struct.c:487:28: runtime error: load of value 32, which is not a valid value for type '_Bool' #0 0x7f50371a7670 in nu_bool cpython3/Modules/_struct.c:487:28 #1 0x7f503719ea3d in

[issue39689] test_struct failure on s390x Fedora Clang buildbot

Petr Viktorin added the comment: Viewing the oss-fuzz bug requires login. Is there any interesting public info in it? -- ___ Python tracker ___

[issue39689] test_struct failure on s390x Fedora Clang buildbot

Gregory P. Smith added the comment: fwiw oss-fuzz also finds this on struct (via ubsan) https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20949 struct.unpack('?', ' ') -- ___ Python tracker

[issue39689] test_struct failure on s390x Fedora Clang buildbot

STINNER Victor added the comment: Charalampos Stratakis also reported this issue to python-dev: "Unpacking native bools in the struct module: Is Python relying on undefined behavior?" https://mail.python.org/archives/list/python-...@python.org/thread/O742VLCYX2AE3RWQK5RBQ3BGUOHESLF5/

[issue39689] test_struct failure on s390x Fedora Clang buildbot

Petr Viktorin added the comment: IMO: - The "native" format should use native _Bool, and we should only test unpacking 0 and 1 - The "standard" format should use portable char semantics: continue to treat any non-zero value as true - The docs should grow a warning that for the native format

[issue39689] test_struct failure on s390x Fedora Clang buildbot

Gregory P. Smith added the comment: the concept of a native _Bool seems fuzzy. the important thing for the struct module is to consume sizeof _Bool bytes from the input stream. how those are interpreted is up to the platform. So if the platform says a bool is 8 bytes and it only ever

[issue39689] test_struct failure on s390x Fedora Clang buildbot

Change by Gregory P. Smith : -- components: +Extension Modules nosy: +gregory.p.smith stage: -> needs patch type: -> behavior ___ Python tracker ___

[issue39689] test_struct failure on s390x Fedora Clang buildbot

Benjamin Peterson added the comment: maybe we should be raising an error if the bytes are not a valid platform _Bool pattern? -- nosy: +benjamin.peterson ___ Python tracker

[issue39689] test_struct failure on s390x Fedora Clang buildbot

Petr Viktorin added the comment: C compiler dev that it's indeed undefined behavior. > Quick and obvious fix: > > static PyObject * > nu_bool(const char *p, const formatdef *f) > { > char x; > memcpy((char *), p, sizeof x); > return

[issue39689] test_struct failure on s390x Fedora Clang buildbot

Petr Viktorin added the comment: The call: struct.unpack('>?', b'\xf0') means to unpack a "native bool", i.e. native size and alignment. Internally, this does: static PyObject * nu_bool(const char *p, const formatdef *f) { _Bool x; memcpy((char *), p, sizeof

[issue39689] test_struct failure on s390x Fedora Clang buildbot

Charalampos Stratakis added the comment: On this loop: for c in [b'\x01', b'\x7f', b'\xff', b'\x0f', b'\xf0']: self.assertTrue(struct.unpack('>?', c)[0]) It fails for the b'\xf0' case -- ___ Python tracker

[issue39689] test_struct failure on s390x Fedora Clang buildbot

Charalampos Stratakis added the comment: Failed assertion here: https://github.com/python/cpython/blob/master/Lib/test/test_struct.py#L520 -- ___ Python tracker ___

[issue39689] test_struct failure on s390x Fedora Clang buildbot

New submission from Charalampos Stratakis : The clang build was recently added for that buildbot and it seems on that particular architecture, test_struct fails with: == FAIL: test_bool (test.test_struct.StructTest)