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

On 29/11/2018 23:34, Greg Clayton wrote: On Nov 29, 2018, at 10:55 AM, Pavel Labath via Phabricator wrote: labath added a comment. I've recently started looking at adding a new symbol file format (breakpad symbols). While researching the best way to achieve that, I started comparing the

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL347960: [Symbol] Search symbols with name and type in a symbol file (authored by aleksandr.urakov, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

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

> On Nov 29, 2018, at 3:31 PM, Zachary Turner wrote: > > Objects have a symbol table, but a Module is just a loaded object file. And a > loaded object file can have multiple symbol tables (from the object file, or > symbol file, etc). > > Would it make sense to have Module provide a

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

Objects have a symbol table, but a Module is just a loaded object file. And a loaded object file can have multiple symbol tables (from the object file, or symbol file, etc). Would it make sense to have Module provide a GetSymtab method that can find all appropriate Symtabs and present a unified

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

> On Nov 29, 2018, at 2:02 PM, Zachary Turner via Phabricator > wrote: > > zturner added a comment. > > In D53368#1313238 , @labath wrote: > >> In D53368#1313145 , @zturner wrote: >> >>> In D53368#1313124

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

> On Nov 29, 2018, at 10:55 AM, Pavel Labath via Phabricator > wrote: > > labath added a comment. > > I've recently started looking at adding a new symbol file format (breakpad > symbols). While researching the best way to achieve that, I started comparing > the operation of PDB and DWARF

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

zturner added a comment. In D53368#1313238 , @labath wrote: > In D53368#1313145 , @zturner wrote: > > > In D53368#1313124 , @labath wrote: > > > > > I've recently started

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

labath added a comment. On 29/11/2018 21:29, Leonard Mosescu wrote: > Hi Aleksandr, yes, no objections to this patch. > > I was responding to Pavel's comments, which I also assume are > forward-looking as well, not strictly related to this patch. Agreed, and I apologise for hijacking your

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

labath added a comment. > Great observations Pavel! I think it's really important to have > orthogonal/composable abstractions here: the symbols should be decoupled > from the container format IMO. > > You know more about the ObjectFile than me so I can't say if > ObjectFileBreakpad is the

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

Hi Aleksandr, yes, no objections to this patch. I was responding to Pavel's comments, which I also assume are forward-looking as well, not strictly related to this patch. On Thu, Nov 29, 2018 at 12:08 PM Aleksandr Urakov < aleksandr.ura...@jetbrains.com> wrote: > Yes, I agree that the current

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

labath added a comment. In D53368#1313145 , @zturner wrote: > In D53368#1313124 , @labath wrote: > > > I've recently started looking at adding a new symbol file format (breakpad > > symbols). While researching the

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

Yes, I agree that the current model of object files - symbol files - symtabs is not clear and orthogonal for cases like PDB, and I think that it requires some redesign too. But can we commit this patch now to proceed with several dependent (and not directly related to the theme) patches? When a

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

Great observations Pavel! I think it's really important to have orthogonal/composable abstractions here: the symbols should be decoupled from the container format IMO. You know more about the ObjectFile than me so I can't say if ObjectFileBreakpad is the best interface, but here are my initial

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

zturner added a comment. In D53368#1313124 , @labath wrote: > I've recently started looking at adding a new symbol file format (breakpad > symbols). While researching the best way to achieve that, I started comparing > the operation of PDB and DWARF

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

labath added a comment. I've recently started looking at adding a new symbol file format (breakpad symbols). While researching the best way to achieve that, I started comparing the operation of PDB and DWARF symbol files. I noticed a very important difference there, and I think that is the

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

aleksandr.urakov added a comment. Ping! Can you take a look, please? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53368/new/ https://reviews.llvm.org/D53368 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

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

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.

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

aleksandr.urakov added a comment. Yes, sure. It happens in the following functions: - `Module::ResolveSymbolContextForAddress` - `DynamicLoaderHexagonDYLD::SetRendezvousBreakpoint` through `findSymbolAddress` - `DynamicLoaderHexagonDYLD::RendezvousBreakpointHit` through `findSymbolAddress` -

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

clayborg added a comment. So it depends on what code was retrieving the symbol table from the object file. Can you detail where this was happening? Repository: rLLDB LLDB https://reviews.llvm.org/D53368 ___ lldb-commits mailing list

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

aleksandr.urakov reopened this revision. aleksandr.urakov added a comment. This revision is now accepted and ready to land. @davide You are right, this patch was the cause of the failure, sorry for that. It seems that I've found a generic issue with this patch. Thanks again for pointing to

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

aleksandr.urakov added a comment. I'm not sure, but have an assumption. Here is the first green build of the green-dragon-24: http://green.lab.llvm.org/green/job/lldb-cmake/12090/ It became green after three changes, one of them is your revert of my commit, while another is Zachary's "Fix the

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

davide added a comment. In https://reviews.llvm.org/D53368#1286361, @aleksandr.urakov wrote: > Thanks for catching that! Unfortunately, I have no access to MacOS, can you > provide some more info about failure, please? Unfortunately the bot logs are gone. When I originally looked at them they

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

aleksandr.urakov added a comment. Thanks for catching that! Unfortunately, I have no access to MacOS, can you provide some more info about failure, please? Repository: rLLDB LLDB https://reviews.llvm.org/D53368 ___ lldb-commits mailing list

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

davide added a comment. This broke MacOS. I'm going to revert this. To reproduce, just run `ninja check-lldb` with your patches. Please let me know if you need other informations. Repository: rLLDB LLDB https://reviews.llvm.org/D53368 ___

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

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB345957: [Symbol] Search symbols with name and type in a symbol file (authored by aleksandr.urakov, committed by ). Repository: rLLDB LLDB https://reviews.llvm.org/D53368 Files:

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

aleksandr.urakov added a comment. Thank you! https://reviews.llvm.org/D53368 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Looks good. Thanks for making the changes. https://reviews.llvm.org/D53368 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

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

aleksandr.urakov added a comment. Ping! Can you look at this, please? https://reviews.llvm.org/D53368 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

aleksandr.urakov updated this revision to Diff 171267. aleksandr.urakov added a comment. Herald added subscribers: arichardson, emaste. Herald added a reviewer: espindola. That's done! https://reviews.llvm.org/D53368 Files: include/lldb/Symbol/SymbolFile.h

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

clayborg added a comment. In https://reviews.llvm.org/D53368#1276152, @aleksandr.urakov wrote: > Yes, I'll implement it tomorrow (I'm already OOO now), thanks. But is it > really necessary to check the number of symbols added if we must to calculate > / finalize the symtab after getting it

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

aleksandr.urakov added a comment. Yes, I'll implement it tomorrow (I'm already OOO now), thanks. But is it really necessary to check the number of symbols added if we must to calculate / finalize the symtab after getting it from object file anyway? May be just always do it after creation and

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

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Very close, just down to making the SymbolVendor::GetSymtab() call symtab.CalculateSymbolSizes() and symtab.Finalize(). Comment at:

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

