[llvm-readobj][COFF] Consistent PDBGUID Formatting #94256

mayanez · 2024-06-03T16:53:33Z

Consistent PDB GUID in `llvm-readobj`

Currently, the PDB GUID is shown as a byte array:
PDBGUID: (D8 4C 88 D9 26 15 1F 11 4C 4C 44 20 50 44 42 2E)

This is inconsistent with llvm-pdbutil (e.g. llvm-pdbutil dump --summary) which shows it as a hexadecimal string.
Additionally, yaml2obj uses the same hexadecimal string format.

In general, the hexadecimal string is the common representation for PDB GUIDs on Windows.

This PR changes it to be consistent as shown below:
PDBGUID: {D9884CD8-1526-111F-4C4C-44205044422E}

github-actions · 2024-06-03T16:53:52Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2024-06-03T16:54:29Z

@llvm/pr-subscribers-platform-windows
@llvm/pr-subscribers-debuginfo
@llvm/pr-subscribers-lld
@llvm/pr-subscribers-lld-coff

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

Author: Miguel A. Arroyo (mayanez)

Changes

Consistent PDB GUID in `llvm-readobj`

Currently, the PDB GUID is shown as a byte array:
PDBGUID: (D8 4C 88 D9 26 15 1F 11 4C 4C 44 20 50 44 42 2E)

This is inconsistent with llvm-pdbutil (e.g. llvm-pdbutil dump --summary) which shows it as a hexadecimal string.
Additionally, llvm-yaml2obj uses the same hexadecimal string format.

In general, the hexadecimal string is the common representation for PDB GUIDs on Windows.

This PR changes it to be consistent as shown below:
PDBGUID: {D9884CD8-1526-111F-4C4C-44205044422E}

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

2 Files Affected:

(modified) llvm/test/tools/llvm-readobj/COFF/debug-directory.test (+1-1)
(modified) llvm/tools/llvm-readobj/COFFDumper.cpp (+5-1)

diff --git a/llvm/test/tools/llvm-readobj/COFF/debug-directory.test b/llvm/test/tools/llvm-readobj/COFF/debug-directory.test
index f67eb70d82095..88295679a290a 100644
--- a/llvm/test/tools/llvm-readobj/COFF/debug-directory.test
+++ b/llvm/test/tools/llvm-readobj/COFF/debug-directory.test
@@ -12,7 +12,7 @@ CHECK:     AddressOfRawData: 0x5B068
 CHECK:     PointerToRawData: 0x5A268
 CHECK:     PDBInfo {
 CHECK:       PDBSignature: 0x53445352
-CHECK:       PDBGUID: (96 83 40 42 81 07 9D 40 90 1B 4A 3C 0D 4F 56 32)
+CHECK:       PDBGUID: {42408396-0781-409D-901B-4A3C0D4F5632}
 CHECK:       PDBAge: 3
 CHECK:       PDBFileName: D:\src\llvm\build\has_pdb.pdb
 CHECK:     }
diff --git a/llvm/tools/llvm-readobj/COFFDumper.cpp b/llvm/tools/llvm-readobj/COFFDumper.cpp
index 32b1d6a2089ca..b2939f7826393 100644
--- a/llvm/tools/llvm-readobj/COFFDumper.cpp
+++ b/llvm/tools/llvm-readobj/COFFDumper.cpp
@@ -27,6 +27,7 @@
 #include "llvm/DebugInfo/CodeView/DebugInlineeLinesSubsection.h"
 #include "llvm/DebugInfo/CodeView/DebugLinesSubsection.h"
 #include "llvm/DebugInfo/CodeView/DebugStringTableSubsection.h"
+#include "llvm/DebugInfo/CodeView/Formatters.h"
 #include "llvm/DebugInfo/CodeView/LazyRandomTypeCollection.h"
 #include "llvm/DebugInfo/CodeView/Line.h"
 #include "llvm/DebugInfo/CodeView/MergingTypeTableBuilder.h"
