Skip to content

[profile] Use base+vaddr for __llvm_write_binary_ids note pointers #114907

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 21, 2024
Merged

[profile] Use base+vaddr for __llvm_write_binary_ids note pointers #114907

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 16 additions & 24 deletions compiler-rt/lib/profile/InstrProfilingPlatformLinux.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,41 +194,33 @@ static int WriteBinaryIds(ProfDataWriter *Writer, const ElfW(Nhdr) * Note,
*/
COMPILER_RT_VISIBILITY int __llvm_write_binary_ids(ProfDataWriter *Writer) {
extern const ElfW(Ehdr) __ehdr_start __attribute__((visibility("hidden")));
extern ElfW(Dyn) _DYNAMIC[] __attribute__((weak, visibility("hidden")));

const ElfW(Ehdr) *ElfHeader = &__ehdr_start;
const ElfW(Phdr) *ProgramHeader =
(const ElfW(Phdr) *)((uintptr_t)ElfHeader + ElfHeader->e_phoff);

/* Compute the added base address in case of position-independent code. */
uintptr_t Base = 0;
for (uint32_t I = 0; I < ElfHeader->e_phnum; I++) {
if (ProgramHeader[I].p_type == PT_PHDR)
Base = (uintptr_t)ProgramHeader - ProgramHeader[I].p_vaddr;
if (ProgramHeader[I].p_type == PT_DYNAMIC && _DYNAMIC)
Base = (uintptr_t)_DYNAMIC - ProgramHeader[I].p_vaddr;
}

int TotalBinaryIdsSize = 0;
uint32_t I;
/* Iterate through entries in the program header. */
for (I = 0; I < ElfHeader->e_phnum; I++) {
for (uint32_t I = 0; I < ElfHeader->e_phnum; I++) {
/* Look for the notes segment in program header entries. */
if (ProgramHeader[I].p_type != PT_NOTE)
continue;

/* There can be multiple notes segment, and examine each of them. */
const ElfW(Nhdr) * Note;
const ElfW(Nhdr) * NotesEnd;
/*
* When examining notes in file, use p_offset, which is the offset within
* the elf file, to find the start of notes.
*/
if (ProgramHeader[I].p_memsz == 0 ||
ProgramHeader[I].p_memsz == ProgramHeader[I].p_filesz) {
Note = (const ElfW(Nhdr) *)((uintptr_t)ElfHeader +
ProgramHeader[I].p_offset);
NotesEnd = (const ElfW(Nhdr) *)((const char *)(Note) +
ProgramHeader[I].p_filesz);
} else {
/*
* When examining notes in memory, use p_vaddr, which is the address of
* section after loaded to memory, to find the start of notes.
*/
Note =
(const ElfW(Nhdr) *)((uintptr_t)ElfHeader + ProgramHeader[I].p_vaddr);
NotesEnd =
(const ElfW(Nhdr) *)((const char *)(Note) + ProgramHeader[I].p_memsz);
}
const ElfW(Nhdr) *Note =
(const ElfW(Nhdr) *)(Base + ProgramHeader[I].p_vaddr);
Copy link
Member Author

@cuviper cuviper Nov 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may need to clarify: this is close to the former ElfHeader + p_vaddr, but in the case of non-PIE, the vaddr already includes the fixed base address. The Base computation above works either way since it's finding the difference between the runtime address and the header's vaddr, so it's just 0 for non-PIE.

Also, I don't think the code was ever reaching the old else case at all. At least in all the files I looked at, p_filesz and p_memsz were always the same, so we would always be in the "examining notes in file" case.

const ElfW(Nhdr) *NotesEnd =
(const ElfW(Nhdr) *)((const char *)(Note) + ProgramHeader[I].p_memsz);

int BinaryIdsSize = WriteBinaryIds(Writer, Note, NotesEnd);
if (TotalBinaryIdsSize == -1)
Expand Down
33 changes: 33 additions & 0 deletions compiler-rt/test/profile/Linux/binary-id-offset.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// REQUIRES: linux
//
// Make sure the build-id can be found in both EXEC and DYN (PIE) files,
// even when the note's section-start is forced to a weird address.
// (The DYN case would also apply to libraries, not explicitly tested here.)

// DEFINE: %{cflags} =
// DEFINE: %{check} = ( \
// DEFINE: %clang_profgen -Wl,--build-id -o %t %s %{cflags} && \
// DEFINE: env LLVM_PROFILE_FILE=%t.profraw %run %t && \
// DEFINE: llvm-readelf --notes %t && \
// DEFINE: llvm-profdata show --binary-ids %t.profraw \
// DEFINE: ) | FileCheck %s

// REDEFINE: %{cflags} = -no-pie
// RUN: %{check}

// REDEFINE: %{cflags} = -pie -fPIE
// RUN: %{check}

// REDEFINE: %{cflags} = -no-pie -Wl,--section-start=.note.gnu.build-id=0x1000000
// RUN: %{check}

// REDEFINE: %{cflags} = -pie -fPIE -Wl,--section-start=.note.gnu.build-id=0x1000000
// RUN: %{check}

// CHECK-LABEL{LITERAL}: .note.gnu.build-id
// CHECK: Build ID: [[ID:[0-9a-f]+]]

// CHECK-LABEL{LITERAL}: Binary IDs:
// CHECK-NEXT: [[ID]]

int main() { return 0; }
Loading