[PATCH] D96434: [CSSPGO][llvm-profgen] Filter out the instructions without location info for symbolizer

[Wenlei He via Phabricator via llvm-commits]

wenlei added a comment.

> symbolizer will return an empty call stack for them which will cause some crash later in profile unwinding.

Can you share more details on what which part had trouble dealing with such cases?



================
Comment at: llvm/tools/llvm-profgen/PerfReader.cpp:60
       uint64_t PrevIP = IP.Address;
-      IP.backward();
+      HasNext = IP.backward();
       // Break into segments for implicit call/return due to inlining
----------------
Since all code addresses for the entire binary is record in a single vector, this `HasNext` check is only going to help for the first/last instruction of the entire binary. 

Is the problem you're seeing very specific to first/last instruction of the entire binary? I thought instruction without location could happen anywhere..


================
Comment at: llvm/tools/llvm-profgen/ProfiledBinary.cpp:431
+    return false;
   Index++;
   Address = Binary->getAddressforIndex(Index);
----------------
What about still advancing the index before returning false? I.e. advancing IP past last instruction should leave IP in invalid state - this might help expose issues where return value from advance is not checked, otherwise we might be slightly accessing last instruction repeatedly. We could also set address to UINT_MAX for such case, so extra check may be avoided. Same for backward, but UINT_MIN for address.


================
Comment at: llvm/tools/llvm-profgen/ProfiledBinary.h:182
   }
+  uint32_t codeAddrsSize() { return CodeAddrs.size(); }
   StringRef getPath() const { return Path; }
----------------
nit: `getNumInstructions` or `getNumCodeAddresses` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96434