Skip to content

Conversation

@nikitalita
Copy link
Contributor

@nikitalita nikitalita commented Feb 20, 2024

This fixes a bug where parsing PDBs with usages of register enums were asserting.

The main problem is that printing out the code view register enums are taken care of here:
https://github.com/nikitalita/llvm-project/blob/e4888a92402f53000a3a5e79d3792c034fc2f343/llvm/lib/ObjectYAML/CodeViewYAMLSymbols.cpp#L152

Which requires a COFF::header in the IO context for the machine type, which we didn't have when dumping a pdb or parsing a yaml file. So, we make a fake one with the machine type.

@llvmbot
Copy link
Member

llvmbot commented Feb 20, 2024

@llvm/pr-subscribers-debuginfo

Author: None (nikitalita)

Changes

The main problem that we were encountering is that printing out the code view register enums are taken care of here:
https://github.com/nikitalita/llvm-project/blob/e4888a92402f53000a3a5e79d3792c034fc2f343/llvm/lib/ObjectYAML/CodeViewYAMLSymbols.cpp#L152

Which requires a COFF::header in the IO context for the machine type, which we didn't have when dumping a pdb or parsing a yaml file. So, we make a fake one with the machine type.


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

3 Files Affected:

  • (modified) llvm/tools/llvm-pdbutil/PdbYaml.cpp (+7)
  • (modified) llvm/tools/llvm-pdbutil/PdbYaml.h (+2)
  • (modified) llvm/tools/llvm-pdbutil/YAMLOutputStyle.cpp (+9)
diff --git a/llvm/tools/llvm-pdbutil/PdbYaml.cpp b/llvm/tools/llvm-pdbutil/PdbYaml.cpp
index a26241967b5add..fac1d89321610b 100644
--- a/llvm/tools/llvm-pdbutil/PdbYaml.cpp
+++ b/llvm/tools/llvm-pdbutil/PdbYaml.cpp
@@ -155,6 +155,13 @@ void MappingTraits<PdbDbiStream>::mapping(IO &IO, PdbDbiStream &Obj) {
   IO.mapOptional("PdbDllRbld", Obj.PdbDllRbld, uint16_t(0U));
   IO.mapOptional("Flags", Obj.Flags, uint16_t(1U));
   IO.mapOptional("MachineType", Obj.MachineType, PDB_Machine::x86);
+  // This is a workaround for IO not having document context with the
+  // machine type. The machine type is needed to properly parse Register enums
+  // in the PDB.
+  if (!IO.getContext()) {
+    Obj.FakeHeader.Machine = static_cast<uint16_t>(Obj.MachineType);
+    IO.setContext(&Obj.FakeHeader);
+  }
   IO.mapOptional("Modules", Obj.ModInfos);
 }
 
diff --git a/llvm/tools/llvm-pdbutil/PdbYaml.h b/llvm/tools/llvm-pdbutil/PdbYaml.h
index 4382e91e209737..b0c0f6a00c499b 100644
--- a/llvm/tools/llvm-pdbutil/PdbYaml.h
+++ b/llvm/tools/llvm-pdbutil/PdbYaml.h
@@ -11,6 +11,7 @@
 
 #include "OutputStyle.h"
 
+#include "llvm/BinaryFormat/COFF.h"
 #include "llvm/DebugInfo/CodeView/SymbolRecord.h"
 #include "llvm/DebugInfo/CodeView/TypeRecord.h"
 #include "llvm/DebugInfo/MSF/MSFCommon.h"
@@ -80,6 +81,7 @@ struct PdbDbiStream {
   PDB_Machine MachineType = PDB_Machine::x86;
 
   std::vector<PdbDbiModuleInfo> ModInfos;
+  COFF::header FakeHeader;
 };
 
 struct PdbTpiStream {
diff --git a/llvm/tools/llvm-pdbutil/YAMLOutputStyle.cpp b/llvm/tools/llvm-pdbutil/YAMLOutputStyle.cpp
index 80b76657facc7c..ecb4c2175e49a3 100644
--- a/llvm/tools/llvm-pdbutil/YAMLOutputStyle.cpp
+++ b/llvm/tools/llvm-pdbutil/YAMLOutputStyle.cpp
@@ -11,6 +11,7 @@
 #include "PdbYaml.h"
 #include "llvm-pdbutil.h"
 
+#include "llvm/BinaryFormat/COFF.h"
 #include "llvm/DebugInfo/CodeView/DebugChecksumsSubsection.h"
 #include "llvm/DebugInfo/CodeView/DebugSubsection.h"
 #include "llvm/DebugInfo/CodeView/DebugUnknownSubsection.h"
@@ -73,7 +74,15 @@ Error YAMLOutputStyle::dump() {
   if (auto EC = dumpPublics())
     return EC;
 
+  // Fake Coff header for dumping register enumerations.
+  COFF::header Header;
+  auto MachineType =
+      Obj.DbiStream ? Obj.DbiStream->MachineType : PDB_Machine::Unknown;
+  Header.Machine = static_cast<uint16_t>(MachineType);
+  Out.setContext(&Header);
   flush();
+  Out.setContext(nullptr);
+
   return Error::success();
 }
 

@github-actions
Copy link

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Discourse for more information.

@nikitalita nikitalita changed the title [lldb-pdbutil] Fix register enum field dumping/parsing [llvm-pdbutil] Fix register enum field dumping/parsing Feb 20, 2024
@nikitalita
Copy link
Contributor Author

fixes #58916

@tru
Copy link
Collaborator

tru commented Apr 10, 2024

I think we need some kind of test to cover this case.

@nikitalita

This comment was marked as outdated.

@nikitalita
Copy link
Contributor Author

Never mind, I found one and I have added unit tests

@nikitalita
Copy link
Contributor Author

The tests are passing now.

@nikitalita
Copy link
Contributor Author

@tru can you please take a look?

@tru
Copy link
Collaborator

tru commented Oct 16, 2024

I don't know enough about the PDB spec to say if this is the best way to solve it. The code looks fine enough.

Maybe @mstorsjo @aganea or @rnk could help

Copy link
Collaborator

@rnk rnk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good.

@nikitalita nikitalita force-pushed the Fix-pdb-register-enums branch from e56606b to 9bdafa5 Compare May 23, 2025 08:52
@nikitalita
Copy link
Contributor Author

@rnk I have rebased on master, can you push the button?

@rnk rnk merged commit 14c2fa2 into llvm:main Jun 2, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants