Page MenuHomePhabricator

[Sanitizers] Remove OpenBSD support
ClosedPublic

Authored by devnexen on Sat, Oct 17, 10:02 PM.

Details

Summary

Removing unused and unusable code.

Diff Detail

Event Timeline

devnexen created this revision.Sat, Oct 17, 10:02 PM
Herald added a project: Restricted Project. · View Herald TranscriptSat, Oct 17, 10:02 PM
Herald added subscribers: Restricted Project, phosek, fedor.sergeev, mgorny. · View Herald Transcript
devnexen requested review of this revision.Sat, Oct 17, 10:02 PM

Please leave builtins intact. ubsan might work, but removing it too is fine.

devnexen updated this revision to Diff 298873.Sun, Oct 18, 6:43 AM

bring back builtins part

krytarowski accepted this revision.Sun, Oct 18, 6:46 AM
This revision is now accepted and ready to land.Sun, Oct 18, 6:46 AM
krytarowski added inline comments.Sun, Oct 18, 6:53 AM
compiler-rt/cmake/base-config-ix.cmake
178

remove this endif()

This revision was automatically updated to reflect the committed changes.
hans added a subscriber: hans.Mon, Oct 19, 5:40 AM
compiler-rt/cmake/base-config-ix.cmake
183

Hang on, you moved the code below from the else-branch of "if not msvc" to the if-branch, effectively flipping the condition under which it's used.

bjope added a subscriber: bjope.Mon, Oct 19, 11:01 AM
bjope added inline comments.
compiler-rt/cmake/base-config-ix.cmake
183

@hans I still don't quite get it given the fix. The fixup you made restores the behavior for "MSVC", but for "NOT MSVC" and non OpenBSD we no longer do lines 180 and 181. So for non-MSVC and non-OpenBSD we still got a problem, or am I missing something?

hans added inline comments.Mon, Oct 19, 11:36 AM
compiler-rt/cmake/base-config-ix.cmake
183

Oh, I assumed the "not msvc" part was deleted for a reason. I was just focusing on the msvc part.

Looking closer, you might be right, this looks like it might be deleting a bit too much...?

dmajor added a subscriber: dmajor.Mon, Oct 19, 12:57 PM
dmajor added inline comments.
compiler-rt/cmake/base-config-ix.cmake
183

Our Linux bots are also complaining about the fixup commit. Looks like non-MSVC no longer does any form of test_target_arch, but it still should?

krytarowski added inline comments.Mon, Oct 19, 1:24 PM
compiler-rt/cmake/base-config-ix.cmake
183

Yes, it looks like deleting a little bit too much. I've milooked this indentation in review.

eugenis added inline comments.
compiler-rt/cmake/base-config-ix.cmake
183

This change disabled _everything_ on x86_64 Linux.
I'll revert.