Skip to content

Commit dcf5fc3

Browse files
perf: Fix profile reading to correctly take segments into account.
LNT's perf parsing implementation has a very subtle bug horrible bug: It's assuming that the text segment starts at the same address that the program was loaded at. Or rather, it assumes the text segment is at address 0. This is usually not the case (and indeed LLD itself puts many non-exec Segments before the first exec one) Because of this no event is matched for certain binaries. Also the code can wrongly try to attribute an event to the wrong MMAP because it only checks the starting address. This means that the order of the MMAPs matters and again as of a few days ago the order coming out of LLD places the binary itself after most SOs including ld.so. By checking only the start address we can attribute an event to an SO instead of to the program since trivially shared libraries are loaded before the program itself in the virtual memory space. This would later fail to resolve but we'd lose the event. This patch changes it to also check for the ending position.
1 parent 8b2fbaf commit dcf5fc3

File tree

1 file changed

+66
-7
lines changed

1 file changed

+66
-7
lines changed

lnt/testing/profile/cPerf.cpp

Lines changed: 66 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ FILE *ForkAndExec(std::string Cmd) {
129129
if (fork() == 0) {
130130
dup2(P[1], 1);
131131
close(P[0]);
132-
execl("/bin/sh", "sh", "-c", Cmd.c_str(), 0);
132+
execl("/bin/sh", "sh", "-c", Cmd.c_str(), (char *)NULL);
133133
} else {
134134
close(P[1]);
135135
}
@@ -364,6 +364,12 @@ struct Map {
364364
const char *Filename;
365365
};
366366

367+
struct EventDesc {
368+
uint64_t Start;
369+
uint64_t End;
370+
size_t MapId;
371+
};
372+
367373
struct Symbol {
368374
uint64_t Start;
369375
uint64_t End;
@@ -382,6 +388,55 @@ class SymTabOutput : public std::vector<Symbol> {
382388
SymTabOutput(std::string Objdump, std::string BinaryCacheRoot)
383389
: Objdump(Objdump), BinaryCacheRoot(BinaryCacheRoot) {}
384390

391+
uint64_t fetchExecSegment(Map *M) {
392+
std::string Cmd = Objdump + " -p -C " +
393+
BinaryCacheRoot + std::string(M->Filename) +
394+
#ifdef _WIN32
395+
" 2> NUL";
396+
#else
397+
" 2>/dev/null";
398+
#endif
399+
auto Stream = ForkAndExec(Cmd);
400+
401+
char *Line = nullptr, *PrevLine = nullptr;
402+
size_t LineLen = 0;
403+
uint64_t offset = 0;
404+
while (true) {
405+
if (PrevLine)
406+
free (PrevLine);
407+
if (Line)
408+
PrevLine = strdup (Line);
409+
ssize_t Len = getline(&Line, &LineLen, Stream);
410+
if (Len == -1)
411+
break;
412+
413+
char* pos;
414+
if ((pos = strstr (Line, "flags r-x")) == NULL
415+
&& (pos = strstr (Line, "flags rwx")) == NULL)
416+
continue;
417+
418+
/* Format is weird.. but we did find the section so punt. */
419+
if ((pos = strstr (PrevLine, "vaddr ")) == NULL)
420+
break;
421+
422+
pos += 6;
423+
offset = strtoull (pos, NULL, 16);
424+
break;
425+
}
426+
if (Line)
427+
free(Line);
428+
if (PrevLine)
429+
free(PrevLine);
430+
431+
#ifdef _WIN32
432+
_pclose(Stream);
433+
#else
434+
fclose(Stream);
435+
wait(NULL);
436+
#endif
437+
return offset;
438+
}
439+
385440
void fetchSymbols(Map *M) {
386441
std::string Cmd = Objdump + " -t -T -C " +
387442
BinaryCacheRoot + std::string(M->Filename) +
@@ -473,6 +528,10 @@ class SymTabOutput : public std::vector<Symbol> {
473528
void reset(Map *M) {
474529
clear();
475530
// Fetch both dynamic and static symbols, sort and unique them.
531+
uint64_t segmentStart = fetchExecSegment (M);
532+
/* Adjust the symbol to a value relative to the start of the load address
533+
to match up with registerNewMapping. */
534+
M->Adjust -= segmentStart;
476535
fetchSymbols(M);
477536

478537
std::sort(begin(), end());
@@ -626,7 +685,7 @@ class PerfReader {
626685
std::map<const char *, uint64_t> TotalEvents;
627686
std::map<uint64_t, std::map<const char *, uint64_t>> TotalEventsPerMap;
628687
std::vector<Map> Maps;
629-
std::map<uint64_t, std::map<uint64_t, size_t>> CurrentMaps;
688+
std::map<uint64_t, std::map<uint64_t, EventDesc>> CurrentMaps;
630689

631690
PyObject *Functions, *TopLevelCounters;
632691
std::vector<PyObject*> Lines;
@@ -800,7 +859,7 @@ void PerfReader::registerNewMapping(unsigned char *Buf, const char *Filename) {
800859
// FIXME: The code assumes perf_event_attr.sample_id_all is set.
801860
uint64_t Time = getTimeFromSampleId(EndOfEvent, EventLayouts.begin()->second);
802861
auto &CurrentMap = CurrentMaps[Time];
803-
CurrentMap.insert({E->start, MapID});
862+
CurrentMap.insert({E->start, { E->start, End, MapID}});
804863
}
805864

806865
unsigned char *PerfReader::readEvent(unsigned char *Buf) {
@@ -841,9 +900,9 @@ unsigned char *PerfReader::readEvent(unsigned char *Buf) {
841900
continue;
842901
--NewI;
843902

844-
if (NewI->first > PC)
903+
if (NewI->second.Start > PC || NewI->second.End < PC)
845904
continue;
846-
MapID = NewI->second;
905+
MapID = NewI->second.MapId;
847906
break;
848907
}
849908
if (MapID != ~0ULL) {
@@ -961,11 +1020,11 @@ void PerfReader::emitMaps() {
9611020
if (AllUnderThreshold)
9621021
continue;
9631022

964-
uint64_t Adjust = Maps[MapID].Adjust;
965-
9661023
SymTabOutput Syms(Objdump, BinaryCacheRoot);
9671024
Syms.reset(&Maps[MapID]);
9681025

1026+
uint64_t Adjust = Maps[MapID].Adjust;
1027+
9691028
// Accumulate the event totals for each symbol
9701029
auto Sym = Syms.begin();
9711030
auto Event = MapEvents.begin();

0 commit comments

Comments
 (0)