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.

Sign up for GitHub

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

Jump to bottom

Undefined behaviour tests ubsan #1313

Open
illume opened this issue Sep 17, 2019 · 3 comments · May be fixed by #4141
Open

Undefined behaviour tests ubsan #1313

illume opened this issue Sep 17, 2019 · 3 comments · May be fixed by #4141
Labels
Code quality/robustness Code quality and resilience to changes easy An easy challenge to solve tooling

Comments

@illume
Copy link
Member

illume commented Sep 17, 2019 •
edited

UndefinedBehaviorSanitizer (UBSan) is a fast undefined behavior detector. UBSan modifies the program at compile-time to catch various kinds of undefined behavior during program execution, for example:

  • Using misaligned or null pointer
  • Signed integer overflow
  • Conversion to, from, or between floating-point types which would overflow the destination

Here is the gcc documentation for Instrumentation Options and UBSAN (https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html).

From https://www.sqlite.org/testing.html

To help ensure that SQLite does not make use of undefined or implementation defined behavior, the test suites are rerun using instrumented builds that try to detect undefined behavior. For example, test suites are run using the "-ftrapv" option of GCC. And they are run again using the "-fsanitize=undefined" option on Clang. And again using the "/RTC1" option in MSVC

To compile a python C extension with a UBSAN with clang on Mac do:

LDFLAGS="-g -fsanitize=undefined" CFLAGS="-g -fsanitize=undefined -fno-omit-frame-pointer" python3 setup.py install

The Microsoft Visual Studio compiler can use the Run Time Error Checks feature to find some issues. /RTC (Run-Time Error Checks) (https://docs.microsoft.com/en-us/cpp/build/reference/rtc-run-time-error-checks?view=vs-2019)
@illume illume added the tooling label Sep 17, 2019
@illume
Copy link
Member Author

illume commented Oct 5, 2019

Here are two issues the UBSAN found on my 64bit Mac whilst running the unit tests.

LDFLAGS="-g -fsanitize=undefined" CFLAGS="-g -fsanitize=undefined -fno-omit-frame-pointer" python3 setup.py install
python3 -m pygame.tests
src_c/draw.c:1273:13: runtime error: store to misaligned address 0x7fa14046e1b3 for type 'Uint16' (aka 'unsigned short'), which requires 2 byte alignment
0x7fa14046e1b3: note: pointer points here
 00  00 00 ff 00 00 00 00 00  ff 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00
              ^ 
src_c/freetype/ft_cache.c:103:43: runtime error: index -1 out of bounds for type 'FT_UInt32 const[8]'

@illume
Copy link
Member Author

illume commented Oct 5, 2019

I tested with both clang and gcc-9 with the -ftrapv command line arg, and ran the tests with no issues.

CC="gcc-9" LDFLAGS="-g -ftrapv" CFLAGS="-g -ftrapv -fno-omit-frame-pointer" python3 setup.py install
LDFLAGS="-g -ftrapv" CFLAGS="-g -ftrapv -fno-omit-frame-pointer" python3 setup.py install

@illume
Copy link
Member Author

illume commented Oct 5, 2019

I ran ubsan under gcc, and only got one issue: the draw.c one that clang found.

CC="gcc-9" LDFLAGS="-g -fsanitize=undefined" CFLAGS="-g -fsanitize=undefined -fno-omit-frame-pointer" python3 setup.py install

@MyreMylar MyreMylar added easy An easy challenge to solve Code quality/robustness Code quality and resilience to changes labels May 16, 2020
matoro added a commit to matoro/pygame that referenced this issue Jan 2, 2024
@matoro
Fix undefined behavior in pixel get/set routines
1092554 
The unit tests only seem to exercise the UB in set_pixel_color, but I
believe the same issue is present in get_pixel_color.  There is no
guarantee that the incoming pixel buffer is aligned, so casting it to a
larger integer with alignment requirements can issue an unaligned load.

I've unified and simplified the LE/BE implementation divergence as much
as possible.  memcpy() should compile to roughly the same asm as a
direct assignment.  Tested on LE and BE.

Should fix pygame#1313
@matoro matoro linked a pull request Jan 2, 2024 that will close this issue
Open
matoro added a commit to matoro/pygame that referenced this issue Jan 2, 2024
@matoro
Fix undefined behavior in pixel get/set routines
0765626 
The unit tests only seem to exercise the UB in set_pixel_color, but I
believe the same issue is present in get_pixel_color.  There is no
guarantee that the incoming pixel buffer is aligned, so casting it to a
larger integer with alignment requirements can issue an unaligned load.

I've unified and simplified the LE/BE implementation divergence as much
as possible.  memcpy() should compile to roughly the same asm as a
direct assignment.  Tested on LE and BE.

Should fix pygame#1313
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Code quality/robustness Code quality and resilience to changes easy An easy challenge to solve tooling
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix undefined behavior in pixel get/set routines matoro/pygame
2 participants
@illume @MyreMylar