[Lldb-commits] [PATCH] D46362: DWARFExpression: Convert file addresses to load addresses early on

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

great fix. Just resolve the file address using the module function and this is good to go.



================
Comment at: source/Core/Value.cpp:683-685
+  ObjectFile *objfile = sc.module_sp->GetObjectFile();
+  if (!objfile)
+    return;
----------------
labath wrote:
> This is not what the original code was doing, but what do you think about getting the section list directly from the `Module`? The two lists aren't exactly the same, but this distinction should not matter here.
> Besides being shorter, this would also make this code work slightly better with object-file-less Modules that Leonard is trying to introduce (D46292 et al.).
Remove these 3 lines


================
Comment at: source/Core/Value.cpp:687
+
+  Address so_addr(file_addr, objfile->GetSectionList());
+  lldb::addr_t load_addr = so_addr.GetLoadAddress(target);
----------------
Let the module resolve the file address for you:

```
Address so_addr;
if (!sc.module_sp->ResolveFileAddress(file_addr, so_addr))
  return;
```


================
Comment at: source/Expression/DWARFExpression.cpp:1385-1387
+      stack.back().ConvertToLoadAddress(
+          frame->GetSymbolContext(eSymbolContextFunction),
+          frame->CalculateTarget().get());
----------------
Be sure there is a test that tests expressions on global variables before we run and before we have a section load list. The result of an expression for a global variable can end up using the file address and currently can read from the object file's data section. Just make sure this still works. Seems like it will. 


https://reviews.llvm.org/D46362