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

fix ASAN error in Vec2<float>::Length() #18493

Merged
merged 1 commit into from
Dec 8, 2023
Merged

Conversation

oltolm
Copy link
Contributor

@oltolm oltolm commented Dec 8, 2023

I compiled ppsspp with Clang on Windows and ASAN and found this bug.

=================================================================
==4456==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x007c60dfb3ff at pc 0x7ff71c8ec86f bp 0x007c60dfafe0 sp 0x007c60dfb028
READ of size 16 at 0x007c60dfb3ff thread T6
    #0 0x7ff71c8ec86e in Math3D::Vec2<float>::Length() const C:/src/ppsspp/GPU/Math3D.cpp:29:14
    #1 0x7ff71c8ed431 in Math3D::Vec2<float>::Normalized() const C:/src/ppsspp/GPU/Math3D.cpp:66:19
    #2 0x7ff71e7303b0 in SoftwareTransform::ExpandLines(int, int&, unsigned short*&, TransformedVertex const*, TransformedVertex*, int&, bool) C:/src/ppsspp/GPU/Common/SoftwareTransformCommon.cpp:778:50
    #3 0x7ff71e72bc73 in SoftwareTransform::BuildDrawingParams(int, int, unsigned int, unsigned short*&, int&, SoftwareTransformResult*) C:/src/ppsspp/GPU/Common/SoftwareTransformCommon.cpp:588:3
    #4 0x7ff71e1092e9 in DrawEngineD3D11::DoFlush() C:/src/ppsspp/GPU/D3D11/DrawEngineD3D11.cpp:429:16
    #5 0x7ff71e113baf in DrawEngineD3D11::Flush() C:/src/ppsspp/GPU/D3D11/DrawEngineD3D11.h:84:3
    #6 0x7ff71e110ccf in DrawEngineD3D11::DispatchFlush() C:/src/ppsspp/GPU/D3D11/DrawEngineD3D11.h:96:3
    #7 0x7ff71dee8a70 in GPUCommonHW::FastRunLoop(DisplayList&) C:/src/ppsspp/GPU/GPUCommonHW.cpp:844:24
    #8 0x7ff71dffd1c6 in GPUCommon::InterpretList(DisplayList&) C:/src/ppsspp/GPU/GPUCommon.cpp:675:4
    #9 0x7ff71dff9c57 in GPUCommon::ProcessDLQueue() C:/src/ppsspp/GPU/GPUCommon.cpp:845:8
    #10 0x7ff71dffad15 in GPUCommon::UpdateStall(int, unsigned int) C:/src/ppsspp/GPU/GPUCommon.cpp:506:2
    #11 0x7ff71c4f1122 in sceGeListUpdateStallAddr(unsigned int, unsigned int) C:/src/ppsspp/Core/HLE/sceGe.cpp:380:14
    #12 0x7ff71c4f0290 in void WrapI_UU<&sceGeListUpdateStallAddr(unsigned int, unsigned int)>() C:/src/ppsspp/Core/HLE/FunctionWrappers.h:200:15
    #13 0x7ff71b69136c in CallSyscallWithoutFlags(HLEFunction const*) C:/src/ppsspp/Core/HLE/HLE.cpp:659:2
    #14 0x7ff71980d50f  (<unknown module>)

Address 0x007c60dfb3ff is located in stack of thread T5 at offset 95 in frame
    #0 0x7ff71e72f68f in SoftwareTransform::ExpandLines(int, int&, unsigned short*&, TransformedVertex const*, TransformedVertex*, int&, bool) C:/src/ppsspp/GPU/Common/SoftwareTransformCommon.cpp:741

  This frame has 8 object(s):
    [32, 36) 'horizontal' (line 775)
    [48, 56) 'addWidth' (line 778)
    [80, 88) 'ref.tmp' (line 778)
    [112, 116) 'ref.tmp36' (line 778) <== Memory access at offset 95 underflows this variable
    [128, 132) 'horizontal156' (line 823)
    [144, 152) 'addWidth172' (line 825)
    [176, 184) 'ref.tmp173' (line 825)
    [208, 212) 'ref.tmp174' (line 825)
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp, SEH and C++ exceptions *are* supported)
Thread T5 created by T0 here:
    #0 0x7ffcc0cf3576 in CreateThread (C:\msys64\clang64\bin\libclang_rt.asan_dynamic-x86_64.dll+0x180053576)
    #1 0x7ffc95a4e755  (<unknown module>)
    #2 0x7ffc93598df1  (<unknown module>)
    #3 0x7ffc935a94ed  (<unknown module>)
    #4 0x7ffc935a9c68  (<unknown module>)
    #5 0x7ffc93497173  (<unknown module>)
    #6 0x7ffc93497500  (<unknown module>)
    #7 0x7ffc934966bd  (<unknown module>)
    #8 0x7ffc93495f13  (<unknown module>)
    #9 0x7ffc934896ac  (<unknown module>)
    #10 0x7ffc934e63ed  (<unknown module>)
    #11 0x7ffc934f2298  (<unknown module>)
    #12 0x7ffc934ffdd2  (<unknown module>)
    #13 0x7ffc934ff924  (<unknown module>)
    #14 0x7ffc9350183a  (<unknown module>)
    #15 0x7ffcb86ad2ed  (<unknown module>)
    #16 0x7ffc9357b84f  (<unknown module>)
    #17 0x7ffcb8695014  (<unknown module>)
    #18 0x7ffcb86b3653  (<unknown module>)
    #19 0x7ff71bdcd1e5 in VulkanMayBeAvailable() C:/src/ppsspp/Common/GPU/Vulkan/VulkanLoader.cpp:447:8
    #20 0x7ff71b41926d in Config::IsBackendEnabled(GPUBackend, bool) C:/src/ppsspp/Core/Config.cpp:537:41
    #21 0x7ff71b3a11b3 in MainWindow::UpdateMenus(bool) C:/src/ppsspp/Windows/MainWindowMenu.cpp:1162:31
    #22 0x7ff71b389b31 in MainWindow::Show(HINSTANCE__*) C:/src/ppsspp/Windows/MainWindow.cpp:521:3
    #23 0x7ff71b3d85d1 in wWinMain C:/src/ppsspp/Windows/main.cpp:991:2
    #24 0x7ff71c16d625 in wmain C:/M/B/src/mingw-w64/mingw-w64-crt/crt/crtexewin.c:67:10
    #25 0x7ff71a8e1318 in __tmainCRTStartup C:/M/B/src/mingw-w64/mingw-w64-crt/crt/crtexe.c:267:15
    #26 0x7ff71a8e1155 in .l_startw C:/M/B/src/mingw-w64/mingw-w64-crt/crt/crtexe.c:157:9
    #27 0x7ffd64fa7343  (C:\WINDOWS\System32\KERNEL32.DLL+0x180017343)
    #28 0x7ffd650e26b0  (C:\WINDOWS\SYSTEM32\ntdll.dll+0x1800526b0)

