[PATCH] D145190: [memprof] Record BuildIDs in the raw profile.

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added inline comments.


================
Comment at: compiler-rt/include/profile/MemProfData.inc:58
   uint64_t Offset;
-  // This field is unused until sanitizer procmaps support for build ids for
-  // Linux-Elf is implemented.
-  uint8_t BuildId[32] = {0};
+  uint64_t BuildIdSize; 
+  uint8_t BuildId[MEMPROF_BUILDID_MAX_SIZE] = {0};
----------------
snehasish wrote:
> tejohnson wrote:
> > Do we need to record the size or can we just pad with zeros if less than the max? Is it ever not 32 in practice? Looks like it is defined to 32 in sanitizer_common.h.
> I think we need it. In practice, we usually have 16 or 20 byte buildids [1] but they could be of arbitrary length if it's provided by the user rather than generated by the linker. Also the build id is a raw sequence of bytes. Padding with zeros is not enough to distinguish the length if the build id itself contains leading/trailing zero bytes.
> 
> [1] https://github.com/llvm/llvm-project/blob/8a5d4eb775c644d8683f24817d44c510d2b853b7/lld/ELF/Writer.cpp#L2938-L2975
Ok got it. I thought sanitizer_common.h always used 32, but I now see another interface where it can be variable (but max of 32, which matches the handling here).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145190