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

[Snehasish Kumar via Phabricator via llvm-commits]

snehasish marked an inline comment as done.
snehasish added a comment.

Thanks for the quick review!



================
Comment at: llvm/lib/ProfileData/RawMemProfReader.cpp:203
+        make_error<StringError>(ErrorMessage,
+                                inconvertibleErrorCode()),"");
+  }
----------------
tejohnson wrote:
> Should we pass the Path instead of "" for the report() Context parameter? If not useful, probably document the constant "" parameter.
Opted to document the param since I don't think printing the profile path is useful (the binary path is unset).


================
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.
----------------
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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148301