Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

chying added a comment. Yes, it is fixed now. Thank you! http://reviews.llvm.org/D17363 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

clayborg added a subscriber: clayborg. clayborg added a comment. This is already fixed. Update your sources and let us know if things are working. http://reviews.llvm.org/D17363 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

This is already fixed. Update your sources and let us know if things are working. > On Mar 2, 2016, at 5:10 PM, Ying Chen wrote: > > chying added a subscriber: chying. > chying added a comment. > > Seems this patch breaks OSX build. I guess the new files were not added to

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

Fixed with r262543 Jim > On Mar 2, 2016, at 5:10 PM, Ying Chen via lldb-commits > wrote: > > chying added a subscriber: chying. > chying added a comment. > > Seems this patch breaks OSX build. I guess the new files were not added to > xcode project file. > Could

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

jingham added a subscriber: jingham. jingham added a comment. Fixed with r262543 Jim http://reviews.llvm.org/D17363 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

chying added a subscriber: chying. chying added a comment. Seems this patch breaks OSX build. I guess the new files were not added to xcode project file. Could you please take a look? http://lab.llvm.org:8011/builders/lldb-x86_64-darwin-13.4/builds/8892/steps/ninja%20build%20local/logs/stdio

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. I guess you already tackled the termination entry! That is exactly what I wanted to see... Sounds like line tables are done! http://reviews.llvm.org/D17363

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. As long as you don't mind your last line entry potentially being really large because it isn't terminated? (lldb) target create

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

zturner added a comment. I looked into this a little bit, it's going to be a couple days of work for me to get termination entries set correctly. Can I go in as-is? I know you already Accepted the revision, just want to double check. I need to go make some changes to `llvm-pdbdump` on the

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

zturner added a comment. In http://reviews.llvm.org/D17363#366522, @clayborg wrote: > You are right. I am slowly seeing that PDB was designed well to be consumed > as a user would want to, not compressed to the point of obfuscation and very > difficult to extract data from like DWARF. > > So

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. You are right. I am slowly seeing that PDB was designed well to be consumed as a user would want to, not compressed to the point of obfuscation and very difficult to extract data from

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

zturner added inline comments. Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:291-292 @@ +290,4 @@ +// , either directly or indirectly. +auto compilands = +m_session_up->findCompilandsForSourceFile(file_spec.GetPath(),

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

You could add a test to that grabs functions bounds, once you implement functions, and then lookup each address inside of a function and make sure each one has a file and line. This might fall down for complex examples, but we also ran into the Swift compiler saying a function went from

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

Regarding the refactoring of ResolveSymbolContext to a lower level. It seems like a worthwhile refactor but probably one that should be done as an independent CL. It seems like it has potential to open up a bit of a rats nest so to speak, and if something ends up breaking as a result, we can

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

You don't add plug-in specify unique_ptr and shared_ptr definitions. You would put these in SymbolFilePDB.h only. > On Mar 1, 2016, at 10:51 AM, Jim Ingham wrote: > > There's an lldb-private-forward.h that you can use for this purpose if you > wish. > > Jim > >> On Mar

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

On Tue, Mar 1, 2016 at 10:10 AM Greg Clayton wrote: > clayborg requested changes to this revision. > clayborg added a comment. > This revision now requires changes to proceed. > > One general comment is the use of "auto". Although it makes the code > shorter, it does make it

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. One general comment is the use of "auto". Although it makes the code shorter, it does make it quite a bit less readable. I will leave the decision to you since this is your

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

zturner updated this revision to Diff 49442. zturner added a comment. Note this should compile now on every platform. Look over the unit tests to see my assumptions about how the API should work. Hopefully this makes reviewing things easier (it definitely made getting things to work easier)

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

I think I see the problem. Somewhere I was calling SymbolVendor::FindPlugin(module) instead of module->GetSymbolVendor() On Mon, Feb 29, 2016 at 2:48 PM Greg Clayton wrote: > You shouldn't have to do any of this. There should be a 1 to 1 mapping > between a Module and a

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

You shouldn't have to do any of this. There should be a 1 to 1 mapping between a Module and a SymbolVendor which has a single SymbolFile. My guess is that you should just return: kAllAbilities for SymbolFilePDB::CalculateAbilities (). Try that and let me know how this goes. These abilities

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

I guess I could make the CreateInstance() method of SymbolFilePDB do some "smarts" to cache instances of SymbolFilePDB already created, and either return those or create a new one depending on whether we've loaded an instance for the requested executable or not. On Mon, Feb 29, 2016 at 1:26 PM

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

