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

Fix stack-use-after-scope error reported by MSVC ASAN #53919

Merged
merged 1 commit into from
Dec 31, 2021
Merged

Fix stack-use-after-scope error reported by MSVC ASAN #53919

merged 1 commit into from
Dec 31, 2021

Conversation

BrettDong
Copy link
Member

Summary

None

Purpose of change

There is a 100% steadily reproducible stack-use-after-scope error reported by Visual Studio Address Sanitizer when starting a new game:

==4164==ERROR: AddressSanitizer: stack-use-after-scope on address 0x00ff86efdb40 at pc 0x7ff757b01a73 bp 0x00ff86efd890 sp 0x00ff86efd898
READ of size 4 at 0x00ff86efdb40 thread T0
==4164==WARNING: Failed to use and restart external symbolizer!
'cataclysm-tiles.exe' (Win32): Loaded 'C:\Windows\System32\dbghelp.dll'. 
    #0 0x7ff757b01a72 in generic_factory<body_part_type>::obj C:\Users\Brett\Git\Cataclysm-DDA\src\generic_factory.h:429
    #1 0x7ff75744d00a in Creature::get_part C:\Users\Brett\Git\Cataclysm-DDA\src\creature.cpp:1947
    #2 0x7ff757431eb2 in get_part_helper<int> C:\Users\Brett\Git\Cataclysm-DDA\src\creature.cpp:1959
    #3 0x7ff7573d8c37 in display::temp_text_color C:\Users\Brett\Git\Cataclysm-DDA\src\panels.cpp:713
    #4 0x7ff7573af8e5 in draw_needs_labels C:\Users\Brett\Git\Cataclysm-DDA\src\panels.cpp:1884
    #5 0x7ff757189d7b in game::draw_panels C:\Users\Brett\Git\Cataclysm-DDA\src\game.cpp:3310
    #6 0x7ff757188747 in game::draw C:\Users\Brett\Git\Cataclysm-DDA\src\game.cpp:3270
    #7 0x7ff757282096 in ui_adaptor::redraw_invalidated C:\Users\Brett\Git\Cataclysm-DDA\src\ui_manager.cpp:282
    #8 0x7ff7570baf74 in do_turn C:\Users\Brett\Git\Cataclysm-DDA\src\do_turn.cpp:687
    #9 0x7ff756ef87d8 in WinMain C:\Users\Brett\Git\Cataclysm-DDA\src\main.cpp:711
    #10 0x7ff758e3f7e9 in __scrt_common_main_seh d:\a01\_work\6\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
    #11 0x7ffb16fb7033 in BaseThreadInitThunk+0x13 (C:\Windows\System32\KERNEL32.DLL+0x180017033)
    #12 0x7ffb18102650 in RtlUserThreadStart+0x20 (C:\Windows\SYSTEM32\ntdll.dll+0x180052650)

Address 0x00ff86efdb40 is located in stack of thread T0 at offset 32 in frame
    #0 0x7ff7573d8b2f in display::temp_text_color C:\Users\Brett\Git\Cataclysm-DDA\src\panels.cpp:705

  This frame has 10 object(s):
    [32, 40) 'compiler temporary' <== Memory access at offset 32 is inside this variable
    [48, 80) 'temp_string'
    [64, 68) 'temp_color'
    [80, 84) 'compiler temporary'
    [96, 100) 'compiler temporary'
    [112, 116) 'compiler temporary'
    [128, 132) 'compiler temporary'
    [144, 148) 'compiler temporary'
    [160, 164) 'compiler temporary'
    [176, 180) 'compiler temporary'
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)
SUMMARY: AddressSanitizer: stack-use-after-scope C:\Users\Brett\Git\Cataclysm-DDA\src\generic_factory.h:429 in generic_factory<body_part_type>::obj
Shadow bytes around the buggy address:
  0x024ea63dfb10: 00 00 00 00 00 00 f1 f1 f1 f1 f8 f2 f8 f8 f8 f8
  0x024ea63dfb20: f2 f2 f2 f2 f8 f8 f8 f8 f3 f3 f3 f3 00 00 00 00
  0x024ea63dfb30: 00 00 00 00 00 00 f1 f1 f1 f1 f8 f8 f8 f8 f2 f2
  0x024ea63dfb40: f2 f2 f8 f2 f8 f8 f8 f8 f2 f2 f2 f2 f8 f8 f8 f8
  0x024ea63dfb50: f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
=>0x024ea63dfb60: 00 00 00 00 f1 f1 f1 f1[f8]f2 00 00 00 00 f2 f2
  0x024ea63dfb70: f2 f2 04 f2 04 f2 f8 f2 f8 f2 f8 f2 f8 f2 f8 f2
  0x024ea63dfb80: f8 f3 f3 f3 f3 00 00 00 00 00 00 00 f1 f1 f1 f1
  0x024ea63dfb90: 00 00 00 00 00 f2 f2 f2 f2 00 00 00 00 00 f2 f2
  0x024ea63dfba0: f2 f2 00 00 00 00 00 f2 f2 f2 f2 00 00 00 00 00
  0x024ea63dfbb0: f2 f2 f2 f2 00 00 00 00 00 f2 f2 f2 f2 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
  Shadow gap:              cc
Exception thrown at 0x00007FF756F29D90 in cataclysm-tiles.exe: 0xE0736171: Access violation reading location 0x000004848D624000.
=================================================================
==4164==ERROR: AddressSanitizer:  on address 0x00ff86efdb40 at pc 0x7ff757b01a73 bp 0x00ff86efd890 sp 0x00ff86efd898
READ of size 4 at 0x00ff86efdb40 thread T0
==4164==WARNING: Failed to use and restart external symbolizer!
    #0 0x7ff757b01a72 in generic_factory<body_part_type>::obj C:\Users\Brett\Git\Cataclysm-DDA\src\generic_factory.h:429
    #1 0x7ff75744d00a in Creature::get_part C:\Users\Brett\Git\Cataclysm-DDA\src\creature.cpp:1947
    #2 0x7ff757431eb2 in get_part_helper<int> C:\Users\Brett\Git\Cataclysm-DDA\src\creature.cpp:1959
    #3 0x7ff7573d8c37 in display::temp_text_color C:\Users\Brett\Git\Cataclysm-DDA\src\panels.cpp:713
    #4 0x7ff7573af8e5 in draw_needs_labels C:\Users\Brett\Git\Cataclysm-DDA\src\panels.cpp:1884
    #5 0x7ff757189d7b in game::draw_panels C:\Users\Brett\Git\Cataclysm-DDA\src\game.cpp:3310
    #6 0x7ff757188747 in game::draw C:\Users\Brett\Git\Cataclysm-DDA\src\game.cpp:3270
    #7 0x7ff757282096 in ui_adaptor::redraw_invalidated C:\Users\Brett\Git\Cataclysm-DDA\src\ui_manager.cpp:282
    #8 0x7ff7570baf74 in do_turn C:\Users\Brett\Git\Cataclysm-DDA\src\do_turn.cpp:687
    #9 0x7ff756ef87d8 in WinMain C:\Users\Brett\Git\Cataclysm-DDA\src\main.cpp:711
    #10 0x7ff758e3f7e9 in __scrt_common_main_seh d:\a01\_work\6\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
    #11 0x7ffb16fb7033 in BaseThreadInitThunk+0x13 (C:\Windows\System32\KERNEL32.DLL+0x180017033)
    #12 0x7ffb18102650 in RtlUserThreadStart+0x20 (C:\Windows\SYSTEM32\ntdll.dll+0x180052650)

Address 0x00ff86efdb40 is located in stack of thread T0 at offset 32 in frame
    #0 0x7ff7573d8b2f in display::temp_text_color C:\Users\Brett\Git\Cataclysm-DDA\src\panels.cpp:705

  This frame has 10 object(s):
    [32, 40) 'compiler temporary' <== Memory access at offset 32 is inside this variable
    [48, 80) 'temp_string'
    [64, 68) 'temp_color'
    [80, 84) 'compiler temporary'
    [96, 100) 'compiler temporary'
    [112, 116) 'compiler temporary'
    [128, 132) 'compiler temporary'
    [144, 148) 'compiler temporary'
    [160, 164) 'compiler temporary'
    [176, 180) 'compiler temporary'
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)
SUMMARY: AddressSanitizer: stack-use-after-scope C:\Users\Brett\Git\Cataclysm-DDA\src\generic_factory.h:429 in generic_factory<body_part_type>::obj
Shadow bytes around the buggy address:
  0x024ea63dfb10: 00 00 00 00 00 00 f1 f1 f1 f1 f8 f2 f8 f8 f8 f8
  0x024ea63dfb20: f2 f2 f2 f2 f8 f8 f8 f8 f3 f3 f3 f3 00 00 00 00
  0x024ea63dfb30: 00 00 00 00 00 00 f1 f1 f1 f1 f8 f8 f8 f8 f2 f2
  0x024ea63dfb40: f2 f2 f8 f2 f8 f8 f8 f8 f2 f2 f2 f2 f8 f8 f8 f8
  0x024ea63dfb50: f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
=>0x024ea63dfb60: 00 00 00 00 f1 f1 f1 f1[f8]f2 00 00 00 00 f2 f2
  0x024ea63dfb70: f2 f2 04 f2 04 f2 f8 f2 f8 f2 f8 f2 f8 f2 f8 f2
  0x024ea63dfb80: f8 f3 f3 f3 f3 00 00 00 00 00 00 00 f1 f1 f1 f1
  0x024ea63dfb90: 00 00 00 00 00 f2 f2 f2 f2 00 00 00 00 00 f2 f2
  0x024ea63dfba0: f2 f2 00 00 00 00 00 f2 f2 f2 f2 00 00 00 00 00
  0x024ea63dfbb0: f2 f2 f2 f2 00 00 00 00 00 f2 f2 f2 f2 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
  Shadow gap:              cc
Address Sanitizer Error: Use of out-of-scope stack memory

Describe the solution

Using a const variable rather than a const reference to receive the return value from temp_delta() makes the error disappear.

Describe alternatives you've considered

Testing

Build with Address Sanitizer on Visual Studio, and start a new game. Should not crash with this patch applied.

Additional context

@github-actions github-actions bot added astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Dec 31, 2021
@ZhilkinSerg ZhilkinSerg merged commit c572450 into master Dec 31, 2021
@BrettDong BrettDong deleted the asan branch January 1, 2022 04:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@BrettDong @ZhilkinSerg