[PATCH] D44407: [DebugInfo] Add a new method IPDBSession::findLineNumbersBySectOffset

[Aaron Smith via Phabricator via llvm-commits]

asmith added inline comments.


================
Comment at: lib/DebugInfo/PDB/DIA/DIASession.cpp:214
   CComPtr<IDiaEnumLineNumbers> LineNumbers;
-  if (S_OK != Session->findLinesByVA(Address, Length, &LineNumbers))
+  if (S_OK != Session->findLinesByVA(Address, Length, &LineNumbers)) {
+    ULONGLONG LoadAddr = 0;
----------------
zturner wrote:
> 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?
Do you also want to delete this code from DIASession::findSymbolByAddress?


Repository:
  rL LLVM

https://reviews.llvm.org/D44407