aleksandr.urakov updated this revision to Diff 171056. aleksandr.urakov marked 3 inline comments as done. https://reviews.llvm.org/D53368 Files: include/lldb/Symbol/SymbolFile.h include/lldb/Symbol/SymbolVendor.h source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp

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

aleksandr.urakov marked 5 inline comments as done. aleksandr.urakov added a comment. Ah, yes, sure! It's my mistake. I didn't pay attention to the fact that a symtab owns symbols. I'll update the patch, thanks! Comment at:

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

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Very close. Just a bit of cleanup now that we changed SymbolFilePDB where we don't seem to need m_public_symbols anymore. See inlined comments and let me know what you think

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

aleksandr.urakov updated this revision to Diff 170889. aleksandr.urakov added a comment. I've implemented things as Greg have said, except for a few moments: - haven't added `AddSymbols` to `SymbolVendor` because it is not used anywhere, and we can just use `SymbolFile::AddSymbols` directly

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

aleksandr.urakov added a comment. Ok, I'll reimplement this, thanks! Repository: rLLDB LLDB https://reviews.llvm.org/D53368 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

zturner added a subscriber: aleksandr.urakov. zturner added a comment. To answer your question, PE/COFF executable symbol tables are basically empty Repository: rLLDB LLDB https://reviews.llvm.org/D53368 ___ lldb-commits mailing list

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

To answer your question, PE/COFF executable symbol tables are basically empty On Mon, Oct 22, 2018 at 10:44 AM Greg Clayton via Phabricator < revi...@reviews.llvm.org> wrote: > clayborg requested changes to this revision. > clayborg added a comment. > This revision now requires changes to

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

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. All symbol tables are currently extracted from the object files via ObjectFile::GetSymtab(). Are symbols only in the PDB file? If so I would vote to add a "virtual void

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

jingham added a reviewer: clayborg. jingham added a comment. This seems like the sort of thing Greg should have a look at as well. Repository: rLLDB LLDB https://reviews.llvm.org/D53368 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

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

aleksandr.urakov created this revision. aleksandr.urakov added reviewers: zturner, asmith, labath. aleksandr.urakov added a project: LLDB. Herald added a subscriber: lldb-commits. This patch adds possibility of searching a public symbol with name and type in a symbol file, not only in a symtab.