[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

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

214

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

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

[asmith]

lib/DebugInfo/PDB/DIA/DIASession.cpp

214

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

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;

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;

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;

lib/DebugInfo/PDB/DIA/DIASession.cpp

 DIASession::findSymbolByAddress(uint64_t Address, PDB_SymType Type) const {
   enum SymTagEnum EnumVal = static_cast<enum SymTagEnum>(Type);
 
   CComPtr<IDiaSymbol> Symbol;
   if (S_OK != Session->findSymbolByVA(Address, EnumVal, &Symbol)) {
     ULONGLONG LoadAddr = 0;
     if (S_OK != Session->get_loadAddress(&LoadAddr))
       return nullptr;
+    if (Address < LoadAddr)
+      return nullptr;
     DWORD RVA = static_cast<DWORD>(Address - LoadAddr);
     if (S_OK != Session->findSymbolByRVA(RVA, EnumVal, &Symbol))
       return nullptr;
   }
   auto RawSymbol = llvm::make_unique<DIARawSymbol>(*this, Symbol);
   return PDBSymbol::create(*this, std::move(RawSymbol));
 }
 
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;
+    if (Address < 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,

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>

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,