Skip to content

Commit b9f547e

Browse files
PiJoulesPiJoules
committed
[llvm][profile] Add padding after binary IDs
Some tests with binary IDs would fail with error: no profile can be merged. This is because raw profiles could have unaligned headers when emitting binary IDs. This means padding should be emitted after binary IDs are emitted to ensure everything else is aligned. This patch adds padding after each binary ID to ensure the next binary ID size is 8-byte aligned. This also adds extra checks to ensure we aren't reading corrupted data when printing binary IDs. Differential Revision: https://reviews.llvm.org/D110365
1 parent 9b944c1 commit b9f547e

File tree

7 files changed

+256
-8
lines changed

7 files changed

+256
-8
lines changed

‎compiler-rt/lib/profile/InstrProfilingPlatformLinux.c

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -95,10 +95,13 @@ static size_t RoundUp(size_t size, size_t align) {
9595
* have a fixed length.
9696
*/
9797
static int WriteOneBinaryId(ProfDataWriter *Writer, uint64_t BinaryIdLen,
98-
const uint8_t *BinaryIdData) {
98+
const uint8_t *BinaryIdData,
99+
uint64_t BinaryIdPadding) {
99100
ProfDataIOVec BinaryIdIOVec[] = {
100101
{&BinaryIdLen, sizeof(uint64_t), 1, 0},
101-
{BinaryIdData, sizeof(uint8_t), BinaryIdLen, 0}};
102+
{BinaryIdData, sizeof(uint8_t), BinaryIdLen, 0},
103+
{NULL, sizeof(uint8_t), BinaryIdPadding, 1},
104+
};
102105
if (Writer->Write(Writer, BinaryIdIOVec,
103106
sizeof(BinaryIdIOVec) / sizeof(*BinaryIdIOVec)))
104107
return -1;
@@ -130,11 +133,12 @@ static int WriteBinaryIdForNote(ProfDataWriter *Writer,
130133
uint64_t BinaryIdLen = Note->n_descsz;
131134
const uint8_t *BinaryIdData =
132135
(const uint8_t *)(NoteName + RoundUp(Note->n_namesz, 4));
133-
if (Writer != NULL &&
134-
WriteOneBinaryId(Writer, BinaryIdLen, BinaryIdData) == -1)
136+
uint8_t BinaryIdPadding = __llvm_profile_get_num_padding_bytes(BinaryIdLen);
137+
if (Writer != NULL && WriteOneBinaryId(Writer, BinaryIdLen, BinaryIdData,
138+
BinaryIdPadding) == -1)
135139
return -1;
136140

137-
BinaryIdSize = sizeof(BinaryIdLen) + BinaryIdLen;
141+
BinaryIdSize = sizeof(BinaryIdLen) + BinaryIdLen + BinaryIdPadding;
138142
}
139143

140144
return BinaryIdSize;

‎compiler-rt/test/profile/Linux/binary-id-padding.c

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
// Set these requirements to ensure that we have an 8-byte binary ID.
2+
// REQUIRES: linux
3+
//
4+
// This will generate a 20-byte build ID, which requires 4-byte padding.
5+
// RUN: %clang_profgen -Wl,--build-id=sha1 -o %t %s
6+
// RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t
7+
// RUN: %run %t %t.profraw
8+
9+
#include <errno.h>
10+
#include <stdio.h>
11+
#include <stdint.h>
12+
#include <stdlib.h>
13+
#include <string.h>
14+
#include <sys/mman.h>
15+
#include <sys/stat.h>
16+
#include <sys/stat.h>
17+
#include <fcntl.h>
18+
#include <unistd.h>
19+
20+
enum ValueKind {
21+
#define VALUE_PROF_KIND(Enumerator, Value, Descr) Enumerator = Value,
22+
#include "profile/InstrProfData.inc"
23+
};
24+
25+
typedef struct __llvm_profile_header {
26+
#define INSTR_PROF_RAW_HEADER(Type, Name, Initializer) Type Name;
27+
#include "profile/InstrProfData.inc"
28+
} __llvm_profile_header;
29+
30+
typedef void *IntPtrT;
31+
typedef struct __llvm_profile_data {
32+
#define INSTR_PROF_DATA(Type, LLVMType, Name, Initializer) Type Name;
33+
#include "profile/InstrProfData.inc"
34+
} __llvm_profile_data;
35+
36+
void bail(const char* str) {
37+
fprintf(stderr, "%s %s\n", str, strerror(errno));
38+
exit(1);
39+
}
40+
41+
void func() {}
42+
43+
int main(int argc, char** argv) {
44+
if (argc == 2) {
45+
int fd = open(argv[1], O_RDONLY);
46+
if (fd == -1)
47+
bail("open");
48+
49+
struct stat st;
50+
if (stat(argv[1], &st))
51+
bail("stat");
52+
uint64_t FileSize = st.st_size;
53+
54+
char* Buf = (char *)mmap(NULL, FileSize, PROT_READ, MAP_SHARED, fd, 0);
55+
if (Buf == MAP_FAILED)
56+
bail("mmap");
57+
58+
__llvm_profile_header *Header = (__llvm_profile_header *)Buf;
59+
if (Header->BinaryIdsSize != 32)
60+
bail("Invalid binary ID size");
61+
62+
char *BinaryIdsStart = Buf + sizeof(__llvm_profile_header);
63+
64+
uint64_t BinaryIdSize = *((uint64_t *)BinaryIdsStart);
65+
if (BinaryIdSize != 20)
66+
bail("Expected a binary ID of size 20");
67+
68+
// Skip the size and the binary ID itself to check padding.
69+
BinaryIdsStart += 8 + 20;
70+
if (*((uint32_t *)BinaryIdsStart))
71+
bail("Found non-zero binary ID padding");
72+
73+
if (munmap(Buf, FileSize))
74+
bail("munmap");
75+
76+
if (close(fd))
77+
bail("close");
78+
} else {
79+
func();
80+
}
81+
return 0;
82+
}

‎llvm/lib/ProfileData/InstrProfReader.cpp

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,9 @@ Error RawInstrProfReader<IntPtrT>::readHeader(
367367
return error(instrprof_error::unsupported_version);
368368

369369
BinaryIdsSize = swap(Header.BinaryIdsSize);
370+
if (BinaryIdsSize % sizeof(uint64_t))
371+
return error(instrprof_error::bad_header);
372+
370373
CountersDelta = swap(Header.CountersDelta);
371374
NamesDelta = swap(Header.NamesDelta);
372375
auto DataSize = swap(Header.DataSize);
@@ -402,6 +405,10 @@ Error RawInstrProfReader<IntPtrT>::readHeader(
402405
NamesStart = Start + NamesOffset;
403406
ValueDataStart = reinterpret_cast<const uint8_t *>(Start + ValueDataOffset);
404407

408+
const uint8_t *BufferEnd = (const uint8_t *)DataBuffer->getBufferEnd();
409+
if (BinaryIdsStart + BinaryIdsSize > BufferEnd)
410+
return error(instrprof_error::bad_header);
411+
405412
std::unique_ptr<InstrProfSymtab> NewSymtab = std::make_unique<InstrProfSymtab>();
406413
if (Error E = createSymtab(*NewSymtab.get()))
407414
return E;
@@ -520,15 +527,32 @@ Error RawInstrProfReader<IntPtrT>::readNextRecord(NamedInstrProfRecord &Record)
520527
return success();
521528
}
522529

530+
static size_t RoundUp(size_t size, size_t align) {
531+
return (size + align - 1) & ~(align - 1);
532+
}
533+
523534
template <class IntPtrT>
524535
Error RawInstrProfReader<IntPtrT>::printBinaryIds(raw_ostream &OS) {
525536
if (BinaryIdsSize == 0)
526537
return success();
527538

528539
OS << "Binary IDs: \n";
529540
const uint8_t *BI = BinaryIdsStart;
530-
while (BI < BinaryIdsStart + BinaryIdsSize) {
541+
const uint8_t *BIEnd = BinaryIdsStart + BinaryIdsSize;
542+
while (BI < BIEnd) {
543+
size_t Remaining = BIEnd - BI;
544+
545+
// There should be enough left to read the binary ID size field.
546+
if (Remaining < sizeof(uint64_t))
547+
return make_error<InstrProfError>(instrprof_error::malformed);
548+
531549
uint64_t BinaryIdLen = swap(*reinterpret_cast<const uint64_t *>(BI));
550+
551+
// There should be enough left to read the binary ID size field, and the
552+
// binary ID.
553+
if (Remaining < sizeof(BinaryIdLen) + BinaryIdLen)
554+
return make_error<InstrProfError>(instrprof_error::malformed);
555+
532556
// Increment by binary id length data type size.
533557
BI += sizeof(BinaryIdLen);
534558
if (BI > (const uint8_t *)DataBuffer->getBufferEnd())
@@ -538,8 +562,9 @@ Error RawInstrProfReader<IntPtrT>::printBinaryIds(raw_ostream &OS) {
538562
OS << format("%02x", BI[I]);
539563
OS << "\n";
540564

541-
// Increment by binary id data length.
542-
BI += BinaryIdLen;
565+
// Increment by binary id data length, rounded to the next 8 bytes. This
566+
// accounts for the zero-padding after each build ID.
567+
BI += RoundUp(BinaryIdLen, sizeof(uint64_t));
543568
if (BI > (const uint8_t *)DataBuffer->getBufferEnd())
544569
return make_error<InstrProfError>(instrprof_error::malformed);
545570
}

‎llvm/test/tools/llvm-profdata/binary-ids-padding.test

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
// Header
2+
//
3+
// INSTR_PROF_RAW_HEADER(uint64_t, Magic, __llvm_profile_get_magic())
4+
// INSTR_PROF_RAW_HEADER(uint64_t, Version, __llvm_profile_get_version())
5+
// INSTR_PROF_RAW_HEADER(uint64_t, BinaryIdsSize, __llvm_write_binary_ids(NULL))
6+
// INSTR_PROF_RAW_HEADER(uint64_t, DataSize, DataSize)
7+
// INSTR_PROF_RAW_HEADER(uint64_t, CountersSize, CountersSize)
8+
// INSTR_PROF_RAW_HEADER(uint64_t, NamesSize, NamesSize)
9+
// INSTR_PROF_RAW_HEADER(uint64_t, CountersDelta, (uintptr_t)CountersBegin)
10+
// INSTR_PROF_RAW_HEADER(uint64_t, NamesDelta, (uintptr_t)NamesBegin)
11+
// INSTR_PROF_RAW_HEADER(uint64_t, ValueKindLast, IPVK_Last)
12+
13+
RUN: printf '\201rforpl\377' > %t.profraw
14+
RUN: printf '\7\0\0\0\0\0\0\0' >> %t.profraw
15+
// There will be 2 20-byte binary IDs, so the total Binary IDs size will be 64 bytes.
16+
// 2 * 8 binary ID sizes
17+
// + 2 * 20 binary IDs (of size 20)
18+
// + 2 * 4 binary ID paddings
19+
// --------
20+
// = 64 bytes
21+
RUN: printf '\100\0\0\0\0\0\0\0' >> %t.profraw
22+
RUN: printf '\2\0\0\0\0\0\0\0' >> %t.profraw
23+
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
24+
RUN: printf '\3\0\0\0\0\0\0\0' >> %t.profraw
25+
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
26+
RUN: printf '\20\0\0\0\0\0\0\0' >> %t.profraw
27+
RUN: printf '\0\0\4\0\1\0\0\0' >> %t.profraw
28+
RUN: printf '\0\0\4\0\2\0\0\0' >> %t.profraw
29+
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
30+
31+
// Binary IDs - There are only two in this case that are 20 bytes.
32+
RUN: printf '\24\0\0\0\0\0\0\0' >> %t.profraw
33+
RUN: printf '\0\1\2\3\4\5\6\7' >> %t.profraw
34+
RUN: printf '\0\1\2\3\4\5\6\7' >> %t.profraw
35+
RUN: printf '\0\1\2\3\0\0\0\0' >> %t.profraw
36+
RUN: printf '\24\0\0\0\0\0\0\0' >> %t.profraw
37+
RUN: printf '\1\1\1\1\1\1\1\1' >> %t.profraw
38+
RUN: printf '\2\2\2\2\2\2\2\2' >> %t.profraw
39+
RUN: printf '\3\3\3\3\0\0\0\0' >> %t.profraw
40+
41+
// Data Section
42+
//
43+
// struct ProfData {
44+
// #define INSTR_PROF_DATA(Type, LLVMType, Name, Initializer) \
45+
// Type Name;
46+
// #include "llvm/ProfileData/InstrProfData.inc"
47+
// };
48+
49+
RUN: printf '\254\275\030\333\114\302\370\134' >> %t.profraw
50+
RUN: printf '\1\0\0\0\0\0\0\0' >> %t.profraw
51+
RUN: printf '\0\0\4\0\1\0\0\0' >> %t.profraw
52+
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
53+
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
54+
RUN: printf '\1\0\0\0\0\0\0\0' >> %t.profraw
55+
56+
RUN: printf '\067\265\035\031\112\165\023\344' >> %t.profraw
57+
RUN: printf '\02\0\0\0\0\0\0\0' >> %t.profraw
58+
RUN: printf '\xd8\xff\3\0\1\0\0\0' >> %t.profraw
59+
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
60+
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
61+
RUN: printf '\02\0\0\0\0\0\0\0' >> %t.profraw
62+
63+
RUN: printf '\023\0\0\0\0\0\0\0' >> %t.profraw
64+
RUN: printf '\067\0\0\0\0\0\0\0' >> %t.profraw
65+
RUN: printf '\101\0\0\0\0\0\0\0' >> %t.profraw
66+
RUN: printf '\7\0foo\1bar\0\0\0\0\0\0\0' >> %t.profraw
67+
68+
// RUN: llvm-profdata show --binary-ids %t.profraw | FileCheck %s
69+
// CHECK: Instrumentation level: Front-end
70+
// CHECK: Binary IDs:
71+
// CHECK-NEXT: 0001020304050607000102030405060700010203
72+
// CHECK-NEXT: 0101010101010101020202020202020203030303

‎llvm/test/tools/llvm-profdata/insufficient-binary-ids-size.test

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
RUN: printf '\201rforpl\377' > %t.profraw
2+
RUN: printf '\7\0\0\0\0\0\0\0' >> %t.profraw
3+
// We should fail on this because the data buffer (profraw file) is not long
4+
// enough to hold this binary IDs size. NOTE that this (combined with the 8-byte
5+
// alignment requirement for binary IDs size) will ensure we can at least read one
6+
// 8-byte size if the binary IDs are provided.
7+
RUN: printf '\8\0\0\0\0\0\0\0' >> %t.profraw
8+
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
9+
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
10+
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
11+
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
12+
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
13+
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
14+
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
15+
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
16+
17+
RUN: printf '\0\0\0\0\0\0\0' >> %t.profraw
18+
19+
// RUN: not llvm-profdata show --binary-ids %t.profraw 2>&1 | FileCheck %s
20+
// CHECK: invalid instrumentation profile data (file header is corrupt)

‎llvm/test/tools/llvm-profdata/large-binary-id-size.test

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
RUN: printf '\201rforpl\377' > %t.profraw
2+
RUN: printf '\7\0\0\0\0\0\0\0' >> %t.profraw
3+
RUN: printf '\40\0\0\0\0\0\0\0' >> %t.profraw
4+
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
5+
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
6+
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
7+
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
8+
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
9+
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
10+
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
11+
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
12+
13+
// Check for a corrupted size being too large past the end of the file.
14+
RUN: printf '\7\7\7\7\7\7\7\7' >> %t.profraw
15+
RUN: printf '\0\1\2\3\4\5\6\7' >> %t.profraw
16+
RUN: printf '\0\1\2\3\4\5\6\7' >> %t.profraw
17+
RUN: printf '\0\1\2\3\0\0\0\0' >> %t.profraw
18+
19+
// RUN: not llvm-profdata show --binary-ids %t.profraw 2>&1 | FileCheck %s
20+
// CHECK: malformed instrumentation profile data

‎llvm/test/tools/llvm-profdata/misaligned-binary-ids-size.test

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
RUN: printf '\201rforpl\377' > %t.profraw
2+
RUN: printf '\7\0\0\0\0\0\0\0' >> %t.profraw
3+
// We should fail on this because the binary IDs is not a multiple of 8 bytes.
4+
RUN: printf '\77\0\0\0\0\0\0\0' >> %t.profraw
5+
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
6+
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
7+
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
8+
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
9+
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
10+
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
11+
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
12+
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
13+
14+
// Binary IDs - There are only two in this case that are 20 bytes.
15+
RUN: printf '\24\0\0\0\0\0\0\0' >> %t.profraw
16+
RUN: printf '\0\1\2\3\4\5\6\7' >> %t.profraw
17+
RUN: printf '\0\1\2\3\4\5\6\7' >> %t.profraw
18+
RUN: printf '\0\1\2\3\0\0\0\0' >> %t.profraw
19+
RUN: printf '\24\0\0\0\0\0\0\0' >> %t.profraw
20+
RUN: printf '\1\1\1\1\1\1\1\1' >> %t.profraw
21+
RUN: printf '\2\2\2\2\2\2\2\2' >> %t.profraw
22+
RUN: printf '\3\3\3\3\0\0\0\0' >> %t.profraw
23+
24+
// RUN: not llvm-profdata show --binary-ids %t.profraw 2>&1 | FileCheck %s
25+
// CHECK: invalid instrumentation profile data (file header is corrupt)

0 commit comments

Comments
 (0)