[llvm-profgen] Loading binary functions from .symtab when DWARF info is incomplete #163654

HighW4y2H3ll · 2025-10-15T22:56:25Z

Indexing in .debug_str section could lead to integer overflow when in DWARF32 format.
https://github.com/llvm/llvm-project/blob/e61e6251b692ffe71910bad22b82e41313f003cf/llvm/lib/DWP/DWP.cpp#L35C30-L35C47

This can lead to missing symbols from the DWARF info, and hurts profile quality. As a workaround, we may use information from the symbol table (.symtab), and recover the missing symbols with addresses and ranges.

Output:

# Before
...
warning: 6.64%(338916009/5106344252) of samples are from ranges that do not belong to any functions.

# After
...
warning: 0.07%(3501587/4906133148) of samples are from ranges that do not belong to any functions.
warning: 5.71%(280266919/4906133148) of samples are from ranges that belong to functions recovered from symbol table.

…is incomplete

llvmbot · 2025-10-15T22:57:00Z

@llvm/pr-subscribers-llvm-binary-utilities

@llvm/pr-subscribers-pgo

Author: None (HighW4y2H3ll)

Changes

Indexing in .debug_str section could lead to integer overflow when in DWARF32 format.
https://github.com/llvm/llvm-project/blob/e61e6251b692ffe71910bad22b82e41313f003cf/llvm/lib/DWP/DWP.cpp#L35C30-L35C47

This can lead to missing symbol names in the DWARF info, and hurts profile quality. As a workaround, we may use information from the symbol table, and recover the missing symbol addresses and ranges.

Full diff: https://github.com/llvm/llvm-project/pull/163654.diff

4 Files Affected:

(modified) llvm/include/llvm/ProfileData/SampleProf.h (+8-2)
(modified) llvm/tools/llvm-profgen/ProfileGenerator.cpp (+27)
(modified) llvm/tools/llvm-profgen/ProfiledBinary.cpp (+44)
(modified) llvm/tools/llvm-profgen/ProfiledBinary.h (+3)

