[DebugInfo] Add a new method IPDBSession::findLineNumbersBySectOffset

[asmith]

Summary

Some PDB symbols do not have a valid VA or RVA but have Addr by Section and Offset. For example, a variable in thread-local storage has the following properties:

get_addressOffset: 0
get_addressSection: 5
get_lexicalParentId: 2
get_name: g_tls
get_symIndexId: 12
get_typeId: 4
get_dataKind: 6
get_symTag: 7
get_locationType: 2

This change provides a new method to locate line numbers by Section and Offset from those symbols.

[zturner]

lib/DebugInfo/PDB/DIA/DIASession.cpp

185 ↗

(On Diff #138117)

Why is this needed? This seems like the user has passed in an invalid address. Can we remove this?

214 ↗

(On Diff #138117)

I'm not sure it's worth putting this extra complexity into this function. Now the function is not so simple to reason about anymore. For example:

• Why would Address be less than LoadAddr? That suggests you're calling this function with an invalid argument.
• If findByVA failed, why would findByRVA succeed? That seems like a violation of DIA's API guarantees.

I think this method should have the same semantics as the DIA method and the same expectations.

Also, whatever the reason for this, it seems orthogonal to the goal of adding a method to find lines by section and offset, which is entirely handled by the new findLineNumbersBySectOffset method. Can we undo these changes?

[asmith]

This comment was removed by asmith.

[asmith]

lib/DebugInfo/PDB/DIA/DIASession.cpp

214 ↗

(On Diff #138117)

Do you also want to delete this code from DIASession::findSymbolByAddress?

[asmith]

lib/DebugInfo/PDB/DIA/DIASession.cpp

214 ↗

(On Diff #138117)

Looks like your commit added the logic to search by VA then RVA.

commit 7c69a5821492050ce95eb489a6411b8320277405
Author: Zachary Turner <[e-mail]>
Date: Fri May 1 20:24:26 2015 +0000

+ if (S_OK != Session->findSymbolByVA(Address, EnumVal, &Symbol)) {
+ ULONGLONG LoadAddr = 0;
+ if (S_OK != Session->get_loadAddress(&LoadAddr))
+ return nullptr;
+ DWORD RVA = static_cast<DWORD>(Address - LoadAddr);
+ if (S_OK != Session->findSymbolByRVA(RVA, EnumVal, &Symbol))
+ return nullptr;
+ }

[zturner]

Weird, I don’t remember it :)

In any case, can you make a common function called findSymbolByVAOrRVA?
Then have every caller use it

[asmith]

findLineNumbersByAddress() uses findLinesByVA/RVA and findSymbolByAddress() uses findSymbolByVA/RVA.

Do you still want them factored out into two methods: findLinesByVAOrRVA and findSymbolByVAOrRVA?

[zturner]

Ahh i see, maybe there is no way to avoid the duplication then. Still wish
I remembered why I did that, because now the code looks odd.

Anyway, lgtm

llvm/trunk/include/llvm/DebugInfo/PDB/DIA/DIASession.h

public:
   std::unique_ptr<PDBSymbol>
   findSymbolByAddress(uint64_t Address, PDB_SymType Type) const override;
 
   std::unique_ptr<IPDBEnumLineNumbers>
   findLineNumbers(const PDBSymbolCompiland &Compiland,
                   const IPDBSourceFile &File) const override;
   std::unique_ptr<IPDBEnumLineNumbers>
   findLineNumbersByAddress(uint64_t Address, uint32_t Length) const override;
+  std::unique_ptr<IPDBEnumLineNumbers>
+  findLineNumbersBySectOffset(uint32_t Section, uint32_t Offset,
+                              uint32_t Length) const override;
 
   std::unique_ptr<IPDBEnumSourceFiles>
   findSourceFiles(const PDBSymbolCompiland *Compiland, llvm::StringRef Pattern,
                   PDB_NameSearchFlags Flags) const override;
   std::unique_ptr<IPDBSourceFile>
   findOneSourceFile(const PDBSymbolCompiland *Compiland,
                     llvm::StringRef Pattern,
                     PDB_NameSearchFlags Flags) const override;

llvm/trunk/include/llvm/DebugInfo/PDB/IPDBSession.h

public:
   virtual std::unique_ptr<PDBSymbol>
   findSymbolByAddress(uint64_t Address, PDB_SymType Type) const = 0;
 
   virtual std::unique_ptr<IPDBEnumLineNumbers>
   findLineNumbers(const PDBSymbolCompiland &Compiland,
                   const IPDBSourceFile &File) const = 0;
   virtual std::unique_ptr<IPDBEnumLineNumbers>
   findLineNumbersByAddress(uint64_t Address, uint32_t Length) const = 0;
+  virtual std::unique_ptr<IPDBEnumLineNumbers>
+  findLineNumbersBySectOffset(uint32_t Section, uint32_t Offset,
+                              uint32_t Length) const = 0;
 
   virtual std::unique_ptr<IPDBEnumSourceFiles>
   findSourceFiles(const PDBSymbolCompiland *Compiland, llvm::StringRef Pattern,
                   PDB_NameSearchFlags Flags) const = 0;
   virtual std::unique_ptr<IPDBSourceFile>
   findOneSourceFile(const PDBSymbolCompiland *Compiland,
                     llvm::StringRef Pattern,
                     PDB_NameSearchFlags Flags) const = 0;

llvm/trunk/include/llvm/DebugInfo/PDB/Native/NativeSession.h

public:
   std::unique_ptr<PDBSymbol>
   findSymbolByAddress(uint64_t Address, PDB_SymType Type) const override;
 
   std::unique_ptr<IPDBEnumLineNumbers>
   findLineNumbers(const PDBSymbolCompiland &Compiland,
                   const IPDBSourceFile &File) const override;
   std::unique_ptr<IPDBEnumLineNumbers>
   findLineNumbersByAddress(uint64_t Address, uint32_t Length) const override;
+  std::unique_ptr<IPDBEnumLineNumbers>
+  findLineNumbersBySectOffset(uint32_t Section, uint32_t Offset,
+                              uint32_t Length) const override;
 
   std::unique_ptr<IPDBEnumSourceFiles>
   findSourceFiles(const PDBSymbolCompiland *Compiland, llvm::StringRef Pattern,
                   PDB_NameSearchFlags Flags) const override;
   std::unique_ptr<IPDBSourceFile>
   findOneSourceFile(const PDBSymbolCompiland *Compiland,
                     llvm::StringRef Pattern,
                     PDB_NameSearchFlags Flags) const override;

llvm/trunk/lib/DebugInfo/PDB/DIA/DIASession.cpp

if (S_OK !=
     return nullptr;
 
   return llvm::make_unique<DIAEnumLineNumbers>(LineNumbers);
 }
 
 std::unique_ptr<IPDBEnumLineNumbers>
 DIASession::findLineNumbersByAddress(uint64_t Address, uint32_t Length) const {
   CComPtr<IDiaEnumLineNumbers> LineNumbers;
-  if (S_OK != Session->findLinesByVA(Address, Length, &LineNumbers))
+  if (S_OK != Session->findLinesByVA(Address, Length, &LineNumbers)) {
+    ULONGLONG LoadAddr = 0;
+    if (S_OK != Session->get_loadAddress(&LoadAddr))
+      return nullptr;
+    DWORD RVA = static_cast<DWORD>(Address - LoadAddr);
+    if (S_OK != Session->findLinesByRVA(RVA, Length, &LineNumbers))
+      return nullptr;
+  }
+  return llvm::make_unique<DIAEnumLineNumbers>(LineNumbers);
+}
+
+std::unique_ptr<IPDBEnumLineNumbers>
+DIASession::findLineNumbersBySectOffset(uint32_t Section, uint32_t Offset,
+                                        uint32_t Length) const {
+  CComPtr<IDiaEnumLineNumbers> LineNumbers;
+  if (S_OK != Session->findLinesByAddr(Section, Offset, Length, &LineNumbers))
     return nullptr;
 
   return llvm::make_unique<DIAEnumLineNumbers>(LineNumbers);
 }
 
 std::unique_ptr<IPDBEnumSourceFiles>
 DIASession::findSourceFiles(const PDBSymbolCompiland *Compiland,
                             llvm::StringRef Pattern,

llvm/trunk/lib/DebugInfo/PDB/Native/NativeSession.cpp

 }
 
 std::unique_ptr<IPDBEnumLineNumbers>
 NativeSession::findLineNumbersByAddress(uint64_t Address,
                                         uint32_t Length) const {
   return nullptr;
 }
 
+std::unique_ptr<IPDBEnumLineNumbers>
+NativeSession::findLineNumbersBySectOffset(uint32_t Section, uint32_t Offset,
+                                           uint32_t Length) const {
+  return nullptr;
+}
+
 std::unique_ptr<IPDBEnumSourceFiles>
 NativeSession::findSourceFiles(const PDBSymbolCompiland *Compiland,
                                StringRef Pattern,
                                PDB_NameSearchFlags Flags) const {
   return nullptr;
 }
 
 std::unique_ptr<IPDBSourceFile>

llvm/trunk/unittests/DebugInfo/PDB/PDBApiTest.cpp

class MockSession : public IPDBSession {
   findLineNumbers(const PDBSymbolCompiland &Compiland,
                   const IPDBSourceFile &File) const override {
     return nullptr;
   }
   std::unique_ptr<IPDBEnumLineNumbers>
   findLineNumbersByAddress(uint64_t Address, uint32_t Length) const override {
     return nullptr;
   }
+  std::unique_ptr<IPDBEnumLineNumbers>
+  findLineNumbersBySectOffset(uint32_t Section, uint32_t Offset,
+                              uint32_t Length) const override {
+    return nullptr;
+  }
   std::unique_ptr<IPDBEnumSourceFiles>
   findSourceFiles(const PDBSymbolCompiland *Compiland, llvm::StringRef Pattern,
                   PDB_NameSearchFlags Flags) const override {
     return nullptr;
   }
   std::unique_ptr<IPDBSourceFile>
   findOneSourceFile(const PDBSymbolCompiland *Compiland,
                     llvm::StringRef Pattern,