This is an archive of the discontinued LLVM Phabricator instance.

[sanitizer] Test ubsan and cfi on android.
ClosedPublic

Authored by eugenis on Oct 5 2017, 5:19 PM.

Details

Summary

Enable check-cfi and check-ubsan on Android.
Check-ubsan includes standalone and ubsan+asan, but not tsan or msan.
Cross-dso cfi tests are disabled for now.

Diff Detail

Repository
rL LLVM

Event Timeline

eugenis created this revision.Oct 5 2017, 5:19 PM
pcc edited edge metadata.Oct 5 2017, 5:48 PM

CFI parts seem reasonable.

compiler-rt/test/cfi/stats.cpp
7 ↗(On Diff #117937)

Do you know why it fails?

compiler-rt/test/ubsan/TestCases/TypeCheck/Function/function.cpp
6 ↗(On Diff #117937)

Presumably this fails when targeting ARM?

Should we instead adjust this to check target instead of host?
http://llvm-cs.pcc.me.uk/projects/compiler-rt/test/ubsan/TestCases/TypeCheck/Function/lit.local.cfg

eugenis added inline comments.Oct 5 2017, 5:51 PM
compiler-rt/test/cfi/stats.cpp
7 ↗(On Diff #117937)

Because sanstats input is not transferred back to host :) It's a general issue in lit tests, I'd assume more than half cases of "UNSUPPORTED: android" are caused by this or a related problem.

compiler-rt/test/ubsan/TestCases/TypeCheck/Function/function.cpp
6 ↗(On Diff #117937)

Oh right, it's not about Android. Function sanitizer is not supported on ARM.

pcc added inline comments.Oct 5 2017, 6:04 PM
compiler-rt/test/cfi/stats.cpp
7 ↗(On Diff #117937)

Okay, please mention that in a comment then.

eugenis updated this revision to Diff 117940.Oct 5 2017, 6:04 PM

disable function sanitizer by target_arch instead of host_arch

eugenis updated this revision to Diff 117941.Oct 5 2017, 6:05 PM

FIXME in stats.cpp

eugenis marked an inline comment as done.Oct 5 2017, 6:06 PM
pcc accepted this revision.Oct 5 2017, 6:11 PM

LGTM

This revision is now accepted and ready to land.Oct 5 2017, 6:11 PM
vitalybuka added inline comments.Oct 5 2017, 6:11 PM
compiler-rt/test/cfi/stats.cpp
9 ↗(On Diff #117941)

I guess XFAIL is preferable for fixable issues.
UNSUPPORTED may stay this way even If someone accidentally fix the issue for other reasons

compiler-rt/test/ubsan/lit.common.cfg
50 ↗(On Diff #117941)

Could we put this into top cfg?

eugenis updated this revision to Diff 118062.Oct 6 2017, 1:25 PM

UNSUPPORTED -> XFAIL

eugenis added inline comments.Oct 6 2017, 1:25 PM
compiler-rt/test/ubsan/lit.common.cfg
50 ↗(On Diff #117941)

Yes, but I'd like to do it in a separate change.

vitalybuka accepted this revision.Oct 6 2017, 1:51 PM
This revision was automatically updated to reflect the committed changes.