Weird, I don’t remember it :)<br><br>In any case, can you make a common function called findSymbolByVAOrRVA?  Then have every caller use it<br><div class="gmail_quote"><div dir="ltr">On Tue, Mar 13, 2018 at 8:43 PM Aaron Smith via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">asmith added inline comments.<br>
<br>
<br>
================<br>
Comment at: lib/DebugInfo/PDB/DIA/DIASession.cpp:214<br>
   CComPtr<IDiaEnumLineNumbers> LineNumbers;<br>
-  if (S_OK != Session->findLinesByVA(Address, Length, &LineNumbers))<br>
+  if (S_OK != Session->findLinesByVA(Address, Length, &LineNumbers)) {<br>
+    ULONGLONG LoadAddr = 0;<br>
----------------<br>
asmith wrote:<br>
> zturner wrote:<br>
> > 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:<br>
> ><br>
> > * Why would Address be less than LoadAddr?  That suggests you're calling this function with an invalid argument.<br>
> > * If findByVA failed, why would findByRVA succeed?  That seems like a violation of DIA's API guarantees.<br>
> ><br>
> > I think this method should have the same semantics as the DIA method and the same expectations.<br>
> ><br>
> > 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?<br>
> Do you also want to delete this code from DIASession::findSymbolByAddress?<br>
Looks like your commit added the logic to search by VA then RVA.<br>
<br>
commit 7c69a5821492050ce95eb489a6411b8320277405<br>
Author: Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>><br>
Date:   Fri May 1 20:24:26 2015 +0000<br>
<br>
+  if (S_OK != Session->findSymbolByVA(Address, EnumVal, &Symbol)) {<br>
+    ULONGLONG LoadAddr = 0;<br>
+    if (S_OK != Session->get_loadAddress(&LoadAddr))<br>
+      return nullptr;<br>
+    DWORD RVA = static_cast<DWORD>(Address - LoadAddr);<br>
+    if (S_OK != Session->findSymbolByRVA(RVA, EnumVal, &Symbol))<br>
+      return nullptr;<br>
+  }<br>
<br>
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="https://reviews.llvm.org/D44407" rel="noreferrer" target="_blank">https://reviews.llvm.org/D44407</a><br>
<br>
<br>
<br>
</blockquote></div>