Page MenuHomePhabricator

[sanitizer_common] Enable trace pc guard coverage test on PPC64/s390x/MIPS
ClosedPublic

Authored by XiaodongLoong on Feb 24 2022, 4:02 AM.

Diff Detail

Event Timeline

XiaodongLoong created this revision.Feb 24 2022, 4:02 AM
XiaodongLoong requested review of this revision.Feb 24 2022, 4:02 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 24 2022, 4:02 AM
Herald added a subscriber: Restricted Project. · View Herald Transcript
MaskRay accepted this revision.Feb 24 2022, 10:21 AM

Thanks!

compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_trace_pc_guard-dso.cpp
3

I assume that this REQUIRE can be removed as well, but I haven't tested.

This revision is now accepted and ready to land.Feb 24 2022, 10:21 AM

delete the stable-runtime REQUIRES

XiaodongLoong added inline comments.Feb 24 2022, 5:23 PM
compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_trace_pc_guard-dso.cpp
3

Thanks! I deleted it. It is OK now.

This revision was landed with ongoing or failed builds.Feb 24 2022, 5:43 PM
This revision was automatically updated to reflect the committed changes.
ro added a comment.Feb 25 2022, 1:30 AM

These test now all seem to fail on s390x:
https://lab.llvm.org/buildbot/#/builders/94/builds/7962

Could this be related to clang not implementing __builtin_extract_return_address, unlike gcc? Cf. D91608 and D91607 for the SPARC equivalent.

@uweigand @ro
I found the following logs, the test failure might caused by none __sanitizer_cov* functions on s390x.

sanitizer_coverage_trace_pc_guard-dso.cpp:71:18: error: CHECK-SANCOV: expected string not found in input
// CHECK-SANCOV: Ignoring {{.*}}_1.so and its coverage because __sanitizer_cov* functions were not found.

@uweigand @ro
I found the following logs, the test failure might caused by none __sanitizer_cov* functions on s390x.

sanitizer_coverage_trace_pc_guard-dso.cpp:71:18: error: CHECK-SANCOV: expected string not found in input
// CHECK-SANCOV: Ignoring {{.*}}_1.so and its coverage because __sanitizer_cov* functions were not found.

Reproducing the failure locally, the output is:

[uweigand@t35lp56 sanitizer_coverage_trace_pc_guard-dso.cpp.tmp_workdir]$ /home/uweigand/llvm/build/llvm-head/./bin/sancov -covered-functions -strip_path_prefix=TestCases/ *.sancov             /home/uweigand/llvm/build/llvm-head/projects/compiler-rt/test/sanitizer_common/asan-s390x-Linux/Output/sanitizer_coverage_trace_pc_guard-dso.cpp.tmp /home/uweigand/llvm/build/llvm-head/projects/compiler-rt/test/sanitizer_common/asan-s390x-Linux/Output/sanitizer_coverage_trace_pc_guard-dso.cpp.tmp_1.so /home/uweigand/llvm/build/llvm-head/projects/compiler-rt/test/sanitizer_common/asan-s390x-Linux/Output/sanitizer_coverage_trace_pc_guard-dso.cpp.tmp_2.so 
WARNING: No coverage file for /home/uweigand/llvm/build/llvm-head/projects/compiler-rt/test/sanitizer_common/asan-s390x-Linux/Output/sanitizer_coverage_trace_pc_guard-dso.cpp.tmp
WARNING: No coverage file for sanitizer_coverage_trace_pc_guard-dso.cpp.tmp.2747370.sancov
WARNING: No coverage file for /home/uweigand/llvm/build/llvm-head/projects/compiler-rt/test/sanitizer_common/asan-s390x-Linux/Output/sanitizer_coverage_trace_pc_guard-dso.cpp.tmp_1.so
WARNING: No coverage file for sanitizer_coverage_trace_pc_guard-dso.cpp.tmp_1.so.2747370.sancov
WARNING: No coverage file for /home/uweigand/llvm/build/llvm-head/projects/compiler-rt/test/sanitizer_common/asan-s390x-Linux/Output/sanitizer_coverage_trace_pc_guard-dso.cpp.tmp_2.so
WARNING: No coverage file for sanitizer_coverage_trace_pc_guard-dso.cpp.tmp_2.so.2747370.sancov
ERROR: No valid coverage files given.

