[fix](build) Let ScopedLSANDisabler compile without a sanitizer#66049
Open
airborne12 wants to merge 1 commit into
Open
[fix](build) Let ScopedLSANDisabler compile without a sanitizer#66049airborne12 wants to merge 1 commit into
airborne12 wants to merge 1 commit into
Conversation
leak_annotations.h computes DORIS_LSAN_ENABLED and guards ANNOTATE_LEAKING_OBJECT_PTR with it, but
ScopedLSANDisabler sits outside every #if and calls __lsan_disable() / __lsan_enable()
unconditionally. Those are only declared in this header; the LeakSanitizer runtime defines them. So
without -fsanitize=address every translation unit that instantiates the class emitted an undefined
reference:
ld.lld: error: undefined symbol: __lsan_disable
>>> referenced by leak_annotations.h:84
>>> cached_remote_file_reader_peer_test.cpp.o
Two instantiate it -- leakcheck_disabler.h and cached_remote_file_reader_peer_test.cpp -- so
doris_be_test could only ever be linked as an ASAN build. BUILD_TYPE_UT=DEBUG and =RELEASE both
failed at link, which also means there was no way to get an optimized un-instrumented test binary,
and unit tests could not be used for any timing work.
The guard now covers the class. With no LeakSanitizer there is nothing to suppress, so it becomes a
no-op, matching what ANNOTATE_LEAKING_OBJECT_PTR already does. The no-op constructor is written out
rather than defaulted on purpose: `= default` makes the type trivial and every
`ScopedLSANDisabler guard;` then trips -Werror,-Wunused-variable.
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Member
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 29447 ms |
Contributor
TPC-DS: Total hot run time: 178215 ms |
Contributor
ClickBench: Total hot run time: 24.87 s |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
doris_be_testcan only be linked as an ASAN build.BUILD_TYPE_UT=DEBUGandBUILD_TYPE_UT=RELEASEboth fail at link:
be/src/util/debug/leak_annotations.hcomputesDORIS_LSAN_ENABLEDand uses it to guardANNOTATE_LEAKING_OBJECT_PTR, butScopedLSANDisablersits outside every#if:__lsan_disable/__lsan_enableare only declared in that header; they are defined by theLeakSanitizer runtime. Without
-fsanitize=address(or=leak) nothing provides them, so everytranslation unit that instantiates the class emits an undefined reference. Two do —
be/src/util/debug/leakcheck_disabler.handbe/test/io/cache/cached_remote_file_reader_peer_test.cpp.This matters beyond tidiness: it means there is no way to produce an optimized, un-instrumented unit
test binary, so unit tests cannot be used for any timing work — ASAN's allocator instrumentation and
-O0dominate whatever is being measured.The guard is extended to cover the class. With no LeakSanitizer there is nothing to suppress, so it
degrades to a no-op, which is the same reasoning
ANNOTATE_LEAKING_OBJECT_PTRalready applies.Release note
None
Check List (For Author)
Test
Regression test
Unit Test
Manual test (add detailed scripts or steps below)
Compiled a translation unit that instantiates
ScopedLSANDisablerboth ways and inspected theundefined symbols, which isolates the change from everything else in the binary:
Then built the unit test binary with
BUILD_TYPE_UT=RELEASE sh run-be-ut.sh. On master itfails with the undefined
__lsan_disable; with this change all translation units compile andthe
-O3 -DNDEBUGbinary links.Note the no-op constructor is written out rather than
= default: a defaulted constructormakes the type trivial, and the RELEASE build then fails with
error: unused variable 'lsan_disabler' [-Werror,-Wunused-variable]at the two use sites.No need to test or manual test. Explain why:
Behavior changed:
No.
Yes.
Under a sanitizer build nothing changes — the calls are still emitted. Without a sanitizer the
scope guard becomes a no-op instead of failing to link, which is the only behaviour it can
meaningfully have when there is no LeakSanitizer runtime to talk to.
Does this need documentation?
Check List (For Reviewer who merge this PR)