[PATCH] D148301: [memprof] Print out profile build ids in the error message.

Mon Apr 17 07:55:44 PDT 2023

tejohnson accepted this revision.
tejohnson added a comment.
This revision is now accepted and ready to land.

lgtm

================
Comment at: llvm/lib/ProfileData/RawMemProfReader.cpp:541
+  // Use a set + vector since a profile file may contain multiple raw profile
+  // dumps, each with segment information. We want them unique and in order they
+  // were stored in the profile.
----------------
snehasish wrote:
> tejohnson wrote:
> > Why do we need them in the order they were stored?
> If we keep them in order then the profiled binary build id *should* be the first entry in this list. We rely on the sanitizer tooling (which calls dl_iterate_phdr [1]) to maintain the order which is read from the program headers and we preserve the order in the profile. Added a short comment.
> 
> [1] https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp#L737
> 
> "The first object visited by callback is the main program."
> https://man7.org/linux/man-pages/man3/dl_iterate_phdr.3.html
Thanks for the clarification. I would just add the man page link and excerpt to the comment to document why it should be first.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148301/new/

https://reviews.llvm.org/D148301