[Lldb-commits] [PATCH] D53368: [Symbol] Search symbols with name and type in a symbol file

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov updated this revision to Diff 174916.
aleksandr.urakov added a comment.

Update the patch, move symtab finalization back to object files.

This patch makes object files and symbol files (in the case if they add symbols in a symtab) to be responsible for finalization of a symtab. It's because a symtab is used in a bunch of places, where it's undesirable to retrieve one through a symbol vendor. For example, when the object file itself uses its symtab, we can't retrieve it from the symbol vendor, because the symbol vendor is implemented in the terms of an object file, so such a solution will introduce a circular dependency, which is undesirable.

But on the other hand, if the object file uses its own symtab, then it likely doesn't rely on presence of symbols from the symbol file in that symtab. The only things it requires are symbols from the object file and finalization of the symtab.

So after this update we have the following guarantees:

- if a symtab is retrieved from an object file, then it's consistent and guaranteed contains symbols from the object file. It may (or may not) also contain symbols from a symbol file;
- if a symtab is retrieved from a symbol vendor, then it's consistent and guaranteed contains symbols from an object file and a symbol file.

I've taken a look at the places, where the symtab is retrieved from an object file, and it seems that the only place we need to fix due to that guarantees is the preventive usage of the symbol vendor in the `Address::GetAddressClass` function.

The disadvantages of the current solution are:

- when symbols are added in the symtab both from an object file and a symbol file, the symtab is finalized twice;
- the symtabs retrieved from different places have different guarantees.

But to solve these we need to make some other more higher-level entity (besides the object file) to own the symtab (e.g. symbol vendor) and to rewrite all the related things in object files and symbol files. The problem is that it's not trivial to make it and not to break a lot of current code.

What do you think about this approach?


https://reviews.llvm.org/D53368

Files:
  include/lldb/Symbol/SymbolFile.h
  include/lldb/Symbol/SymbolVendor.h
  source/Core/Address.cpp
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  source/Symbol/SymbolVendor.cpp
  unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D53368.174916.patch
Type: text/x-patch
Size: 6824 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20181121/744be964/attachment-0001.bin>