SUMMARY: AddressSanitizer: stack-buffer-overflow C:/src/ppsspp/GPU/Math3D.cpp:29:14 in Math3D::Vec2<float>::Length() const
Shadow bytes around the buggy address:
  0x007c60dfb100: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x007c60dfb180: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x007c60dfb200: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 f2 f2 f2
  0x007c60dfb280: 04 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
  0x007c60dfb300: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x007c60dfb380: 00 00 00 00 f1 f1 f1 f1 04 f2 00 f2 f2 f2 00[f2]
  0x007c60dfb400: f2 f2 04 f2 f8 f2 f8 f2 f2 f2 f8 f2 f2 f2 f8 f3
  0x007c60dfb480: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x007c60dfb500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x007c60dfb580: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x007c60dfb600: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
Thread T6 created by T0 here:
    #0 0x7ffcc0cf3576 in CreateThread (C:\msys64\clang64\bin\libclang_rt.asan_dynamic-x86_64.dll+0x180053576)
    #1 0x7ffd63011896  (C:\WINDOWS\System32\ucrtbase.dll+0x180021896)
    #2 0x7ffd3562a785 in std::__1::__libcpp_thread_create(void**, void* (*)(void*), void*) (C:\msys64\clang64\bin\libc++.dll+0x18009a785)
    #3 0x7ff71b31ebfb in std::__1::thread::thread<void (*)(), void>(void (*&&)()) C:/msys64/clang64/include/c++/v1/__thread/thread.h:248:16
    #4 0x7ff71b31bcc2 in MainThread_Start(bool) C:/src/ppsspp/Windows/EmuThread.cpp:64:15
    #5 0x7ff71b3d890e in wWinMain C:/src/ppsspp/Windows/main.cpp:1018:2
    #6 0x7ff71c16d625 in wmain C:/M/B/src/mingw-w64/mingw-w64-crt/crt/crtexewin.c:67:10
    #7 0x7ff71a8e1318 in __tmainCRTStartup C:/M/B/src/mingw-w64/mingw-w64-crt/crt/crtexe.c:267:15
    #8 0x7ff71a8e1155 in .l_startw C:/M/B/src/mingw-w64/mingw-w64-crt/crt/crtexe.c:157:9
    #9 0x7ffd64fa7343  (C:\WINDOWS\System32\KERNEL32.DLL+0x180017343)
    #10 0x7ffd650e26b0  (C:\WINDOWS\SYSTEM32\ntdll.dll+0x1800526b0)

==4456==ABORTING

@hrydgard
Copy link
Owner

hrydgard commented Dec 8, 2023

Yeah that's a good fix (though in practice, it's not gonna matter, but good to be asan-clean).

@hrydgard hrydgard added the Code Cleanup Cleanup to make future work easier. Needs to be done sometimes. label Dec 8, 2023
@hrydgard hrydgard added this to the v1.17.0 milestone Dec 8, 2023
@hrydgard hrydgard merged commit f8eb042 into hrydgard:master Dec 8, 2023
18 checks passed
@oltolm oltolm deleted the asan_bug branch December 8, 2023 20:45
@fp64
Copy link
Contributor

fp64 commented Dec 9, 2023

// Doubt this is worth it for a vec2 :/

FWIW, while this might be slightly faster for random access, clang autovectorizes the fallback version to something 3x faster for sequential access over a bunch of Vec2's:
https://godbolt.org/z/MxW7j8zKn

Seems unlikely to matter either way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Cleanup Cleanup to make future work easier. Needs to be done sometimes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants