Cleanup two ASAN initialization order issues #2330
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Constructing the pacer along with the other SDL conf items fixes an initialization-order-fiasco caught by ASAN. The issue is that the pacer tracks elapsed time and takes a measurement at construction time - however, because it (was previously) a global, it was reading timer values before they were assigned.
Constructing the mouse interfaces during the mouse init phase fixes an initialization-order-fiasco caught by ASAN: ==1147016==ERROR: AddressSanitizer: initialization-order-fiasco on address 0x559608682e88 at pc 0x559602ee8108 bp 0x7ffc4c848510 sp 0x7ffc4c848500 READ of size 8 at 0x559608682e88 thread T0 An initialization race previously occured: the interfaces were previously being initializaed before the MouseInfo and MouseShared objects (both also globals) members, however the interfaces need to use their members. Test with ASAN ~~~~~~~~~~~~~~ Setup: meson setup -Dbuildtype=debug -Db_sanitize=address build/asan Compile: meson compile -C build/asan Binary: build/asan/dosbox Run with: export ASAN_OPTIONS=strict_string_checks=1:detect_stack_use_after_return=1:check_initialization_order=1:strict_init_order=1:detect_leaks=1
kcgen
added
the
cleanup
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Render pacer
Previously the pacer was a global object initialized on startup, however it accessed timer values even before they had a chance to initialize:
The first commit moves the pacer's construction to when its conf setting is parsed.
Mouse interface objects
Similar to the pacer, previously the mouse interfaces were initialized on startup, however they accessed members of the two mouse state variables (
mouse_infoandmouse_shared) even before they had a chance to initialize:The second commit moves the interface construction into the mouse interface initialization function.
In both commits, the objects are now managed using
std::unique_ptrs.