Use segment's off and vaddr fields instead of IsSharedObject heuristic

This patch unifies the handling of ET_EXEC and ET_DYN ELF files by
clarifying the difference between "offset" of function's code in ELF
file, its "virtual address" that is recorded in the file (according to
ELF specification, it is the "symbol value" written to st_value field in
a symbol table entry in case of executable and shared object ELF files)
and the actual address of function in the address space of a process
after dynamic relocations took place.

Please note that file offset and virtual address are usually the same in
ET_DYN files (such as shared objects and position-independent
executables) but it is not required and this assumption is sometimes
violated in real-life scenarios.

Reviewed By: tnfchris

Differential Revision: https://reviews.llvm.org/D144852

~~

Huawei RRI, OS Lab

Author: kpdev

lnt/testing/profile/cPerf.cpp

@@ -178,30 +178,6 @@ void Assert(bool Expr, const char *ExprStr, const char *File, int Line) {
   throw std::logic_error(Str);
 }
 
-// Returns true if the ELF file given by filename
-// is a shared object (DYN).
-bool IsSharedObject(const std::string &Fname) {
-  // We replicate the first part of an ELF header here
-  // so as not to rely on <elf.h>.
-  struct PartialElfHeader {
-    unsigned char e_ident[16];
-    uint16_t e_type;
-  };
-  const int ET_DYN = 3;
-
-  FILE *stream = fopen(Fname.c_str(), "r");
-  if (stream == NULL)
-    return false;
-
-  PartialElfHeader H;
-  auto NumRead = fread(&H, 1, sizeof(H), stream);
-  assert(NumRead == sizeof(H));
-
-  fclose(stream);
-
-  return H.e_type == ET_DYN;
-}
-
 //===----------------------------------------------------------------------===//
 // Perf structures. Taken from https://lwn.net/Articles/644919/
 //===----------------------------------------------------------------------===//