So it seems it doesn't even recognize the .sancov files as coverage files. Not sure if they were written incorrectly or if this is a problem on the reader side ...

In any case, I think this should remain XFAILed until the underlying problem is fixed, so get the CI green again.

There is already a fix,

In any case, I think this should remain XFAILed until the underlying problem is fixed, so get the CI green again.

There is already a fix, https://reviews.llvm.org/D120541

do you think it is OK ? I will commit it if you accept it.

There is already a fix,

In any case, I think this should remain XFAILed until the underlying problem is fixed, so get the CI green again.

There is already a fix, https://reviews.llvm.org/D120541

do you think it is OK ? I will commit it if you accept it.

Yes, this LGTM, thanks!

So it seems it doesn't even recognize the .sancov files as coverage files. Not sure if they were written incorrectly or if this is a problem on the reader side ...

Looks like an endian problem. The docs say:

The format of ``*.sancov`` files is very simple: the first 8 bytes is the magic,
one of ``0xC0BFFFFFFFFFFF64`` and ``0xC0BFFFFFFFFFFF32``. The last byte of the
magic defines the size of the following offsets. The rest of the data is the
offsets in the corresponding binary/DSO that were executed during the run.

However, code in llvm/tools/sancov/sancov.cpp interprets the first 8 bytes as two 4-byte integers:

struct FileHeader {
  uint32_t Bitness;
  uint32_t Magic;
};

static const uint32_t BinCoverageMagic = 0xC0BFFFFF;
static const uint32_t Bitness32 = 0xFFFFFF32;
static const uint32_t Bitness64 = 0xFFFFFF64;

but of course on a big-endian system this gets the order of Bitness vs. Magic wrong:

(gdb) print/x *Header
$6 = {Bitness = 0xc0bfffff, Magic = 0xffffff64}

Should be easy enough to fix for native files, but I'm wondering whether the intent was that those files should be portable across architectures? In that case, more fundamental changes would need to be made (endian-aware accesses everywhere) ...

I've disabled these tests on Arm Thumb as well: https://reviews.llvm.org/rGee95fe5caa3c

Though the failure is different I think.

ERROR: Coverage points in binary and .sancov file do not match.

Random guess, Thumb's mode bit being set on one of the pairs of PC value is causing that.

On further inspection on Thumb we do get some results:

$ xxd *.sancov
00000000: 32ff ffff ffff bfc0 ee53 0300 1854 0300  2........S...T..

The issue is how sancov finds the calls to __sanitizer_cov_trace_pc_guard. The triple in getObjectCoveragePoints defaults to arm-unknown-unknown so any thumb instruction fails to disassemble. So by the time we get to main we're way out of sync. Forcing thumb-unknown-unknown fixes that (obviously you couldn't just do that but to demo) but then the PCs recorded in the file are still off:

Coverage points:
size: 2
0x000353ec
0x00035416
Data points:
0x000353ee
0x00035418

Taking one of them:

35412:       f2c0 0005       movt    r0, #5
35416:       f7f0 f803       bl      25420 <__sanitizer_cov_trace_pc_guard>
3541a:       f642 3000       movw    r0, #11008      ; 0x2b00

You can see that sancov found the call in the binary correctly but it was recorded as being in the middle of the bl. Perhaps this is just an unconditional "add/substract the size of one instruction" somewhere, that doesn't account for 16 and 32 bit encodings.

Anyway, I'm happy to leave it disabled for now but let me know if this is supposed to work at all for Thumb. I can at least put this info in an issue for future reference.