[PDB][NativeSession] Use better error code for invalid format

[MuellerMP]

Replaces the default "Success" std::error_code with a more meaningful one if Magic != file_magic::pdb.

[llvmbot]

@llvm/pr-subscribers-debuginfo

@llvm/pr-subscribers-platform-windows

Author: Mirko (MuellerMP)

Changes

Replaces the default "Success" std::error_code with a more meaningful one if Magic != file_magic::pdb.

---

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

1 Files Affected:

• (modified) llvm/lib/DebugInfo/PDB/Native/NativeSession.cpp (+8-2)

diff --git a/llvm/lib/DebugInfo/PDB/Native/NativeSession.cpp b/llvm/lib/DebugInfo/PDB/Native/NativeSession.cpp
index 8967a2eb1749e..982153beb32b4 100644
--- a/llvm/lib/DebugInfo/PDB/Native/NativeSession.cpp
+++ b/llvm/lib/DebugInfo/PDB/Native/NativeSession.cpp
@@ -99,9 +99,12 @@ loadPdbFile(StringRef PdbPath, std::unique_ptr<BumpPtrAllocator> &Allocator) {
   PdbPath = Buffer->getBufferIdentifier();
   file_magic Magic;
   auto EC = identify_magic(PdbPath, Magic);
-  if (EC || Magic != file_magic::pdb)
+  if (EC)
     return make_error<RawError>(EC);
 
+  if (Magic != file_magic::pdb)
+    return make_error<RawError>(raw_error_code::invalid_format);
+
   auto Stream = std::make_unique<MemoryBufferByteStream>(
       std::move(Buffer), llvm::endianness::little);
 
@@ -154,9 +157,12 @@ Error NativeSession::createFromExe(StringRef ExePath,
 
   file_magic Magic;
   auto EC = identify_magic(PdbPath.get(), Magic);
-  if (EC || Magic != file_magic::pdb)
+  if (EC)
     return make_error<RawError>(EC);
 
+  if (Magic != file_magic::pdb)
+    return make_error<RawError>(raw_error_code::invalid_format);
+
   auto Allocator = std::make_unique<BumpPtrAllocator>();
   auto File = loadPdbFile(PdbPath.get(), Allocator);
   if (!File)

[MuellerMP]

This fixes the case where identify_magic(PdbPath, Magic) can identify a Magic, but its not the PDB one (e.g. input is a ascii text file).
Can also add a unit test where i input the SimpleTest.cpp and assert that the message is not the same as a default constructed std::error_code if required.

[aganea]

Can also add a unit test where i input the SimpleTest.cpp and assert that the message is not the same as a default constructed std::error_code if required.

Yes, please add a test.

[MuellerMP]

Can also add a unit test where i input the SimpleTest.cpp and assert that the message is not the same as a default constructed std::error_code if required.

Yes, please add a test.

done -> also improved the error message and reduced the original code duplication a bit

[MuellerMP]

Hey @aganea - would you perhaps have time to review this?

[aganea]

LGTM, just a nit:

[aganea]

You can write if (auto EC = ... ) to make it shorter.

[MuellerMP]

thanks - made it shorter

sorry for the amount of commit updates - i messed up my local git config

[MuellerMP]

@aganea thanks for the review!
Would it be possible that you merge this for me since I don't have commit access?