This is an archive of the discontinued LLVM Phabricator instance.

Fix gcov profiling on big-endian machines
ClosedPublic

Authored by uweigand on Jul 10 2018, 7:12 AM.

Details

Summary

The compiler-rt library support for gcov is currently only working correctly on little-endian systems.

Two separate fixes are required to handle big-endian systems as well:

  • 64-bit counter values are stored in a mixed-endian format in the gcov files: a 32-bit low-part followed by a 32-bit high part. Note that this is already implemented correctly on the LLVM side, see GCOVBuffer::readInt64.
  • The tag values (e.g. arcs tag, object summary tag, ...) are aways written as the same sequence of bytes independent of byte order. But when *reading* them back in, the code reads them as 32-bit values in host byte order. For the comparisons to work correctly, this should instead always read them as little-endian values.

Diff Detail

Repository
rL LLVM

Event Timeline

uweigand created this revision.Jul 10 2018, 7:12 AM
marco-c accepted this revision.Jul 10 2018, 7:37 AM

I've just relanded https://reviews.llvm.org/D48538 in r336678, once the issues which made the tests fail are fixed we should remove the XFAILs. I've filed https://bugs.llvm.org/show_bug.cgi?id=38121 to track it.

This revision is now accepted and ready to land.Jul 10 2018, 7:37 AM
This revision was automatically updated to reflect the committed changes.

This caused a build bot failure here:
http://lab.llvm.org:8011/builders/clang-x86_64-linux-abi-test/builds/28963

Apparently this file may not use C99 syntax "for (int i = ...". I've checked in the obvious fix as r336706.

This change is wrong. I can understand the intention: llvm-cov does not process big-endian .gcno/.gcda correctly.
gcov reads/writes integral values in the executable's endianness. On a big-endian machine, the values are stored in big-endian. This change is not going to work with gcov shipped by GCC.

I am fixing some gcov problems. We should just UNSUPPORTED: host-byteorder-big-endian until llvm-cov and compiler-rt/lib/profile are actually fixed for big endian.

davidxl added a reviewer: vsk.May 11 2020, 8:24 PM

Hmm, I still think the read/write_64bit_value change is correct, GCC does read/write 64-bit counter values as lo/hi pairs even on big-endian systems.

However, it does appear that you're right about the read_le_32bit_value change; I've added this for the tags checks, but this looks wrong. GCC does read/write those in native endian byte order. I guess this part of the change should be reverted, and instead the code that writes tags should be changed to use write_32bit_value instead of write_bytes.