Page MenuHomePhabricator

Fix qsort() interceptor for FreeBSD

Authored by arichardson on Jul 24 2020, 3:55 AM.



When the FreeBSD qsort() implementation recurses, it does so using an
interposable function call, so we end up calling the interceptor again
and set the saved comparator to wrapped_qsort_compar. This results in an
infinite loop and a eventually a stack overflow since wrapped_qsort_compar
ends up calling itself. This means that ASAN is completely broken on
FreeBSD for programs that call qsort(). I found this while running
check-all on a FreeBSD system a ASAN-instrumented LLVM.

Fix this by checking whether we are recursing inside qsort before writing
to qsort_compar. The same bug exists in the qsort_r interceptor, so use the
same approach there. I did not test the latter since the qsort_r function
signature does not match and therefore it's not intercepted on FreeBSD/macOS.


Diff Detail

Event Timeline

arichardson created this revision.Jul 24 2020, 3:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 24 2020, 3:55 AM
Herald added subscribers: Restricted Project, krytarowski, emaste. · View Herald Transcript
eugenis added inline comments.Jul 24 2020, 9:42 AM

I think we'd still want to save/restore the size argument, in the likely case when the recursive call is passed a different value.

arichardson marked an inline comment as done.Jul 26 2020, 3:41 AM
arichardson added inline comments.

The size argument is the size of the elements which should not change in recursive calls. I could add an assertion/warning?

vitalybuka added inline comments.Jul 27 2020, 6:28 PM

CHECK_EQ would be nice

eugenis accepted this revision.Jul 28 2020, 4:52 PM
eugenis added inline comments.

Yes, I misunderstood the code.
LGTM with the check.

This revision is now accepted and ready to land.Jul 28 2020, 4:52 PM
  • Add CHECK()
  • Add new test
Hang added a subscriber: Hang.Aug 4 2020, 11:03 PM
Hang added inline comments.

I think compar != qsort_compar here as compar == wrapped_sort_compar in this situation.

  • Fix inverted condition
  • Make arrays in the test case large enough to actually trigger the recursive case
eugenis accepted this revision.Aug 5 2020, 10:32 AM


This revision was landed with ongoing or failed builds.Aug 6 2020, 1:17 AM
This revision was automatically updated to reflect the committed changes.