This is an archive of the discontinued LLVM Phabricator instance.

[sanitizer] No THREADLOCAL in qsort and bsearch
ClosedPublic

Authored by vitalybuka on Aug 26 2021, 1:31 AM.

Details

Summary

qsort can reuse qsort_r if available.
bsearch always passes key as the first comparator argument, so we
can use it to wrap the original comparator.

Diff Detail

Event Timeline

vitalybuka requested review of this revision.Aug 26 2021, 1:31 AM
vitalybuka created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptAug 26 2021, 1:31 AM
Herald added a subscriber: Restricted Project. · View Herald Transcript

this looks good. Too good, even. Why did not we do it before?

compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h
573

I'd rather place this logic in the .inc file - i.e. define the thread-local qsort interceptor if SANITIZER_INTERCEPT_QSORT && !SANITIZER_INTERCEPT_QSORT_R

move !SANITIZER_INTERCEPT_QSORT_R

vitalybuka marked an inline comment as done.

fix build

clang-format

eugenis added inline comments.Aug 26 2021, 3:51 PM
compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
9952

do you need a !qsort_r check here to avoid defining two interceptors for qsort?

9953

Wouldn't it be easier to call the qsort_r interceptor directly?

9955

Let's avoid nested ifdef-else-endif-else-endif?
#if SANITIZER_INTERCEPT_QSORT && SANITIZER_INTERCEPT_QSORT_R

vitalybuka marked 3 inline comments as done.

cleanup

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
9952

it was not needed with nested ifs

restore new line

eugenis accepted this revision.Aug 26 2021, 4:12 PM

LGTM

This revision is now accepted and ready to land.Aug 26 2021, 4:12 PM

simplify ifs

This revision was landed with ongoing or failed builds.Aug 26 2021, 4:55 PM
This revision was automatically updated to reflect the committed changes.