This is an archive of the discontinued LLVM Phabricator instance.

sanitizers: Do not include crypt.h if SANITIZER_INTERCEPT_CRYPT_R is undef
ClosedPublic

Authored by thakis on May 31 2022, 6:18 AM.

Details

Summary

sanitizer_intercept_overriders.h might override SANITIZER_INTERCEPT_CRYPT_R to
be undefined. There's no need to require crypt.h in that case.

(The motivation is that crypt() moved from glibc into its own package at some
point, which makes intercepting it and building with a single sysroot that
supports both pre-bullseye and post-bullseye a bit hairy.)

Diff Detail

Event Timeline

thakis created this revision.May 31 2022, 6:18 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 31 2022, 6:18 AM
Herald added a subscriber: Enna1. · View Herald Transcript
thakis requested review of this revision.May 31 2022, 6:18 AM
thakis added inline comments.
compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h
21 ↗(On Diff #433065)

(not sure if adding this include here is proper?)

thakis added inline comments.May 31 2022, 6:32 AM
compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h
21 ↗(On Diff #433065)

Ah hm, looks like it isn't: sanitizer_platform_interceptors.h includes sanitizer_platform_limits_posix.h.

Any suggestions on how prevent the crypt.h include if sanitizer_intercept_overriders.h sets SANITIZER_INTERCEPT_CRYPT_R to 0?

I guess I could keep the struct_crypt_data_sz declaration here and only omit its initialization in sanitizer_platform_limits_posix.cpp? Also a bit iffy, but maybe the best available option.

Looks like pre-merge checks are now fairly happy with this. They come back red only due to clang-format complaints, and those look wrong (I left a comment about that on D100238).

thakis edited the summary of this revision. (Show Details)May 31 2022, 7:54 AM
vitalybuka accepted this revision.May 31 2022, 10:14 AM
This revision is now accepted and ready to land.May 31 2022, 10:14 AM
This revision was landed with ongoing or failed builds.Jun 1 2022, 10:27 AM
This revision was automatically updated to reflect the committed changes.
Herald added a subscriber: Restricted Project. · View Herald TranscriptJun 1 2022, 10:27 AM