Fix LSan by fixing getauxval() (resubmit v4) #39430
Merged
Conversation
This file contains 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
robot-ch-test-poll2
added
the
pr-not-for-changelog
azat
changed the title
azat
changed the title
azat
changed the title
robot-ch-test-poll2
added
pr-build
rschu1ze
approved these changes
Jul 21, 2022
Not interesting failure for this PR |
rschu1ze
approved these changes
Jul 22, 2022
(cherry picked from commit 51e7c41) v2: fix type check Signed-off-by: Azat Khuzhin <[email protected]>
getauxval() from glibc-compatibility did not work always correctly: - It does not work after setenv(), and this breaks vsyscalls, like sched_getcpu() [1] (and BaseDaemon.cpp always set TZ if timezone is defined, which is true for CI [2]). Also note, that fixing setenv() will not fix LSan, since the culprit is getauxval() [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1163404 [2]: ClickHouse#32928 (comment) - Another think that is definitely broken is LSan (Leak Sanitizer), it relies on worked getauxval() but it does not work if __environ is not initialized yet (there is even a commit about this). And because of, at least, one leak had been introduced [3]: [3]: ClickHouse#33840 Fix this by using /proc/self/auxv with fallback to environ solution to make it compatible with environment that does not allow reading from auxv (or no procfs). v2: add fallback to environ solution v3: fix return value for __auxv_init_procfs() (cherry picked from commit f187c34) v4: more verbose message on errors, CI founds [1]: AUXV already has value (529267711) [1]: https://s3.amazonaws.com/clickhouse-test-reports/39103/2325f7e8442d1672ce5fb43b11039b6a8937e298/stress_test__memory__actions_.html v5: break at AT_NULL v6: ignore AT_IGNORE v7: suppress TSan and remove superior check to avoid abort() in case of race v8: proper suppressions (not inner function but itself) Refs: ClickHouse#33957 Signed-off-by: Azat Khuzhin <[email protected]>
This was referenced Jun 13, 2024
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.
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Fix LSan by fixing getauxval()
getauxval() from glibc-compatibility did not work always correctly:
it does not work after setenv(), and this breaks vsyscalls,
like sched_getcpu() 1 (and BaseDaemon.cpp always set TZ if timezone
is defined, which is true for CI 2).
another think that is definitely broken is LSan (Leak Sanitizer), it
relies on worked getauxval() but it does not work if __environ is not
initialized yet (there is even a commit about this).
Fix this by using /proc/self/auxv with fallback to environ solution to
make it compatible with environment that does not allow reading from
auxv (or no procfs), since previous attempt w/o it failed - #33957 (comment)
Fixes: #39299 (cc @rschu1ze )
Fixes: #39452
Note: I have few patches on top, but I want to see this error again.