diff --git a/llvm/include/llvm/ProfileData/SampleProf.h b/llvm/include/llvm/ProfileData/SampleProf.h
index 3dd34aba2d716..4adbe13b6712b 100644
--- a/llvm/include/llvm/ProfileData/SampleProf.h
+++ b/llvm/include/llvm/ProfileData/SampleProf.h
@@ -1214,12 +1214,18 @@ class FunctionSamples {
     // Note the sequence of the suffixes in the knownSuffixes array matters.
     // If suffix "A" is appended after the suffix "B", "A" should be in front
     // of "B" in knownSuffixes.
-    const char *KnownSuffixes[] = {LLVMSuffix, PartSuffix, UniqSuffix};
+    SmallVector<StringRef> KnownSuffixes ({LLVMSuffix, PartSuffix, UniqSuffix});
+    return getCanonicalFnName(FnName, KnownSuffixes, Attr);
+  }
+
+  static StringRef getCanonicalFnName(StringRef FnName,
+                                      const SmallVector<StringRef> &Suffixes,
+                                      StringRef Attr = "selected") {
     if (Attr == "" || Attr == "all")
       return FnName.split('.').first;
     if (Attr == "selected") {
       StringRef Cand(FnName);
-      for (const auto &Suf : KnownSuffixes) {
+      for (const auto &Suf : Suffixes) {
         StringRef Suffix(Suf);
         // If the profile contains ".__uniq." suffix, don't strip the
         // suffix for names in the IR.
diff --git a/llvm/tools/llvm-profgen/ProfileGenerator.cpp b/llvm/tools/llvm-profgen/ProfileGenerator.cpp
index 3b875c5de3c09..058b154fc5a57 100644
--- a/llvm/tools/llvm-profgen/ProfileGenerator.cpp
+++ b/llvm/tools/llvm-profgen/ProfileGenerator.cpp
@@ -449,29 +449,56 @@ bool ProfileGeneratorBase::collectFunctionsFromRawProfile(
   // Go through all the stacks, ranges and branches in sample counters, use
   // the start of the range to look up the function it belongs and record the
   // function.
+  uint64_t ErrStkAddr = 0, ErrFuncRange = 0, ErrSrc = 0, ErrTgt = 0;
+  uint64_t TotalStkAddr = 0, TotalFuncRange = 0, TotalSrc = 0, TotalTgt = 0;
   for (const auto &CI : *SampleCounters) {
     if (const auto *CtxKey = dyn_cast<AddrBasedCtxKey>(CI.first.getPtr())) {
       for (auto StackAddr : CtxKey->Context) {
+        uint64_t inc = Binary->addressIsCode(StackAddr) ? 1 : 0;
+        TotalStkAddr += inc;
         if (FuncRange *FRange = Binary->findFuncRange(StackAddr))
           ProfiledFunctions.insert(FRange->Func);
+        else
+          ErrStkAddr += inc;
       }
     }
 
     for (auto Item : CI.second.RangeCounter) {
       uint64_t StartAddress = Item.first.first;
+      uint64_t inc = Binary->addressIsCode(StartAddress) ? 1 : 0;
+      TotalFuncRange += inc;
       if (FuncRange *FRange = Binary->findFuncRange(StartAddress))
         ProfiledFunctions.insert(FRange->Func);
+      else
+        ErrFuncRange += inc;
     }
 
     for (auto Item : CI.second.BranchCounter) {
       uint64_t SourceAddress = Item.first.first;
       uint64_t TargetAddress = Item.first.second;
+      uint64_t srcinc = Binary->addressIsCode(SourceAddress) ? 1 : 0;
+      uint64_t tgtinc = Binary->addressIsCode(TargetAddress) ? 1 : 0;
+      TotalSrc += srcinc;
       if (FuncRange *FRange = Binary->findFuncRange(SourceAddress))
         ProfiledFunctions.insert(FRange->Func);
+      else
+        ErrSrc += srcinc;
+      TotalTgt += tgtinc;
       if (FuncRange *FRange = Binary->findFuncRange(TargetAddress))
         ProfiledFunctions.insert(FRange->Func);
+      else
+        ErrTgt += tgtinc;
     }
   }
+
+  if (ErrStkAddr)
+    WithColor::warning() << "Cannot find Stack Address from DWARF Info: " << ErrStkAddr << "/" << TotalStkAddr << " missing\n";
+  if (ErrFuncRange)
+    WithColor::warning() << "Cannot find Function Range from DWARF Info: " << ErrFuncRange << "/" << TotalFuncRange << " missing\n";
+  if (ErrSrc)
+    WithColor::warning() << "Cannot find LBR Source Addr from DWARF Info: " << ErrSrc << "/" << TotalSrc << " missing\n";
+  if (ErrTgt)
+    WithColor::warning() << "Cannot find LBR Target Addr from DWARF Info: " << ErrTgt << "/" << TotalTgt << " missing\n";
   return true;
 }
 
diff --git a/llvm/tools/llvm-profgen/ProfiledBinary.cpp b/llvm/tools/llvm-profgen/ProfiledBinary.cpp
index 94728ce4abffe..2d9a13b97114c 100644
--- a/llvm/tools/llvm-profgen/ProfiledBinary.cpp
+++ b/llvm/tools/llvm-profgen/ProfiledBinary.cpp
@@ -257,6 +257,8 @@ void ProfiledBinary::load() {
   if (ShowDisassemblyOnly)
     decodePseudoProbe(Obj);
 
+  populateSymbolsFromElf(Obj);
+
   // Disassemble the text sections.
   disassemble(Obj);
 
@@ -820,6 +822,48 @@ void ProfiledBinary::populateSymbolAddressList(const ObjectFile *Obj) {
   }
 }
 
+void ProfiledBinary::populateSymbolsFromElf(
+    const ObjectFile *Obj) {
+  // Load binary functions from ELF symbol table when DWARF info is incomplete
+  StringRef FileName = Obj->getFileName();
+  for (const ELFSymbolRef Symbol : Obj->symbols()) {
+    const SymbolRef::Type Type = unwrapOrError(Symbol.getType(), FileName);
+    const uint64_t Addr = unwrapOrError(Symbol.getAddress(), FileName);
+    const StringRef Name = unwrapOrError(Symbol.getName(), FileName);
+    const uint64_t Size = Symbol.getSize();
+
+    if (Size == 0 || Type != SymbolRef::ST_Function)
+      continue;
+
+    SmallVector<StringRef> Suffixes(
+      {".destroy", ".resume", ".llvm.", ".cold", ".warm"});
+    const StringRef SymName = FunctionSamples::getCanonicalFnName(Name, Suffixes);
+
+    auto Ret = BinaryFunctions.emplace(SymName, BinaryFunction());
+    auto &Func = Ret.first->second;
+    if (Ret.second)
+      Func.FuncName = Ret.first->first;
+
+    if (auto Range = findFuncRange(Addr)) {
+      if (Ret.second && ShowDetailedWarning)
+        WithColor::warning()
+            << "Symbol " << Name << " start address "
+            << format("%8" PRIx64, Addr) << " already exists in DWARF at "
+            << format("%8" PRIx64, Range->StartAddress) << " in function "
+            << Range->getFuncName() << "\n";
+    } else {
+      // Store/Update Function Range from SymTab
+      Func.Ranges.emplace_back(Addr, Addr + Size);
+
+      auto R = StartAddrToFuncRangeMap.emplace(Addr, FuncRange());
+      FuncRange &FRange = R.first->second;
+      FRange.Func = &Func;
+      FRange.StartAddress = Addr;
+      FRange.EndAddress = Addr + Size;
+    }
+  }
+}
+
 void ProfiledBinary::loadSymbolsFromDWARFUnit(DWARFUnit &CompilationUnit) {
   for (const auto &DieInfo : CompilationUnit.dies()) {
     llvm::DWARFDie Die(&CompilationUnit, &DieInfo);
diff --git a/llvm/tools/llvm-profgen/ProfiledBinary.h b/llvm/tools/llvm-profgen/ProfiledBinary.h
index 5a814b7dbd52d..238c27fbc4c9f 100644
--- a/llvm/tools/llvm-profgen/ProfiledBinary.h
+++ b/llvm/tools/llvm-profgen/ProfiledBinary.h
@@ -356,6 +356,9 @@ class ProfiledBinary {
   // Create symbol to its start address mapping.
   void populateSymbolAddressList(const object::ObjectFile *O);
 
+  // Load functions from its symbol table (when DWARF info is missing).
+  void populateSymbolsFromElf(const object::ObjectFile *O);
+
   // A function may be spilt into multiple non-continuous address ranges. We use
   // this to set whether start a function range is the real entry of the
   // function and also set false to the non-function label.

github-actions · 2025-10-15T22:58:00Z

✅ With the latest revision this PR passed the C/C++ code formatter.

dtellenbach · 2025-10-15T23:36:24Z

llvm/tools/llvm-profgen/ProfiledBinary.cpp

  }
 }

+void ProfiledBinary::populateSymbolsFromElf(const ObjectFile *Obj) {


Seems like this is intended to work for ELF only, so it needs to be conditional on the object format being ELF, at least. I think an abstraction in between would be helpful to allow other object formats to implement similar functionality if needed.

Good point! llvm-profgen does support for non-ELF format, e.g. #158207, then this may break other format.

cc @HaohaiWen

Thanks for the feedbacks! Let me fix up the non-ELF compatibility first. I added another SymbolRef::getSize() API to take care of that. Not sure if this is the best way to abstract things, but I'm thinking of following with another NFC patch that renames ObjectFile::getCommonSymbolSizeImpl to ObjectFile::getSymbolSizeImpl. (so this API no longer attaches to the SymbolRef::SF_Common flag?)

… jmp 0)

wlei-llvm

Thanks for adding this feature! Maybe we can add a flag for this, there are multiple modes, presumably we only tested on pseudo-probe mode, we could consider enabling it by default for pseudo-probe mode. And it would be also helpful to test this feature(flag on vs off).

It'd be great to share some numbers in the PR summary, say how much perf is recovered for an overflow case.

wlei-llvm · 2025-10-16T17:33:56Z

llvm/include/llvm/ProfileData/SampleProf.h

    // If suffix "A" is appended after the suffix "B", "A" should be in front
    // of "B" in knownSuffixes.
-    const char *KnownSuffixes[] = {LLVMSuffix, PartSuffix, UniqSuffix};
+    SmallVector<StringRef> KnownSuffixes({LLVMSuffix, PartSuffix, UniqSuffix});


This function is widely used and maybe be expensive. Now with this changing to use vector, I suspect it would introduce more overheads(alloc, dealloc ..), if so, would it possible to continue using the static array or constexpr?

wlei-llvm · 2025-10-16T17:44:28Z

llvm/tools/llvm-profgen/ProfileGenerator.cpp

  for (const auto &CI : *SampleCounters) {
    if (const auto *CtxKey = dyn_cast<AddrBasedCtxKey>(CI.first.getPtr())) {
      for (auto StackAddr : CtxKey->Context) {
+        uint64_t inc = Binary->addressIsCode(StackAddr) ? 1 : 0;


How about doing the stats using the weighted number, i,e. the sample count in SampleCounters
We do this for the other stats(https://github.com/llvm/llvm-project/blob/main/llvm/tools/llvm-profgen/PerfReader.cpp#L1293), it may be more insightful for performance investigations.

wlei-llvm · 2025-10-16T17:45:20Z

llvm/tools/llvm-profgen/ProfileGenerator.cpp

        if (FuncRange *FRange = Binary->findFuncRange(StackAddr))
          ProfiledFunctions.insert(FRange->Func);
+        else
+          ErrStkAddr += inc;


Nit: inc --> Inc , same to other places below.

wlei-llvm · 2025-10-16T17:54:35Z

llvm/tools/llvm-profgen/ProfileGenerator.cpp

  }
+
+  if (ErrStkAddr)
+    WithColor::warning() << "Cannot find Stack Address from DWARF Info: "


Nit: we have a helper function for emitting warning(though may need to rephrase the message info ), https://github.com/llvm/llvm-project/blob/main/llvm/tools/llvm-profgen/PerfReader.cpp#L1325-L1337

Also we have a similar warning in "of samples are from ranges that do not belong to any functions."(https://github.com/llvm/llvm-project/blob/main/llvm/tools/llvm-profgen/PerfReader.cpp#L1328-L1330), the goal should be same as the code here, I wonder if we can consolidate them or perhaps remove the earlier one if it's redundant, that'd be great.

Thanks! I've made an update on the logging! For a comparison, it seems the earlier warning from "Ranges" has a little bit more samples counted comparing to the later warning from the aggregated "SampleCounters". Maybe it's caused by the filtering here?

llvm-project/llvm/tools/llvm-profgen/PerfReader.cpp

Lines 1014 to 1018 in ef46f8a

if (Binary->addressIsCode(StartAddress) &&

Binary->addressIsCode(EndAddress) &&

isValidFallThroughRange(StartAddress, EndAddress, Binary))

Counter.recordRangeCount(StartAddress, EndAddress, Repeat);

EndAddress = SourceAddress;

warning: 0.07%(3501587/4906133148) of samples are from ranges that do not belong to any functions. warning: 0.07%(3500591/4835840652) of function range samples do not belong to any function

wlei-llvm · 2025-10-16T17:57:17Z

llvm/tools/llvm-profgen/ProfiledBinary.cpp

  }
 }

+void ProfiledBinary::populateSymbolsFromElf(const ObjectFile *Obj) {


Good point! llvm-profgen does support for non-ELF format, e.g. #158207, then this may break other format.

cc @HaohaiWen

wlei-llvm · 2025-10-16T18:03:56Z

llvm/tools/llvm-profgen/ProfiledBinary.cpp

+    if (Size == 0 || Type != SymbolRef::ST_Function)
+      continue;
+
+    SmallVector<StringRef> Suffixes(


Similar to my previous comment, this is inside a hot loop, I guess this introduce the alloc/dealloc overheads, we may find a more efficient way(at least we can hoist loop invariant part).

HaohaiWen · 2025-10-21T03:07:21Z

llvm/include/llvm/Object/ObjectFile.h

 }

+inline uint64_t SymbolRef::getSize() const {
+  return getObject()->getCommonSymbolSizeImpl(getRawDataRefImpl());


For COFF. getCommonSymbolSizeImpl returns symbol value which is start address offset of this function.

llvm-project/llvm/lib/Object/COFFObjectFile.cpp

Lines 240 to 243 in 7a54353

uint64_t COFFObjectFile::getCommonSymbolSizeImpl(DataRefImpl Ref) const {

COFFSymbolRef Symb = getCOFFSymbol(Ref);

return Symb.getValue();

}

Ahh, thanks for checking on this! Is this behavior intended for COFF? Because there's another SymbolRef::getCommonSymbolSize that does exactly the same thing but with a assertation check:

llvm-project/llvm/include/llvm/Object/ObjectFile.h

Line 314 in d4713ec

assert(*SymbolFlagsOrErr & SymbolRef::SF_Common);

Would it be better to fix the SymbolRef or the COFF getCommonSymbolSizeImpl to either get the actual symbol size or return 0?

getCommonSymbolSize is used for common symbol.
You can use -fcommon to explicitly generate them. e.g. for a simple global variable int cval;

ELF: Symbol { Name: cval (35) Value: 0x4 Size: 4 Binding: Global (0x1) Type: Object (0x1) Other: 0 Section: Common (0xFFF2) } COFF: Symbol { Name: cval Value: 4 Section: IMAGE_SYM_UNDEFINED (0) BaseType: Null (0x0) ComplexType: Null (0x0) StorageClass: External (0x2) AuxSymbolCount: 0 }

For ELF, there's size filed. size and Value of common symbol are all its size.
For COFF, there's no size field , the symbol's value is its size. I think COFF's implementation is correct.

Regarding function symbol's size.
For ELF, It is the function's size. I think you can use ELFSymbolRef::getSize() safely.
For COFF, it does not support it currently. I think we should use value 0.

I think size for COFF can be supported by PE format, although it has not been supported yet.
https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#auxiliary-format-2-bf-and-ef-symbols
May be we can file another PR to support it.

Gotcha! For now I've added another virtual function ObjectFile::getSymbolSizeImpl that returns 0 by default. The ELFObjectFile overrides this and returns getSymbolSize.

I don't think we should add a new virtual function in ObjectFile since the symbol size is only valid for part of symbol table. That's why there's getSize methond in ELFSymbolRef.
BTW, ObjectFile::getSymbolSizeImpl should return the size for common symbol just like getCommonSize.

How about define a function/lambda to get function symbol's size in ProfiledBinary::populateSymbolsFromBinary to avoid modifying ObjectFile interface?

makes sense. let me change it to ELF-specific then. I did something like this:

// ProfiledBinary::populateSymbolsFromBinary() uint64_t Size = 0; if (isa<ELFObjectFileBase>(Symbol.getObject())) { ELFSymbolRef ElfSymbol(Symbol); Size = ElfSymbol.getSize(); }

HaohaiWen · 2025-10-21T03:08:20Z

llvm/tools/llvm-profgen/ProfiledBinary.cpp

+
+    if (auto Range = findFuncRange(Addr)) {
+      if (Ret.second && ShowDetailedWarning)
+        WithColor::warning()


Will this result in a large number of warnings since most of the symbols can be obtained from DWARF?

I think the Ret.second check shall skip all the symbols that already obtained from DWARF. (the symbol name will exists in the BinaryFunctions and the emplace shall prevent the insertation and Ret.second will be set to "false")

llvm-project/llvm/tools/llvm-profgen/ProfiledBinary.cpp

Line 842 in e12e694

auto Ret = BinaryFunctions.emplace(SymName, BinaryFunction());

HaohaiWen · 2025-10-23T01:24:07Z

I suggest to add some test cases since it's a new feature.

HaohaiWen · 2025-10-24T01:30:57Z

llvm/test/tools/llvm-profgen/missing-dwarf.test

+; RUN: cd %t
+
+; RUN: echo -e "1\n401120-40113b:1\n1\n40112f->401110:1" > %t.prof
+; RUN: cp %S/Inputs/missing-dwarf.exe %t/missing-dwarf.exe


Why need an extra copy here?

Hah, right.... I copied this from another test and forgot to clean it up. This copy is unnecessary. lol

HaohaiWen · 2025-10-24T01:41:32Z

llvm/include/llvm/ProfileData/SampleProf.h

    // If suffix "A" is appended after the suffix "B", "A" should be in front
    // of "B" in knownSuffixes.
-    const char *KnownSuffixes[] = {LLVMSuffix, PartSuffix, UniqSuffix};
+    const char *KnownSuffixes[] = {LLVMSuffix, PartSuffix, UniqSuffix, nullptr};


NIT:

llvm-project/llvm/include/llvm/ADT/ArrayRef.h

Line 128 in 6a0f392

ArrayRef(const iterator_range<U *> &Range)

getCanonicalFnName(FnName, KnownSuffixes, Attr); static StringRef getCanonicalFnName(StringRef FnName, ArrayRef<StringRef> Suffixes, StringRef Attr = "selected") {

HaohaiWen

LGTM for loading functions from .symtab part.

WenleiHe · 2025-10-27T20:27:02Z

llvm/tools/llvm-profgen/ProfileGenerator.cpp

+        uint64_t Inc = Binary->addressIsCode(StackAddr) ? 1 : 0;
+        TotalStkAddr += Inc;


nit for readability:

bool StackAddrIsCode = Binary->addressIsCode(StackAddr); if (StackAddrIsCode) TotalStkAddr++; if (...) ProfiledFunctions.insert(FRange->Func); else if (StackAddrIsCode) ErrStkAddr++; ... same for all other instances.

Also a question, do we expect StackAddrIsCode == false, but findFuncRange has a valid function range?

WenleiHe · 2025-10-27T20:49:45Z

llvm/tools/llvm-profgen/ProfileGenerator.cpp

+  if (ErrStkAddr)
+    emitWarningSummary(
+        ErrStkAddr, TotalStkAddr,
+        "of stack address samples do not belong to any function");
+  if (ErrFuncRange)
+    emitWarningSummary(
+        ErrFuncRange, TotalFuncRange,
+        "of function range samples do not belong to any function");
+  if (ErrSrc)
+    emitWarningSummary(ErrSrc, TotalSrc,
+                       "of LBR source samples do not belong to any function");
+  if (ErrTgt)
+    emitWarningSummary(ErrTgt, TotalTgt,
+                       "of LBR target samples do not belong to any function");


Do we need all of this? range samples, source, target are kind of duplicated info?

If we only track range, can we move the warning back into WarnInvalidRange?

hmmm, that makes sense. It a bit too much logging here, and it was mostly just for debugging at the first place... let me move the warning back :)

WenleiHe · 2025-10-27T20:52:22Z

llvm/tools/llvm-profgen/ProfiledBinary.cpp


+void ProfiledBinary::populateSymbolsFromBinary(const ObjectFile *Obj) {
+  // Load binary functions from symbol table when Debug info is incomplete
+  const SmallVector<StringRef> Suffixes(


This does not need to cover .llvm, .unique, .part... etc?

Add a comment to explain why we care about different suffixes?

WenleiHe · 2025-10-27T20:55:19Z

llvm/tools/llvm-profgen/ProfiledBinary.cpp

+
+    if (auto Range = findFuncRange(Addr)) {
+      if (Ret.second && ShowDetailedWarning)
+        WithColor::warning()


Symbol table finding entry already existing in dwarf should not be a warning.

When that happens, we should check if they both point to same ranges, and only warn when

symbol table and dwarf conflicts for the actual range.

something is in symbol table, but not in dwarf (suggesting dwarf corrupted, e.g. due to relo overflow)

Updated this part and the warning shall now capture case 1. And I've merged (2) into the PerfScriptReader::warnInvalidRange :)

Thanks. Can you test on a big binary to make sure this warning does not fire?

I actually didn't see this warning firing at all in my test, but it shall capture the naming conflicts between dwarf and symbol table, like... in case I missed to strip some internal suffixes...

WenleiHe · 2025-10-27T20:56:24Z

llvm/tools/llvm-profgen/ProfileGenerator.cpp

+  if (ErrFuncRange)
+    emitWarningSummary(
+        ErrFuncRange, TotalFuncRange,
+        "of function range samples do not belong to any function");


ideally, we'd like to how the count that 1) does not belong to any function in Dwarf, and 2) the count that does not belong to any function in Dwarf and symbol table.

This tells us how much loading symbol table is helping, and how much dwarf is screwed (due to relo overflow).

Let me move this message to the ProfiledBinary::populateSymbolsFromBinary instead :)

WenleiHe · 2025-10-27T20:58:35Z

llvm/tools/llvm-profgen/ProfiledBinary.cpp

    cl::cat(ProfGenCategory));

+static cl::opt<bool>
+    LoadFunctionFromSymbol("load-function-from-symbol", cl::init(true),


@wlei-llvm somehow i remember we had something like that before (long time ago)? did we remove the symbol table loading code in the past?

Yeah, iirc, we initially only loaded the function ranges from symbol table, then after we found the issue with the multi-version function(foo.cold, foo.resume...), we redesigned and fully switched to load the symbol from dwarf.

At that time, the code was implemented inside the dissassembleSymbol(which also iterates the symbols),
https://github.com/llvm/llvm-project/blob/main/llvm/tools/llvm-profgen/ProfiledBinary.cpp#L529

For reference, this is the patch https://reviews.llvm.org/D112282 we did the switch to use dwarf symbol.

WenleiHe · 2025-10-27T20:59:03Z

llvm/tools/llvm-profgen/ProfiledBinary.cpp

  if (ShowDisassemblyOnly)
    decodePseudoProbe(Obj);

+  if (LoadFunctionFromSymbol && UsePseudoProbes)


Why is pseudo-probe required here?

Ohh, this is to be compatible with the code here:

llvm-project/llvm/tools/llvm-profgen/ProfileGenerator.cpp

Lines 510 to 514 in af110e1

if (Binary->usePseudoProbes()) {

generateProbeBasedProfile();

} else {

generateLineNumBasedProfile();

}

UsePseudoProbes is needed so we won't use line numbers to generate profile, which won't work with symtab because symtab only have addresses

wlei-llvm · 2025-10-16T18:30:37Z

llvm/tools/llvm-profgen/ProfiledBinary.cpp

+  StringRef FileName = Obj->getFileName();
+  for (const ELFSymbolRef Symbol : Obj->symbols()) {
+    const SymbolRef::Type Type = unwrapOrError(Symbol.getType(), FileName);
+    const uint64_t Addr = unwrapOrError(Symbol.getAddress(), FileName);


Nit: maybe Addr --> StartAddr

wlei-llvm · 2025-10-28T18:57:11Z

llvm/tools/llvm-profgen/ProfiledBinary.cpp

    cl::cat(ProfGenCategory));

+static cl::opt<bool>
+    LoadFunctionFromSymbol("load-function-from-symbol", cl::init(true),


Yeah, iirc, we initially only loaded the function ranges from symbol table, then after we found the issue with the multi-version function(foo.cold, foo.resume...), we redesigned and fully switched to load the symbol from dwarf.

At that time, the code was implemented inside the dissassembleSymbol(which also iterates the symbols),
https://github.com/llvm/llvm-project/blob/main/llvm/tools/llvm-profgen/ProfiledBinary.cpp#L529

wlei-llvm · 2025-10-28T19:54:52Z

llvm/tools/llvm-profgen/ProfileGenerator.cpp

    }
  }
+
+  if (ErrStkAddr)


Nit: No need to check the value here.
There are checks in the emitWarningSummary (https://github.com/llvm/llvm-project/blob/main/llvm/tools/llvm-profgen/ErrorHandling.h#L50)

wlei-llvm · 2025-10-28T19:59:39Z

llvm/tools/llvm-profgen/ProfiledBinary.cpp


+void ProfiledBinary::populateSymbolsFromBinary(const ObjectFile *Obj) {
+  // Load binary functions from symbol table when Debug info is incomplete
+  const SmallVector<StringRef> Suffixes(


nit: SmallVector<StringRef> -> SmallVector<StringRef, 5>

IIUC, the default N is 4 for inlined elements., https://llvm.org/docs/ProgrammersManual.html#llvm-adt-smallvector-h

In the absence of a well-motivated choice for the number of inlined elements N, it is recommended to use SmallVector (that is, omitting the N). This will choose a default number of inlined elements reasonable for allocation on the stack (for example, trying to keep sizeof(SmallVector) around 64 bytes).

WenleiHe

Can you update the change summary? the messages there are now out dated.

WenleiHe · 2025-10-30T23:50:05Z

llvm/tools/llvm-profgen/ProfiledBinary.cpp

+
+    if (auto Range = findFuncRange(Addr)) {
+      if (Ret.second && ShowDetailedWarning)
+        WithColor::warning()


Thanks. Can you test on a big binary to make sure this warning does not fire?

WenleiHe · 2025-10-31T00:11:43Z

llvm/tools/llvm-profgen/ProfiledBinary.cpp

+            << ". The DWARF indicates a range from " << format("%8" PRIx64, Range->StartAddress) << " to "
+            << format("%8" PRIx64, Range->EndAddress) << "\n";
+    } else {
+      // Store/Update Function Range from SymTab


This should only be storing additional range, not "updating" existing range, right?

Also suggest to structure the code like below for clarify:

auto &Func = Ret.first->second; if (Ret.second) { // Function from symbol table not found previously in DWARF, store ranges. Func.FuncName = Ret.first->first; Func.FromSymtab = true; HashBinaryFunctions[MD5Hash(StringRef(SymName))] = &Func; ... } else { // Function already found from DWARF, check consistency between symbol table and DWARF. ... }

Hmm, right.... "updating" is actually not a correct behavior. let me flag that in a warning too :)

actually... we probably still need to "update" the range.. because like, coroutine creates a few functions: "foo", "foo.destroy", "foo.resume", "foo.cleanup", and each of them has a different address range in the symbol table. Even Ret.second == false could still be some function added from the symbol table...

Maybe the StartAddr works better to check if a function is already found in DWARF or not? I'm changing the logic to something like this:

auto Range = findFuncRange(StartAddr); if (!Range || Range->StartAddress != StartAddr) { // Function from symbol table not found previously in DWARF, store ranges. ... } else if (SymName != Range->getFuncName() && ShowDetailedWarning) { // Function already found from DWARF, check consistency between symbol table and DWARF. WithColor::warning()... }

WenleiHe · 2025-10-31T00:14:09Z

llvm/tools/llvm-profgen/ProfiledBinary.cpp

  }
 }

+void ProfiledBinary::populateSymbolsFromBinary(const ObjectFile *Obj) {


this should be loadSymbolsFromSymtab to be consistent with loadSymbolsFromDWARFUnit?

WenleiHe · 2025-11-09T18:47:50Z

llvm/tools/llvm-profgen/ProfiledBinary.cpp

+        FunctionSamples::getCanonicalFnName(Name, Suffixes);
+
+    auto Range = findFuncRange(StartAddr);
+    if (!Range || Range->StartAddress != StartAddr) {


What is the cause for a range from symbol table to be found in dwarf, but with different start address?

Isn't that also an "inconsistent" case that we should warn about as part of the "else"?

So basically, if range for StartAddr is found, we expect the dwarf range and symbol table range to be identical, right?

Even if this is a funclet range (e.g. coroutine.resume), the range should still be identical from the one found in dwarf?

Yup, updated the logic to something like this:

if (!Range) { // Function from symbol table not found previously in DWARF, store ranges. } else if (SymName != Range->getFuncName()) { // Function range already found from DWARF, but the symbol name from symbol table is inconsistent with debug info. assert(StartAddr == Range->StartAddress && EndAddr == Range->EndAddress); // Update the function name... } else if (if (StartAddr != Range->StartAddress && EndAddr != Range->EndAddress) { // Function already found in DWARF, but the address range from symbol table conflicts/overlaps with the debug info. llvm_unreachable(); }

So far I only see symbol name conflicts, and all the address ranges matches between symbol table and dwarf..

WenleiHe · 2025-11-09T18:49:53Z

llvm/tools/llvm-profgen/ProfiledBinary.cpp

+    auto Range = findFuncRange(StartAddr);
+    if (!Range || Range->StartAddress != StartAddr) {
+      // Function from symbol table not found previously in DWARF, store ranges.
+      auto Ret = BinaryFunctions.emplace(SymName, BinaryFunction());


If a range is found, we should expect function to be found. probably want to assert(!Range || !Ret.second).

WenleiHe · 2025-11-09T18:52:06Z

llvm/tools/llvm-profgen/ProfiledBinary.cpp

+      }
+
+      Func.FromSymtab = true;
+      Func.Ranges.emplace_back(StartAddr, StartAddr + Size);


So if a range is found but start address differ, we create a new range, but isn't this creating an overlapping range with existing one (from dwarf)? We should never have two ranges that covers the same address (i.e. ranges must not overlap).

github-actions · 2025-11-17T23:00:06Z

🐧 Linux x64 Test Results

186558 tests passed
4882 tests skipped

wlei-llvm · 2025-11-19T21:23:56Z

llvm/tools/llvm-profgen/ProfiledBinary.cpp

+
+    } else if (SymName != Range->getFuncName()) {
+      // Function range already found from DWARF, but the symbol name from
+      // symbol table is inconsistent with debug info. Log this discrepaency and


discrepaency --> discrepancy

wlei-llvm · 2025-11-19T21:24:30Z

llvm/tools/llvm-profgen/ProfiledBinary.cpp

+    const uint64_t EndAddr = StartAddr + Size;
+    const StringRef SymName =
+        FunctionSamples::getCanonicalFnName(Name, Suffixes);
+    assert(StartAddr < EndAddr && StartAddr >= getPreferredBaseAddress());


Nit: Add the assertion message, same to other assertions

wlei-llvm · 2025-11-19T21:37:36Z

llvm/tools/llvm-profgen/ProfiledBinary.cpp

+      assert(StartAddr == Range->StartAddress && EndAddr == Range->EndAddress &&
+             "Mismatched function range");
+
+      Range->Func->HasSymtabName = true;


HasSymtabName might be confusing. I think every the binaryfunction has the symtab name(it's just whether we use it or not).

How about use an enum instead of the bool to represent the status of symbol name and dwarf name match. Like:

enum class DwarfNameStatus : { Match, Missing, Mismatch, };

Then iiuc:

if the dwarf name is missing, we load the symtab name as the name of binaryfunction

if the dwarf name exists but mismatches symtab name, we still use the dwarf name(possibly invalid), but we store the alternative symtab used later for the Guidfilter.

other invalid cases.

Either way, please update the comments to clarify this. Right now it’s confusing that with HasSymtabName setting to true, but sometimes the name comes from symtab and sometimes from DWARF.

Thanks! Updated with this in the new commit! :)

wlei-llvm · 2025-11-19T22:05:57Z

llvm/tools/llvm-profgen/ProfileGenerator.cpp

+  if (RestoreSymbolName && FRange->Func->HasSymtabName) {
+    const AddressProbesMap &Address2ProbesMap = Binary->getAddress2ProbesMap();
+    for (const MCDecodedPseudoProbe &Probe :
+         Address2ProbesMap.find(TargetAddress)) {


(I'm not 100% sure) but I feel this approach might not work.

IIUC, the pseudo probe is not attached to the prologue instruction of the function, so we won't get the probes for the TargetAddress(unless the prologues are optimized away). Did you see if this condition is hit in your test?

I guess then we need another way to find the first/entry probe from the range.

Also I wonder if we can just fix the FRange-->Func--> funcName in a early time(maybe a central place), the function getCalleeNameForAddress here is called for every branch sample, it would be costly. I'm thinking if we can do it earlier by iterating the function ranges just once, say if the range's function is a dwarf-symbol mismatch(i.e. theHasSymtabName), we can find the entry/first probe and use the probe GUID --> function name to fix the function name in the range, then we can avoid the fix here.

I see. Thanks for checking on that! I updated with a loadSymbolsFromPseudoProbe function. That loads the pseudo probe names based on the function range start address and pseudo probe address. I used a lower_bound to get the first pseudo probe of the function address, and traverse up through the inline tree to get the top-level function.

wlei-llvm · 2025-11-19T22:07:18Z

llvm/tools/llvm-profgen/ProfileGenerator.cpp

    FunctionProfile.addBodySamples(CallProbe->getIndex(), 0, Count);
    FunctionProfile.addTotalSamples(Count);
-    StringRef CalleeName = getCalleeNameForAddress(TargetAddress);
+    StringRef CalleeName = getCalleeNameForAddress(TargetAddress, true);


Also need to support the probe-only(non-cs probe) path.

Fixed! I added loadSymbolsFromPseudoProbe after each decodePseudoProbe call, and looking up the pseudo probe name is now by default in getCalleeNameForAddress

wlei-llvm · 2025-11-19T22:27:03Z

llvm/tools/llvm-profgen/ProfiledBinary.cpp

  } else {
    for (auto *F : ProfiledFunctions) {
      GuidFilter.insert(Function::getGUIDAssumingExternalLinkage(F->FuncName));
+      // Function may have different names in symbol table. Adding


Function may have different names in symbol table. this is not very accurate?(before the fix, we also have the different names). Please make the comment explicit about the problem and the fix.

Updated the comment now, making it explicit that this is to solve the DWARF32 debug_str section exceeding 4GB, and causing an integer overflow..

wlei-llvm · 2025-11-21T23:03:00Z

llvm/tools/llvm-profgen/ProfiledBinary.cpp

+    if (PseudoProbeNames.count(Func))
+      continue;
+#endif
+    const auto &Probe = Address2ProbesMap.find(Addr).begin();


Should we search in the function range(using the end address as the safeguard ) instead of the whole map

I found this from BOLT matching: https://github.com/llvm/llvm-project/pull/100446/files#diff-9c8ed910a52bb3d6726fd342b03b8fd3c77c651a9459531a439bbea725b44279R644-R651

I think we can use the same look-up

auto Probes = Decoder->getAddress2ProbesMap().find(Addr, Addr + Size); ...

Otherwise if it returns something that's not in the current function range, we get a wrong function.

Good point! let me update this!

wlei-llvm · 2025-11-21T23:06:03Z

llvm/tools/llvm-profgen/ProfiledBinary.cpp

+    return;
+
+  const AddressProbesMap &Address2ProbesMap = getAddress2ProbesMap();
+  for (auto &[Addr, Range] : StartAddrToFuncRangeMap) {


Nit: maybe we can iterate over the ProfiledFunctions list, which should be smaller scope.

I'm actually leaning towards iterating over all the Ranges... because I can check Range.IsFuncEntry and skip a lot of non-entry function ranges... The computation complexity is likely still the same...? but BinaryFunction doesn't contain a full FuncRange struct, so checking function entry points won't be that convenient with ProfiledFunctions...

ohh.. wait.. my mistake, you mean the ProfiledFunctions.. let me update this!

ohh.. wait.. my mistake, you mean the ProfiledFunctions.. let me update this!

Yeah, we might just profile a small set of the functions in the binary, that would help. I think you're also right, it needs to find a full FuncRange struct in StartAddrToFuncRangeMap, before I thought it's in the BinaryFunction.Ranges.

wlei-llvm · 2025-11-21T23:10:30Z

llvm/tools/llvm-profgen/ProfiledBinary.cpp

+    if (!Range.IsFuncEntry || Func->NameStatus != DwarfNameStatus::Mismatch)
+      continue;
+#ifndef NDEBUG
+    if (PseudoProbeNames.count(Func))


this is not NFC changes, why put it in to assertion mode? If we want to avoid the repeating, we also need it in release mode.

You are right.. Let me remove this at all! The llvm-profgen run time doesn't seem noticeably longer nor shorter... but now with the benefit of using the following assertations for safe...

wlei-llvm · 2025-11-21T23:14:06Z

llvm/tools/llvm-profgen/ProfiledBinary.cpp

+StringRef ProfiledBinary::findPseudoProbeName(const BinaryFunction *Func) {
+  auto ProbeName = PseudoProbeNames.find(Func);
+  if (ProbeName == PseudoProbeNames.end())
+    return StringRef();
+  return ProbeName->second;
+}


I'm wondering if we really need an extra map. The symbol from pseudo probe should be the ground truth(otherwise it's not loaded by compiler), how about we just overwriting the function name field in BinaryFunction (or is it because the BinaryFunction is immutable in profile generator?)

hmmm, the problem is BinaryFunctions is a map using its original DWARF name as the key.. If I'm going to replace the function name field of BinaryFunction, it probably be good to also update the key? but there're tons of pointer references pointing to the BinaryFunction in this map... It just feels safer (and.. maybe cleaner) to use a separate lookaside map and avoid patching the BinaryFunction at a later stage..?

Ah, that makes sense, thanks for the clarification!

llvm/tools/llvm-profgen/ProfileGenerator.cpp

wlei-llvm · 2025-11-24T18:30:04Z

llvm/tools/llvm-profgen/ProfiledBinary.cpp

+StringRef ProfiledBinary::findPseudoProbeName(const BinaryFunction *Func) {
+  auto ProbeName = PseudoProbeNames.find(Func);
+  if (ProbeName == PseudoProbeNames.end())
+    return StringRef();
+  return ProbeName->second;
+}


Ah, that makes sense, thanks for the clarification!

wlei-llvm · 2025-11-24T18:34:41Z

llvm/tools/llvm-profgen/ProfileGenerator.cpp

  if (!FRange || !FRange->IsFuncEntry)
    return StringRef();

+  auto FuncName = Binary->findPseudoProbeName(FRange->Func);


This probably needs to be under Binary->usePseudoProbes()

also please add a comment for this.

updated! :)

[llvm-profgen] Loading binary functions from .symtab when DWARF info …

f34ab2d

…is incomplete

HighW4y2H3ll requested review from WenleiHe and wlei-llvm October 15, 2025 22:56

llvmbot added the PGO Profile Guided Optimizations label Oct 15, 2025

HighW4y2H3ll and others added 2 commits October 15, 2025 16:07

Merge branch 'main' into fix-profgen-symbol

a1f32b5

formatting

0fd352d

dtellenbach reviewed Oct 15, 2025

View reviewed changes

Fix branch target check when an instruction branches to itself. (i.e.…

c097d37

… jmp 0)

wlei-llvm reviewed Oct 16, 2025

View reviewed changes

wlei-llvm requested review from HaohaiWen and apolloww October 16, 2025 18:16

Making the API compatible with non-ELF binaries

a19064d

llvmbot added the llvm:binary-utilities label Oct 17, 2025

Fix

e12e694

HaohaiWen reviewed Oct 21, 2025

View reviewed changes

Clean up getSymbolSize API and warnings

5eead6b

HighW4y2H3ll added 3 commits October 22, 2025 20:13

Add cmdline option --load-function-from-symbol

8f59bfa

Get symbol size only for ELFObjectFile

a967994

Add unit test

0dc2c66

HaohaiWen reviewed Oct 24, 2025

View reviewed changes

Nit

75dc996

HaohaiWen approved these changes Oct 27, 2025

View reviewed changes

WenleiHe reviewed Oct 27, 2025

View reviewed changes

wlei-llvm reviewed Oct 28, 2025

View reviewed changes

Cleanup loggings and comments

5600e83

WenleiHe reviewed Oct 31, 2025

View reviewed changes

HighW4y2H3ll added 3 commits October 30, 2025 22:47

Mark function FromSymtab too if new range is found

0df504e

Formatting

644fce9

Fix suffix strip && refactor the function checks

9188eff

WenleiHe reviewed Nov 9, 2025

View reviewed changes

HighW4y2H3ll added 4 commits November 11, 2025 23:56

Fixup corrupted DWARF function names using symbol table info

b6ae0ba

Fixup overwritten DWARF symbol name when decoding pseudo probe

b1cbdd0

Fixup GuidFilter && Pseudo probe callee mismatch

8927a27

format

108bc08

HighW4y2H3ll added 2 commits November 17, 2025 15:27

Clean up fixup logic in pseudo probe

6e3a5ac

Further cleanup and add more GuidFilters

3d8ad53

wlei-llvm reviewed Nov 19, 2025

View reviewed changes

Infer callee name with pseudo probe names

1344cdd

wlei-llvm reviewed Nov 21, 2025

View reviewed changes

HighW4y2H3ll added 3 commits November 21, 2025 21:30

promote eligible entry point functions

2978e21

Update pseudo probe search range && range checks

eaab9df

Iterate through ProfiledFunctions to reduce scope

c53e0ae

wlei-llvm reviewed Nov 24, 2025

View reviewed changes

Add usePseudoProbes() guard and comments

b707e80

	if (Binary->addressIsCode(StartAddress) &&
	Binary->addressIsCode(EndAddress) &&
	isValidFallThroughRange(StartAddress, EndAddress, Binary))
	Counter.recordRangeCount(StartAddress, EndAddress, Repeat);
	EndAddress = SourceAddress;

	uint64_t COFFObjectFile::getCommonSymbolSizeImpl(DataRefImpl Ref) const {
	COFFSymbolRef Symb = getCOFFSymbol(Ref);
	return Symb.getValue();
	}

		uint64_t Inc = Binary->addressIsCode(StackAddr) ? 1 : 0;
		TotalStkAddr += Inc;

	if (Binary->usePseudoProbes()) {
	generateProbeBasedProfile();
	} else {
	generateLineNumBasedProfile();
	}

[llvm-profgen] Loading binary functions from .symtab when DWARF info is incomplete #163654

Are you sure you want to change the base?

[llvm-profgen] Loading binary functions from .symtab when DWARF info is incomplete #163654

Uh oh!

Conversation

HighW4y2H3ll commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dtellenbach Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wlei-llvm Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wlei-llvm left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wlei-llvm Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HaohaiWen commented Oct 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HaohaiWen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

HighW4y2H3ll commented Oct 15, 2025 •

edited

Loading

llvmbot commented Oct 15, 2025 •

edited

Loading

github-actions bot commented Oct 15, 2025 •

edited

Loading

dtellenbach Oct 15, 2025 •

edited

Loading

wlei-llvm Oct 16, 2025 •

edited

Loading

wlei-llvm left a comment •

edited

Loading

wlei-llvm Oct 16, 2025 •

edited

Loading

wlei-llvm Oct 29, 2025 •

edited

Loading