I'm running into what I think is the final issue. My SymbolFilePDB plugin creates created many many times. Specifically, every single call to SymbolVendor::FindPlugin(module) will cause a brand new instance of SymbolFilePDB to get created. This effectively kills any caching I've got going on,

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

Thanks! I think I've got a handle on it. I'll upload another patch this week hopefully. On Mon, Feb 29, 2016 at 10:30 AM Greg Clayton wrote: > > > On Feb 26, 2016, at 3:33 PM, Zachary Turner wrote: > > > > Ok, so back to check_inlines. I realized

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

> On Feb 26, 2016, at 3:33 PM, Zachary Turner wrote: > > Ok, so back to check_inlines. I realized after I hit send that the > explanation I had written out is exactly what I thought I had to do for > check_inlines == true. > > I guess a concrete example would make it

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

Ok, so back to check_inlines. I realized after I hit send that the explanation I had written out is exactly what I thought I had to do for check_inlines == true. I guess a concrete example would make it clearer. If I have this code: // foo.cpp #include "foo.h" int main(int argc, char **argv)

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

> On Feb 26, 2016, at 3:22 PM, Zachary Turner wrote: > > > > On Fri, Feb 26, 2016 at 3:16 PM Greg Clayton wrote: > > > On Feb 26, 2016, at 2:49 PM, Zachary Turner wrote: > > > I'm coming back around to this now. What happens if

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

On Fri, Feb 26, 2016 at 3:16 PM Greg Clayton wrote: > > > On Feb 26, 2016, at 2:49 PM, Zachary Turner wrote: > > > I'm coming back around to this now. What happens if check_inlines is > False, but the FileSpec is a header file like . You said "If >

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

> On Feb 26, 2016, at 2:49 PM, Zachary Turner wrote: > > > > On Thu, Feb 18, 2016 at 6:16 PM Greg Clayton wrote: > > > Just to make sure I understand, is it safe to say that: > > > > If check_inlines is false, sc_list should return with exactly 1 > >

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

On Thu, Feb 18, 2016 at 6:16 PM Greg Clayton wrote: > > > Just to make sure I understand, is it safe to say that: > > > > If check_inlines is false, sc_list should return with exactly 1 > SymbolContext with m_comp_unit set to the main source file? > > You would get one

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. So your LineTable::Entry objects that you create are still invalid since you didn't implement the support files. All comments became unaligned but you did comment that: > In

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

zturner added inline comments. Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:150 @@ +149,3 @@ +return TranslateLanguage(details->getLanguage()); +} + Ahh i see the problem. The problem is not the value Im' specifying for the id of the compile

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

zturner updated this revision to Diff 48544. zturner added a comment. This should address all (I think) issues except for the one about `check_inlines`, which I'll do in a followup. Can you respond to my comment in a previous update about the uniqueness / index-ness of the comp_unit id?

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

zturner marked 6 inline comments as done. Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:82-83 @@ +81,4 @@ +auto error = llvm::loadDataForEXE(llvm::PDB_ReaderType::DIA, llvm::StringRef(exePath), m_session); +if (error != llvm::PDB_ErrorCode::Success)

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

clayborg added a comment. General things to know about SymbolFiles: - We use SymbolContext objects to refer to a specific symbol context: module, compile unit, function, deepest block, line entry and symbol. - A load address should produce 1 symbol context. Never more. - A file address can

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

clayborg added a comment. There are problems with the source file IDs you are using as they currently must be indexes into the compile unit's support files. See inline comments for more details. Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:146 @@ +145,3 @@ + +

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

zturner added inline comments. Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:98-100 @@ +97,5 @@ +{ +auto global = m_session->getGlobalScope(); +auto compilands = global->findAllChildren(); +return compilands->getChildCount(); +} clayborg

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. So the main issue is to fix the: uint32_t SymbolFilePDB::ResolveSymbolContext(const lldb_private::FileSpec _spec, uint32_t line, bool check_inlines,

Re: [Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

zturner added a comment. Added a few notes to clarify things that might not be obvious upon a first look at the patch. Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:81 @@ +80,3 @@ +std::string exePath = m_obj_file->GetFileSpec().GetPath(); +auto

[Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.

zturner created this revision. zturner added a reviewer: clayborg. zturner added a subscriber: lldb-commits. This is a first attempt at getting `SymbolFilePDB` working with line table support. A few notes: * This won't compile until a corresponding patch goes in on the LLVM side. The patch