@@ -360,9 +336,20 @@ static const char* sw_event_names[PERF_COUNT_SW_MAX] = {
 //===----------------------------------------------------------------------===//
 
 struct Map {
-  uint64_t Start, End, Adjust;
-  bool isSO;
+  Map(uint64_t Start, uint64_t End, const char *Filename)
+    : Start(Start), End(End), Filename(Filename) {}
+
+  uint64_t Start, End;
   const char *Filename;
+
+  // Mapping-related adjustments. Here FileOffset(func) is the offset of func
+  // in the ELF file, VAddr(func) is the virtual address associated with this
+  // symbol (in case of executable and shared object ELF files, st_value field
+  // of a symbol table's entry is symbol's virtual address) and &func is the
+  // actual memory address after relocations took place in the address space of
+  // the process being profiled.
+  uint64_t FileToPCOffset;    // FileOffset(func) + FileToPCOffset == &func
+  uint64_t VAddrToFileOffset; // VAddr(func) + VAddrToFileOffset == FileOffset(func)
 };
 
 struct EventDesc {
@@ -389,7 +376,7 @@ class SymTabOutput : public std::vector<Symbol> {
   SymTabOutput(std::string Objdump, std::string BinaryCacheRoot)
     : Objdump(Objdump), BinaryCacheRoot(BinaryCacheRoot) {}
 
-  uint64_t fetchExecSegment(Map *M) {
+  void fetchExecSegment(Map *M, uint64_t *FileOffset, uint64_t *VAddr) {
     std::string Cmd = Objdump + " -p -C " +
                       BinaryCacheRoot + std::string(M->Filename) +
 #ifdef _WIN32
@@ -401,7 +388,7 @@ class SymTabOutput : public std::vector<Symbol> {
 
     char *Line = nullptr, *PrevLine = nullptr;
     size_t LineLen = 0;
-    uint64_t offset = 0;
+    *FileOffset = *VAddr = 0;
     while (true) {
       if (PrevLine)
         free (PrevLine);
@@ -411,17 +398,22 @@ class SymTabOutput : public std::vector<Symbol> {
       if (Len == -1)
         break;
 
-      char* pos;
-      if ((pos = strstr (Line, "flags r-x")) == NULL
-          && (pos = strstr (Line, "flags rwx")) == NULL)
+      if (!strstr(Line, "flags r-x") && !strstr(Line, "flags rwx"))
         continue;
 
       /* Format is weird.. but we did find the section so punt.  */
-      if ((pos = strstr (PrevLine, "vaddr ")) == NULL)
+      const char *OFFSET_LABEL = "off ";
+      const char *VADDR_LABEL = "vaddr ";
+      char *pos_offset = strstr(PrevLine, OFFSET_LABEL);
+      char *pos_vaddr = strstr(PrevLine, VADDR_LABEL);
+      if (!pos_offset || !pos_vaddr)
         break;
 
-      pos += 6;
-      offset = strtoull (pos, NULL, 16);
+      pos_offset += strlen(OFFSET_LABEL);
+      pos_vaddr += strlen(VADDR_LABEL);
+      *FileOffset = strtoull(pos_offset, NULL, 16);
+      *VAddr = strtoull(pos_vaddr, NULL, 16);
+
       break;
     }
     if (Line)
@@ -435,7 +427,6 @@ class SymTabOutput : public std::vector<Symbol> {
     fclose(Stream);
     wait(NULL);
 #endif
-    return offset;
   }
 
   void fetchSymbols(Map *M) {
@@ -528,15 +519,14 @@ class SymTabOutput : public std::vector<Symbol> {
 
   void reset(Map *M) {
     clear();
+
+    // Take possible difference between "offset" and "virtual address" of
+    // the executable segment into account.
+    uint64_t FileOffset, VAddr;
+    fetchExecSegment(M, &FileOffset, &VAddr);
+    M->VAddrToFileOffset = FileOffset - VAddr;
+
     // Fetch both dynamic and static symbols, sort and unique them.
-    /* If we're a relocatable object then take the actual start of the text
-       segment into account.  */
-    if (M->isSO) {
-      uint64_t segmentStart = fetchExecSegment (M);
-      /* Adjust the symbol to a value relative to the start of the load address
-         to match up with registerNewMapping.  */
-      M->Adjust -= segmentStart;
-    }
     fetchSymbols(M);
 
     std::sort(begin(), end());
@@ -670,8 +660,7 @@ class PerfReader {
   void emitSymbol(
       Symbol &Sym, Map &M,
       std::map<uint64_t, std::map<const char *, uint64_t>>::iterator Event,
-      std::map<const char *, uint64_t> &SymEvents,
-      uint64_t Adjust);
+      std::map<const char *, uint64_t> &SymEvents);
   PyObject *complete();
 
 private:
@@ -851,13 +840,11 @@ static uint64_t getTimeFromSampleId(unsigned char *EndOfStruct,
 void PerfReader::registerNewMapping(unsigned char *Buf, const char *Filename) {
   perf_event_mmap_common *E = (perf_event_mmap_common *)Buf;
   auto MapID = Maps.size();
-  // EXEC ELF objects aren't relocated. DYN ones are,
-  // so if it's a DYN object adjust by subtracting the
-  // map base.
-  bool IsSO = IsSharedObject(BinaryCacheRoot + std::string(Filename));
+
   uint64_t End = E->start + E->extent;
-  uint64_t Adjust = IsSO ? E->start - E->pgoff : 0;
-  Maps.push_back({E->start, End, Adjust, IsSO, Filename});
+  Map NewMapping(E->start, End, Filename);
+  NewMapping.FileToPCOffset = E->start - E->pgoff;
+  Maps.push_back(NewMapping);
 
   unsigned char *EndOfEvent = Buf + E->header.size;
   // FIXME: The first EventID is used for every event.
@@ -1025,24 +1012,25 @@ void PerfReader::emitMaps() {
     if (AllUnderThreshold)
       continue;
 
+    Map &M = Maps[MapID];
     SymTabOutput Syms(Objdump, BinaryCacheRoot);
-    Syms.reset(&Maps[MapID]);
+    Syms.reset(&M);
 
-    uint64_t Adjust = Maps[MapID].Adjust;
+    uint64_t VAddrToPCOffset = M.VAddrToFileOffset + M.FileToPCOffset;
 
     // Accumulate the event totals for each symbol
     auto Sym = Syms.begin();
     auto Event = MapEvents.begin();
     std::map<uint64_t, std::map<const char*, uint64_t>> SymToEventTotals;
     while (Event != MapEvents.end() && Sym != Syms.end()) {
       // Skip events until we find one after the start of Sym
-      auto PC = Event->first - Adjust;
-      if (PC < Sym->Start) {
+      auto VAddr = Event->first - VAddrToPCOffset;
+      if (VAddr < Sym->Start) {
         ++Event;
         continue;
       }
       // Skip symbols until the event is before the end of Sym
-      if (PC >= Sym->End) {
+      if (VAddr >= Sym->End) {
         ++Sym;
         continue;
       }
@@ -1062,26 +1050,28 @@ void PerfReader::emitMaps() {
         }
       }
       if (Keep)
-        emitSymbol(Sym, Maps[MapID], MapEvents.lower_bound(Sym.Start),
-                   SymToEventTotals[Sym.Start], Adjust);
+        emitSymbol(Sym, M, MapEvents.lower_bound(Sym.Start + VAddrToPCOffset),
+                   SymToEventTotals[Sym.Start]);
     }
   }
 }
 
 void PerfReader::emitSymbol(
     Symbol &Sym, Map &M,
     std::map<uint64_t, std::map<const char *, uint64_t>>::iterator Event,
-    std::map<const char *, uint64_t> &SymEvents,
-    uint64_t Adjust) {
+    std::map<const char *, uint64_t> &SymEvents) {
+  uint64_t VAddrToPCOffset = M.VAddrToFileOffset + M.FileToPCOffset;
   ObjdumpOutput Dump(Objdump, BinaryCacheRoot);
   Dump.reset(&M, Sym.Start, Sym.End);
 
   emitFunctionStart(Sym.Name);
+  assert(Sym.Start <= Event->first - VAddrToPCOffset &&
+         Event->first - VAddrToPCOffset < Sym.End);
   for (uint64_t I = Dump.next(); I < Sym.End; I = Dump.next()) {
-    auto PC = Event->first - Adjust;
+    auto VAddr = Event->first - VAddrToPCOffset;
 
     auto Text = Dump.getText();
-    if (PC == I) {
+    if (VAddr == I) {
       emitLine(I, &Event->second, Text);
       ++Event;
     } else {

tests/testing/Inputs/Sources/segments.c

@@ -0,0 +1,17 @@
+#include <stdlib.h>
+
+volatile unsigned n = 0;
+
+__attribute__((noinline))
+__attribute__((section(".text.correct")))
+__attribute__((aligned(0x1000)))
+void correct(long count) {
+  for (long i = 0; i < count; ++i) {
+    n += 1;
+  }
+}
+
+int main(int argc, const char *argv[]) {
+  correct(atol(argv[1]));
+  return 0;
+}

tests/testing/Inputs/Sources/segments.lds

@@ -0,0 +1,9 @@
+SECTIONS {
+  .text (. + 0x1000) : {
+    *(.text)
+    *(.text.correct)
+  }
+} INSERT BEFORE .init;
+/* .init is the first section placed to the executable segment in a binary
+ * produced by clang at the time of writing
+ */

tests/testing/Inputs/Sources/segments.sh

@@ -0,0 +1,86 @@
+#!/bin/bash
+
+# While it is quite common for ET_DYN ELF files to have virtual addresses equal
+# to file offsets, these are different entities. For example, the code segment
+# is sometimes shifted by one page or so.
+#
+# This script prepares an executable file with code contained in a section
+# that has VirtAddr == FileOffset + 0x1000.
+#
+# In addition, this script also creates two regular executables:
+# a position-independent executable and a static one to check the handling of
+# the more traditional layout of ELF segments for ET_DYN and ET_EXEC binaries.
+#
+# A few simple checks are performed to make sure the heuristics used to create
+# the required segment layouts still work.
+
+cd "$(dirname $0)"
+
+save_objdump_output() {
+  local path_to_elf="$1"
+  local addr_correct="$2"
+
+  local basename="$(basename "$path_to_elf")"
+
+  llvm-objdump "$path_to_elf" -t > "../${basename}.objdump.out"
+  llvm-objdump "$path_to_elf" -p > "../${basename}.objdump.p.out"
+  llvm-objdump "$path_to_elf" -j .text --disassemble-symbols=correct > "../${basename}.objdump.${addr_correct}.out"
+}
+
+record_perf_data() {
+  local path_to_elf="$1"
+  local basename="$(basename "$path_to_elf")"
+  local path_to_perf_data="../${basename}.perf_data"
+  local num_of_iterations=100000000
+
+  rm -f "$path_to_perf_data"
+  perf record -e cpu-clock -o "$path_to_perf_data" "$path_to_elf" $num_of_iterations
+
+  # It is probably not a good idea to put very large *.perf_data files to git
+  size_in_bytes=$(stat --format='%s' "$path_to_perf_data")
+  if [ $size_in_bytes -gt 50000 ]; then
+    echo "perf produced too large output file ${path_to_perf_data}, try decreasing"
+    echo "the number of iterations or passing -F option to 'perf record'."
+    exit 1
+  fi
+}
+
+save_test_case() {
+  local path_to_elf="$1"
+  local addr_correct="$2"
+
+  record_perf_data "$path_to_elf"
+  save_objdump_output "$path_to_elf" $addr_correct
+}
+
+check_file() {
+  local file="$1"
+  local line="$2"
+
+  # Use pcregrep to simplify handling of newlines (it is possible to embed \n
+  # into the regex and not have them being matched by a dot)
+  if ! pcregrep -M "$line" "$file"; then
+    echo "Unexpected test case generated: file '$file' should contain '$line'"
+    exit 1
+  fi
+}
+
+clang -Os -o /tmp/segments-shifted segments.c -pie -Wl,-T,segments.lds
+clang -Os -o /tmp/segments-dyn     segments.c -pie
+clang -Os -o /tmp/segments-exec    segments.c -static
+
+save_test_case /tmp/segments-shifted 0x2000
+check_file ../segments-shifted.objdump.out "00002000 .* correct"
+# The expected objdump -p output is something like this (note off != vaddr):
+#     LOAD off    0x0000000000000618 vaddr 0x0000000000001618 paddr 0x0000000000001618 align 2**12
+#          filesz 0x0000000000002a3d memsz 0x0000000000002a3d flags r-x
+check_file ../segments-shifted.objdump.p.out "LOAD off    0x(0+)0000(...) vaddr 0x\g{1}0001\g{2} paddr.*\n.*flags r-x"
+
+# Feel free to update the value of "correct" symbol in the static case if it is changed
+save_test_case /tmp/segments-exec 0x403000
+check_file ../segments-exec.objdump.out "00403000 .* correct"
+check_file ../segments-exec.objdump.p.out "LOAD off    0x(0+)0001000 vaddr 0x(0+)0401000 paddr.*\n.*flags r-x"
+
+save_test_case /tmp/segments-dyn 0x3000
+check_file ../segments-dyn.objdump.out "00003000 .* correct"
+check_file ../segments-dyn.objdump.p.out "LOAD off    0x(0+)0001000 vaddr 0x(0+)0001000 paddr.*\n.*flags r-x"

tests/testing/Inputs/fake-objdump.py

@@ -16,4 +16,7 @@ def exit_with_fake_output(suffix):
     if arg.startswith('-t'):
         exit_with_fake_output('out')
 
+    if arg.startswith('-p'):
+        exit_with_fake_output('p.out')
+
 sys.exit(1)

tests/testing/Inputs/fib-aarch64.objdump.p.out

@@ -0,0 +1,16 @@
+Fake "objdump -p" output.
+
+The original test case was added when ET_EXEC and ET_DYN ELF binaries were
+handled differently (assuming ET_EXEC by default - if IsSharedObject() function
+cannot find the file).
+
+This test input was added to fix the existing tests after the removal of the
+heuristic relying on virtual addresses being equal to file offsets for ET_DYN
+case and to final addresses in the process' address space for ET_EXEC case,
+respectively.
+
+The "off" and "vaddr" fields are set to some reasonable values based on the
+mmap2 records from *.perf_data file.
+
+LOAD off    0x00000000 vaddr 0x00400000 paddr ...
+     ...        ...     ...      ...    flags r-x

tests/testing/Inputs/fib2-aarch64.objdump.p.out

@@ -0,0 +1,16 @@
+Fake "objdump -p" output.
+
+The original test case was added when ET_EXEC and ET_DYN ELF binaries were
+handled differently (assuming ET_EXEC by default - if IsSharedObject() function
+cannot find the file).
+
+This test input was added to fix the existing tests after the removal of the
+heuristic relying on virtual addresses being equal to file offsets for ET_DYN
+case and to final addresses in the process' address space for ET_EXEC case,
+respectively.
+
+The "off" and "vaddr" fields are set to some reasonable values based on the
+mmap2 records from *.perf_data file.
+
+LOAD off    0x00000000 vaddr 0x00400000 paddr ...
+     ...        ...     ...      ...    flags r-x

tests/testing/Inputs/segments-dyn.objdump.0x3000.out

@@ -0,0 +1,12 @@
+
+/tmp/segments-dyn:	file format elf64-x86-64
+
+Disassembly of section .text:
+
+0000000000003000 <correct>:
+    3000: 48 85 ff                     	testq	%rdi, %rdi
+    3003: 7e 0b                        	jle	0x3010 <correct+0x10>
+    3005: ff 05 11 30 00 00            	incl	12305(%rip)             # 0x601c <n>
+    300b: 48 ff cf                     	decq	%rdi
+    300e: 75 f5                        	jne	0x3005 <correct+0x5>
+    3010: c3                           	retq