@@ -793,7 +794,10 @@ void COFFDumper::printCOFFDebugDirectory() {
       DictScope PDBScope(W, "PDBInfo");
       W.printHex("PDBSignature", DebugInfo->Signature.CVSignature);
       if (DebugInfo->Signature.CVSignature == OMF::Signature::PDB70) {
-        W.printBinary("PDBGUID", ArrayRef(DebugInfo->PDB70.Signature));
+        SmallString<38> FormattedGUIDStr;
+        raw_svector_ostream OS(FormattedGUIDStr);
+        fmt_guid(DebugInfo->PDB70.Signature).format(OS, "" /*Style=unused*/);
+        W.printString("PDBGUID", FormattedGUIDStr);
         W.printNumber("PDBAge", DebugInfo->PDB70.Age);
         W.printString("PDBFileName", PDBFileName);
       }

mayanez · 2024-06-03T21:42:36Z

@MaskRay @jh7370
Based on the commit history for similar changes your names came up frequently. If this doesn't seem appropriate, I apologize in advance.

Cheers!

rnk

Thanks for the patch, I have a minor suggestion

llvm/tools/llvm-readobj/COFFDumper.cpp

jh7370

This looks reasonable to me, but you should probably wait for @rnk to approve before this gets landed, as I'm not a regular COFF maintainer (I tend to focus on the ELF side of things).

One description nit: there's no such tool as "llvm-yaml2obj" - the tool is simply called "yaml2obj".

rnk

Looks good, sorry to bother with the nits, I just want to get it to a state that I can merge from the web UI.

llvm/lib/DebugInfo/CodeView/Formatters.cpp

llvm/include/llvm/DebugInfo/CodeView/Formatters.h

mayanez · 2024-06-04T16:42:48Z

Looks good, sorry to bother with the nits, I just want to get it to a state that I can merge from the web UI.

Not a problem. I've rebased the changes to the original commit to keep the history tidy.

github-actions · 2024-06-04T16:43:39Z

@mayanez Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested
by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as
the builds can include changes from many authors. It is not uncommon for your
change to be included in a build that fails due to someone else's changes, or
infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself.
This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

llvmbot added the llvm:binary-utilities label Jun 3, 2024

mayanez force-pushed the main branch from 3b1f907 to 3ffca5f Compare June 3, 2024 21:37

mayanez changed the title ~~[llvm-readobj] Consistent PDBGUID Formatting~~ [llvm-readobj][COFF] Consistent PDBGUID Formatting Jun 3, 2024

llvmbot added lld lld:COFF labels Jun 3, 2024

rnk reviewed Jun 3, 2024

View reviewed changes

llvm/tools/llvm-readobj/COFFDumper.cpp Outdated Show resolved Hide resolved

llvmbot added debuginfo platform:windows labels Jun 3, 2024

mayanez force-pushed the main branch from f0308e0 to 8f0e24f Compare June 4, 2024 00:19

jh7370 approved these changes Jun 4, 2024

View reviewed changes

rnk reviewed Jun 4, 2024

View reviewed changes

llvm/lib/DebugInfo/CodeView/Formatters.cpp Outdated Show resolved Hide resolved

llvm/include/llvm/DebugInfo/CodeView/Formatters.h Outdated Show resolved Hide resolved

[llvm-readobj][COFF] Consistent PDBGUID Formatting

b093578

mayanez force-pushed the main branch from cfffb1a to b093578 Compare June 4, 2024 16:42

rnk approved these changes Jun 4, 2024

View reviewed changes

rnk merged commit 0cb66a7 into llvm:main Jun 4, 2024
3 of 5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[llvm-readobj][COFF] Consistent PDBGUID Formatting #94256

[llvm-readobj][COFF] Consistent PDBGUID Formatting #94256

Uh oh!

mayanez commented Jun 3, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Jun 3, 2024

Uh oh!

llvmbot commented Jun 3, 2024 •

edited

Loading

Consistent PDB GUID in `llvm-readobj`

Uh oh!

mayanez commented Jun 3, 2024

Uh oh!

rnk left a comment

Uh oh!

Uh oh!

jh7370 left a comment

Uh oh!

rnk left a comment

Uh oh!

Uh oh!

Uh oh!

mayanez commented Jun 4, 2024

Uh oh!

Uh oh!

github-actions bot commented Jun 4, 2024

Uh oh!

Uh oh!

[llvm-readobj][COFF] Consistent PDBGUID Formatting #94256

[llvm-readobj][COFF] Consistent PDBGUID Formatting #94256

Uh oh!

Conversation

mayanez commented Jun 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Consistent PDB GUID in llvm-readobj

Uh oh!

github-actions bot commented Jun 3, 2024

Uh oh!

llvmbot commented Jun 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Consistent PDB GUID in llvm-readobj

Uh oh!

mayanez commented Jun 3, 2024

Uh oh!

rnk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jh7370 left a comment

Choose a reason for hiding this comment

Uh oh!

rnk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mayanez commented Jun 4, 2024

Uh oh!

Uh oh!

github-actions bot commented Jun 4, 2024

Uh oh!

Uh oh!

mayanez commented Jun 3, 2024 •

edited

Loading

Consistent PDB GUID in `llvm-readobj`

llvmbot commented Jun 3, 2024 •

edited

Loading

Consistent PDB GUID in `llvm-readobj`