[Pseudo Probe] Remove the assert of allowing only one call probe for a callsite.

[hoy]

Summary

Compiler-generated static symbols, such as the global initializers, can shared the same name and can coexist in the binary. As a result, their pseudo probes are all kept in the binary too. This could cause multiple call probes decoded against one callsite, as probes are decoded against there owning functions by name. I'm temporarily disabling an assert to keep the debug build green until we have a better fix.

[wenlei]

Due to the the way .pseudo_probe section is decoded, probes of the same-named independent static functions are merged

Is this because dwarf based name (the container for probes) doesn't have the concept of static function and all functions are keyed on names (TopLevelProbeFrameMap), so we can't differentiate static functions of the same name?

I might misunderstand it initially, but this seems like a flaw we should try to fix? Two static functions can be completely different, and we need to have the capability to differentiate them..

[hoy]

In D153588#4445416, @wenlei wrote:

Due to the the way .pseudo_probe section is decoded, probes of the same-named independent static functions are merged

Is this because dwarf based name (the container for probes) doesn't have the concept of static function and all functions are keyed on names (TopLevelProbeFrameMap), so we can't differentiate static functions of the same name?

In this case the linkage names are also the same. Those functions are global ctors which bypass the unique linkage phase and end up same-named.

I might misunderstand it initially, but this seems like a flaw we should try to fix? Two static functions can be completely different, and we need to have the capability to differentiate them..

User static functions has no such problem since they have unique linkage names for both elf and dwarf. It only happens to compiler-generated functions such as global ctors. Those functions are usually not hot functions. I'm not sure they are worth being fixed.

[wenlei]

In D153588#4445437, @hoy wrote:

In D153588#4445416, @wenlei wrote:

Due to the the way .pseudo_probe section is decoded, probes of the same-named independent static functions are merged

Is this because dwarf based name (the container for probes) doesn't have the concept of static function and all functions are keyed on names (TopLevelProbeFrameMap), so we can't differentiate static functions of the same name?

In this case the linkage names are also the same. Those functions are global ctors which bypass the unique linkage phase and end up same-named.

I might misunderstand it initially, but this seems like a flaw we should try to fix? Two static functions can be completely different, and we need to have the capability to differentiate them..

User static functions has no such problem since they have unique linkage names for both elf and dwarf. It only happens to compiler-generated functions such as global ctors. Those functions are usually not hot functions. I'm not sure they are worth being fixed.

Ok, can you update the comment to explicit mention compiler generated static functions? also mention that user defined static functions are fine. Otherwise LGTM.

[hoy]

Updating comments

llvm/lib/MC/MCPseudoProbe.cpp

MCPseudoProbeDecoder::getCallProbeForAddr(uint64_t Address) const {
   auto It = Address2ProbesMap.find(Address);
   if (It == Address2ProbesMap.end())
     return nullptr;
   const auto &Probes = It->second;
 
   const MCDecodedPseudoProbe *CallProbe = nullptr;
   for (const auto &Probe : Probes) {
     if (Probe.isCall()) {
-      assert(!CallProbe &&
-             "There should be only one call probe corresponding to address "
-             "which is a callsite.");
+      // Disabling the assert and returning first call probe seen so far.
+      // Subsequent call probes, if any, are ignored. Due to the the way
+      // .pseudo_probe section is decoded, probes of the same-named independent
+      // static functions are merged thus multiple call probes may be seen for a
+      // callsite. This should only happen to compiler-generated statics, with
+      // -funique-internal-linkage-names where user statics get unique names.
+      //
+      // TODO: re-enable or narrow down the assert to static functions only.
+      //
+      // assert(!CallProbe &&
+      //        "There should be only one call probe corresponding to address "
+      //        "which is a callsite.");
       CallProbe = &Probe;
+      break;
     }
   }
   return CallProbe;
 }
 
 const MCPseudoProbeFuncDesc *
 MCPseudoProbeDecoder::getFuncDescForGUID(uint64_t GUID) const {
   auto It = GUID2FuncDescMap.find(GUID);