[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.73%(338851792/5033840248) of function range samples do not belong to any function
warning: 6.80%(347897624/5112382988) of LBR source samples do not belong to any function
warning: 6.74%(350108613/5196409152) of LBR target samples do not belong to any function

# After
warning: 0.07%(3500591/4835840652) of function range samples do not belong to any function
warning: 0.07%(3614033/4910228185) of LBR source samples do not belong to any function
warning: 0.07%(3612799/4991681753) of LBR target samples do not belong to any function

…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

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff origin/main HEAD --extensions h,cpp -- llvm/include/llvm/ProfileData/SampleProf.h llvm/tools/llvm-profgen/PerfReader.cpp llvm/tools/llvm-profgen/ProfiledBinary.cpp llvm/tools/llvm-profgen/ProfiledBinary.h --diff_from_common_commit

⚠️
The reproduction instructions above might return results for more than one PR
in a stack if you are using a stacked PR workflow. You can limit the results by
changing origin/main to the base branch/commit you want to compare against.
⚠️

View the diff from clang-format here.

diff --git a/llvm/tools/llvm-profgen/PerfReader.cpp b/llvm/tools/llvm-profgen/PerfReader.cpp
index 1c4b79554..a8a9c6eda 100644
--- a/llvm/tools/llvm-profgen/PerfReader.cpp
+++ b/llvm/tools/llvm-profgen/PerfReader.cpp
@@ -1332,9 +1332,9 @@ void PerfScriptReader::warnInvalidRange() {
   emitWarningSummary(
       UnmatchedRange, TotalRangeNum,
       "of samples are from ranges that do not belong to any functions.");
-  emitWarningSummary(
-      RecoveredRange, TotalRangeNum,
-      "of samples are from ranges that belong to functions recovered from symbol table.");
+  emitWarningSummary(RecoveredRange, TotalRangeNum,
+                     "of samples are from ranges that belong to functions "
+                     "recovered from symbol table.");
   emitWarningSummary(
       RangeCrossFunc, TotalRangeNum,
       "of samples are from ranges that do cross function boundaries.");
diff --git a/llvm/tools/llvm-profgen/ProfiledBinary.cpp b/llvm/tools/llvm-profgen/ProfiledBinary.cpp
index e65bafa9a..8b2072ee4 100644
--- a/llvm/tools/llvm-profgen/ProfiledBinary.cpp
+++ b/llvm/tools/llvm-profgen/ProfiledBinary.cpp
@@ -833,15 +833,17 @@ void ProfiledBinary::populateSymbolAddressList(const ObjectFile *Obj) {
 void ProfiledBinary::populateSymbolsFromBinary(const ObjectFile *Obj) {
   // Load binary functions from symbol table when Debug info is incomplete.
   // Strip the internal suffixes which are not reflected in the DWARF info.
-  const SmallVector<StringRef, 6> Suffixes(
-      {
-        // Internal suffixes from CoroSplit pass
-        ".cleanup", ".destroy", ".resume",
-        // Internal suffixes from Bolt
-        ".cold", ".warm",
-        // Compiler internal
-        ".llvm.",
-      });
+  const SmallVector<StringRef, 6> Suffixes({
+      // Internal suffixes from CoroSplit pass
+      ".cleanup",
+      ".destroy",
+      ".resume",
+      // Internal suffixes from Bolt
+      ".cold",
+      ".warm",
+      // Compiler internal
+      ".llvm.",
+  });
   StringRef FileName = Obj->getFileName();
   for (const SymbolRef &Symbol : Obj->symbols()) {
     const SymbolRef::Type Type = unwrapOrError(Symbol.getType(), FileName);
@@ -871,8 +873,10 @@ void ProfiledBinary::populateSymbolsFromBinary(const ObjectFile *Obj) {
       if (Ret.second && Range->getFuncName() != SymName && ShowDetailedWarning)
         WithColor::warning()
             << "Conflicting symbol " << Name << " already exists in DWARF as "
-            << Range->getFuncName() << " at address " << format("%8" PRIx64, StartAddr)
-            << ". The DWARF indicates a range from " << format("%8" PRIx64, Range->StartAddress) << " to "
+            << Range->getFuncName() << " at address "
+            << format("%8" PRIx64, StartAddr)
+            << ". 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

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 :)

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

[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

	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

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

Uh oh!

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