[llvm] r232346 - InstrProf: Do a better job of reading coverage mapping data.
Justin Bogner
mail at justinbogner.com
Sun Mar 15 23:55:46 PDT 2015
Author: bogner
Date: Mon Mar 16 01:55:45 2015
New Revision: 232346
URL: http://llvm.org/viewvc/llvm-project?rev=232346&view=rev
Log:
InstrProf: Do a better job of reading coverage mapping data.
This code was casting regions of a memory buffer to a couple of
different structs. This is wrong in a few ways:
1. It breaks aliasing rules.
2. If the buffer isn't aligned, it hits undefined behaviour.
3. It completely ignores endianness differences.
4. The structs being defined for this aren't specifying their padding
properly, so this doesn't even represent the data properly on some
platforms.
This commit is mostly NFC, except that it fixes reading coverage for
32 bit binaries as a side effect of getting rid of the mispadded
structs. I've included a test for that.
I've also baked in that we only handle little endian more explicitly,
since that was true in practice already. I'll fix this to handle
endianness properly in a followup commit.
Added:
llvm/trunk/test/tools/llvm-cov/Inputs/binary-formats.macho32l (with props)
llvm/trunk/test/tools/llvm-cov/Inputs/binary-formats.macho64l (with props)
llvm/trunk/test/tools/llvm-cov/Inputs/binary-formats.proftext
llvm/trunk/test/tools/llvm-cov/binary-formats.c
Modified:
llvm/trunk/include/llvm/Support/Endian.h
llvm/trunk/lib/ProfileData/CoverageMappingReader.cpp
llvm/trunk/test/tools/llvm-cov/lit.local.cfg
Modified: llvm/trunk/include/llvm/Support/Endian.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/Endian.h?rev=232346&r1=232345&r2=232346&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/Endian.h (original)
+++ llvm/trunk/include/llvm/Support/Endian.h Mon Mar 16 01:55:45 2015
@@ -58,8 +58,9 @@ inline value_type read(const void *memor
/// Read a value of a particular endianness from a buffer, and increment the
/// buffer past that value.
-template<typename value_type, endianness endian, std::size_t alignment>
-inline value_type readNext(const unsigned char *&memory) {
+template<typename value_type, endianness endian, std::size_t alignment,
+ typename CharT>
+inline value_type readNext(const CharT *&memory) {
value_type ret = read<value_type, endian, alignment>(memory);
memory += sizeof(value_type);
return ret;
Modified: llvm/trunk/lib/ProfileData/CoverageMappingReader.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ProfileData/CoverageMappingReader.cpp?rev=232346&r1=232345&r2=232346&view=diff
==============================================================================
--- llvm/trunk/lib/ProfileData/CoverageMappingReader.cpp (original)
+++ llvm/trunk/lib/ProfileData/CoverageMappingReader.cpp Mon Mar 16 01:55:45 2015
@@ -17,6 +17,7 @@
#include "llvm/Object/MachOUniversal.h"
#include "llvm/Object/ObjectFile.h"
#include "llvm/Support/Debug.h"
+#include "llvm/Support/Endian.h"
#include "llvm/Support/LEB128.h"
using namespace llvm;
@@ -288,24 +289,6 @@ std::error_code RawCoverageMappingReader
}
namespace {
-/// \brief The coverage mapping data for a single function.
-/// It points to the function's name.
-template <typename IntPtrT> struct CoverageMappingFunctionRecord {
- IntPtrT FunctionNamePtr;
- uint32_t FunctionNameSize;
- uint32_t CoverageMappingSize;
- uint64_t FunctionHash;
-};
-
-/// \brief The coverage mapping data for a single translation unit.
-/// It points to the array of function coverage mapping records and the encoded
-/// filenames array.
-template <typename IntPtrT> struct CoverageMappingTURecord {
- uint32_t FunctionRecordsSize;
- uint32_t FilenamesSize;
- uint32_t CoverageMappingsSize;
- uint32_t Version;
-};
/// \brief A helper structure to access the data from a section
/// in an object file.
@@ -337,72 +320,71 @@ std::error_code readCoverageMappingData(
SectionData &ProfileNames, StringRef Data,
std::vector<BinaryCoverageReader::ProfileMappingRecord> &Records,
std::vector<StringRef> &Filenames) {
+ using namespace support;
llvm::DenseSet<T> UniqueFunctionMappingData;
// Read the records in the coverage data section.
- while (!Data.empty()) {
- if (Data.size() < sizeof(CoverageMappingTURecord<T>))
+ for (const char *Buf = Data.data(), *End = Buf + Data.size(); Buf < End;) {
+ if (Buf + 4 * sizeof(uint32_t) > End)
return instrprof_error::malformed;
- auto TU = reinterpret_cast<const CoverageMappingTURecord<T> *>(Data.data());
- Data = Data.substr(sizeof(CoverageMappingTURecord<T>));
- switch (TU->Version) {
+ uint32_t NRecords = endian::readNext<uint32_t, little, unaligned>(Buf);
+ uint32_t FilenamesSize = endian::readNext<uint32_t, little, unaligned>(Buf);
+ uint32_t CoverageSize = endian::readNext<uint32_t, little, unaligned>(Buf);
+ uint32_t Version = endian::readNext<uint32_t, little, unaligned>(Buf);
+
+ switch (Version) {
case CoverageMappingVersion1:
break;
default:
return instrprof_error::unsupported_version;
}
- auto Version = CoverageMappingVersion(TU->Version);
- // Get the function records.
- auto FunctionRecords =
- reinterpret_cast<const CoverageMappingFunctionRecord<T> *>(Data.data());
- if (Data.size() <
- sizeof(CoverageMappingFunctionRecord<T>) * TU->FunctionRecordsSize)
- return instrprof_error::malformed;
- Data = Data.substr(sizeof(CoverageMappingFunctionRecord<T>) *
- TU->FunctionRecordsSize);
+ // Skip past the function records, saving the start and end for later.
+ const char *FunBuf = Buf;
+ Buf += NRecords * (sizeof(T) + 2 * sizeof(uint32_t) + sizeof(uint64_t));
+ const char *FunEnd = Buf;
// Get the filenames.
- if (Data.size() < TU->FilenamesSize)
+ if (Buf + FilenamesSize > End)
return instrprof_error::malformed;
- auto RawFilenames = Data.substr(0, TU->FilenamesSize);
- Data = Data.substr(TU->FilenamesSize);
size_t FilenamesBegin = Filenames.size();
- RawCoverageFilenamesReader Reader(RawFilenames, Filenames);
+ RawCoverageFilenamesReader Reader(StringRef(Buf, FilenamesSize), Filenames);
if (auto Err = Reader.read())
return Err;
+ Buf += FilenamesSize;
- // Get the coverage mappings.
- if (Data.size() < TU->CoverageMappingsSize)
+ // We'll read the coverage mapping records in the loop below.
+ const char *CovBuf = Buf;
+ Buf += CoverageSize;
+ const char *CovEnd = Buf;
+ if (Buf > End)
return instrprof_error::malformed;
- auto CoverageMappings = Data.substr(0, TU->CoverageMappingsSize);
- Data = Data.substr(TU->CoverageMappingsSize);
- for (unsigned I = 0; I < TU->FunctionRecordsSize; ++I) {
- auto &MappingRecord = FunctionRecords[I];
+ while (FunBuf < FunEnd) {
+ // Read the function information
+ T NamePtr = endian::readNext<T, little, unaligned>(FunBuf);
+ uint32_t NameSize = endian::readNext<uint32_t, little, unaligned>(FunBuf);
+ uint32_t DataSize = endian::readNext<uint32_t, little, unaligned>(FunBuf);
+ uint64_t FuncHash = endian::readNext<uint64_t, little, unaligned>(FunBuf);
- // Get the coverage mapping.
- if (CoverageMappings.size() < MappingRecord.CoverageMappingSize)
+ // Now use that to read the coverage data.
+ if (CovBuf + DataSize > CovEnd)
return instrprof_error::malformed;
- auto Mapping =
- CoverageMappings.substr(0, MappingRecord.CoverageMappingSize);
- CoverageMappings =
- CoverageMappings.substr(MappingRecord.CoverageMappingSize);
+ auto Mapping = StringRef(CovBuf, DataSize);
+ CovBuf += DataSize;
// Ignore this record if we already have a record that points to the same
- // function name.
- // This is useful to ignore the redundant records for the functions
- // with ODR linkage.
- if (!UniqueFunctionMappingData.insert(MappingRecord.FunctionNamePtr)
- .second)
+ // function name. This is useful to ignore the redundant records for the
+ // functions with ODR linkage.
+ if (!UniqueFunctionMappingData.insert(NamePtr).second)
continue;
- StringRef FunctionName;
- if (auto Err =
- ProfileNames.get(MappingRecord.FunctionNamePtr,
- MappingRecord.FunctionNameSize, FunctionName))
- return Err;
+
+ // Finally, grab the name and create a record.
+ StringRef FuncName;
+ if (std::error_code EC = ProfileNames.get(NamePtr, NameSize, FuncName))
+ return EC;
Records.push_back(BinaryCoverageReader::ProfileMappingRecord(
- Version, FunctionName, MappingRecord.FunctionHash, Mapping,
+ CoverageMappingVersion(Version), FuncName, FuncHash, Mapping,
FilenamesBegin, Filenames.size() - FilenamesBegin));
}
}
Added: llvm/trunk/test/tools/llvm-cov/Inputs/binary-formats.macho32l
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-cov/Inputs/binary-formats.macho32l?rev=232346&view=auto
==============================================================================
Binary files llvm/trunk/test/tools/llvm-cov/Inputs/binary-formats.macho32l (added) and llvm/trunk/test/tools/llvm-cov/Inputs/binary-formats.macho32l Mon Mar 16 01:55:45 2015 differ
Propchange: llvm/trunk/test/tools/llvm-cov/Inputs/binary-formats.macho32l
------------------------------------------------------------------------------
svn:executable = *
Added: llvm/trunk/test/tools/llvm-cov/Inputs/binary-formats.macho64l
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-cov/Inputs/binary-formats.macho64l?rev=232346&view=auto
==============================================================================
Binary files llvm/trunk/test/tools/llvm-cov/Inputs/binary-formats.macho64l (added) and llvm/trunk/test/tools/llvm-cov/Inputs/binary-formats.macho64l Mon Mar 16 01:55:45 2015 differ
Propchange: llvm/trunk/test/tools/llvm-cov/Inputs/binary-formats.macho64l
------------------------------------------------------------------------------
svn:executable = *
Added: llvm/trunk/test/tools/llvm-cov/Inputs/binary-formats.proftext
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-cov/Inputs/binary-formats.proftext?rev=232346&view=auto
==============================================================================
--- llvm/trunk/test/tools/llvm-cov/Inputs/binary-formats.proftext (added)
+++ llvm/trunk/test/tools/llvm-cov/Inputs/binary-formats.proftext Mon Mar 16 01:55:45 2015
@@ -0,0 +1,4 @@
+main
+0x0
+1
+100
Added: llvm/trunk/test/tools/llvm-cov/binary-formats.c
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-cov/binary-formats.c?rev=232346&view=auto
==============================================================================
--- llvm/trunk/test/tools/llvm-cov/binary-formats.c (added)
+++ llvm/trunk/test/tools/llvm-cov/binary-formats.c Mon Mar 16 01:55:45 2015
@@ -0,0 +1,11 @@
+// Checks for reading various formats.
+
+// CHECK: 100| [[@LINE+1]]|int main
+int main(int argc, const char *argv[]) {}
+
+// RUN: llvm-profdata merge %S/Inputs/binary-formats.proftext -o %t.profdata
+// RUN: llvm-cov show %S/Inputs/binary-formats.macho32l -instr-profile %t.profdata -no-colors -filename-equivalence %s | FileCheck %s
+// RUN: llvm-cov show %S/Inputs/binary-formats.macho64l -instr-profile %t.profdata -no-colors -filename-equivalence %s | FileCheck %s
+
+// llvm-cov doesn't work on big endian yet
+// XFAIL: powerpc64-, s390x, mips-, mips64-, sparc
Modified: llvm/trunk/test/tools/llvm-cov/lit.local.cfg
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-cov/lit.local.cfg?rev=232346&r1=232345&r2=232346&view=diff
==============================================================================
--- llvm/trunk/test/tools/llvm-cov/lit.local.cfg (original)
+++ llvm/trunk/test/tools/llvm-cov/lit.local.cfg Mon Mar 16 01:55:45 2015
@@ -1,5 +1 @@
config.suffixes = ['.test', '.m', '.cpp', '.c']
-
-# http://llvm.org/bugs/show_bug.cgi?id=20979
-if 'ubsan' in config.available_features:
- config.unsupported = True
More information about the llvm-commits
mailing list