[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

aleksandr.urakov added a comment. It would be great, thank you! I'll also do it on Monday, if it's already possible. https://reviews.llvm.org/D49410 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/lis

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

zturner added a comment. In the meantime, perhaps you could request commit access :) https://reviews.llvm.org/D49410 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

In the meantime, perhaps you could request commit access :) On Thu, Jul 26, 2018 at 9:30 PM Aleksandr Urakov via Phabricator < revi...@reviews.llvm.org> wrote: > aleksandr.urakov added a comment. > > Thanks! > > I have created the corresponding patch for Clang ( > https://reviews.llvm.org/D49871,

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

aleksandr.urakov added a comment. Thanks! I have created the corresponding patch for Clang (https://reviews.llvm.org/D49871, @rnk have accepted it. Can you commit this, please? I have no commit access). But current patch intersects with https://reviews.llvm.org/D49368, and I'm preparing the c

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

zturner accepted this revision. zturner added a comment. This revision is now accepted and ready to land. I am not sure what are the drawbacks of such a solution are. I guess the best way to find out is to try it and see if you observe any strange behavior :) I'm ok with that, for now. https

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

aleksandr.urakov added a subscriber: zturner. aleksandr.urakov added a comment. Yes, I have understood that, thank you. It may be not trivial in a case of multiple nested unnamed unions/structs, but it's solvable for clang-emitted PDBs. But I'm not sure about cl-emitted PDBs too. Can we leave t

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

zturner added a comment. Wouldn’t the location of the unnamed struct be the location of its first member? We already print the offsets of the individual members, so that part is solvable (although that code is horrendously complex). The second part is figuring out how to correlate the member back

Re: [Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

Wouldn’t the location of the unnamed struct be the location of its first member? We already print the offsets of the individual members, so that part is solvable (although that code is horrendously complex). The second part is figuring out how to correlate the member back to the unnamed struct it c

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

aleksandr.urakov added a comment. In https://reviews.llvm.org/D49410#1174787, @zturner wrote: > When you have a DIA interface for struct S, can you just call > findChildren()? Will that enumerate tge unnamed struct? I have checked this again with `cl` and `clang-cl` (but I have enumerated all

Re: [Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

When you have a DIA interface for struct S, can you just call findChildren()? Will that enumerate tge unnamed struct? The fact that pdbutil doesn’t is only an indication of how the printing code behaves, you shouldn’t interpret anything about what information is available from it On Wed, Jul 25, 20

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

zturner added a subscriber: asmith. zturner added a comment. When you have a DIA interface for struct S, can you just call findChildren()? Will that enumerate tge unnamed struct? The fact that pdbutil doesn’t is only an indication of how the printing code behaves, you shouldn’t interpret anything

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

aleksandr.urakov added inline comments. Comment at: lit/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp:37 + }; + union { // Test unnamed union. MSVC treats it as `int a; float b;` +int a; aleksandr.urakov wrote: > aleksandr.urakov wrote: > > Hui wrote: > > > al

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

aleksandr.urakov added inline comments. Comment at: lit/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp:37 + }; + union { // Test unnamed union. MSVC treats it as `int a; float b;` +int a; aleksandr.urakov wrote: > Hui wrote: > > aleksandr.urakov wrote: > > > He

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

zturner added a comment. I think the problem is in lld, we don’t emit S_UDT symbols because it caused weird problems in WinDbg. There’s a comment explaining it in PDB.cpp. But after some recent fixes this may just work. So we should probably try again to emit the S_UDT in lld, I think that shou

Re: [Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

I think the problem is in lld, we don’t emit S_UDT symbols because it caused weird problems in WinDbg. There’s a comment explaining it in PDB.cpp. But after some recent fixes this may just work. So we should probably try again to emit the S_UDT in lld, I think that should fix it On Fri, Jul 20,

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

aleksandr.urakov added inline comments. Comment at: lit/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp:37 + }; + union { // Test unnamed union. MSVC treats it as `int a; float b;` +int a; Hui wrote: > aleksandr.urakov wrote: > > Here is a problem. `MicrosoftRec

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

Hui added inline comments. Comment at: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:696 + base_ast_type.GetOpaqueQualType(), access, + pdb_base_class->isVirtualBaseClass(), /*base_of_class*/ true); + base_classes.push_back(base_spec); ale

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

Hui added inline comments. Comment at: lit/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp:37 + }; + union { // Test unnamed union. MSVC treats it as `int a; float b;` +int a; aleksandr.urakov wrote: > Here is a problem. `MicrosoftRecordLayoutBuilder` asserts ev

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

aleksandr.urakov added inline comments. Comment at: lit/SymbolFile/PDB/class-layout.test:3 +RUN: clang-cl -m32 /Z7 /c /GS- %S/Inputs/ClassLayoutTest.cpp /o %T/ClassLayoutTest.cpp.obj +RUN: link %T/ClassLayoutTest.cpp.obj /DEBUG /nodefaultlib /ENTRY:main /OUT:%T/ClassLayoutTest.

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

zturner added inline comments. Comment at: lit/SymbolFile/PDB/class-layout.test:3 +RUN: clang-cl -m32 /Z7 /c /GS- %S/Inputs/ClassLayoutTest.cpp /o %T/ClassLayoutTest.cpp.obj +RUN: link %T/ClassLayoutTest.cpp.obj /DEBUG /nodefaultlib /ENTRY:main /OUT:%T/ClassLayoutTest.cpp.exe +

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

aleksandr.urakov added inline comments. Comment at: lit/SymbolFile/PDB/class-layout.test:3 +RUN: clang-cl -m32 /Z7 /c /GS- %S/Inputs/ClassLayoutTest.cpp /o %T/ClassLayoutTest.cpp.obj +RUN: link %T/ClassLayoutTest.cpp.obj /DEBUG /nodefaultlib /ENTRY:main /OUT:%T/ClassLayoutTest.

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

aleksandr.urakov added inline comments. Comment at: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:273 +udt->getClassParent()) { + access = GetDefaultAccessibilityForUdtKind(udt_kind); +} Yes, we can occur here, as I have mentioned at the line 18

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

aleksandr.urakov added inline comments. Comment at: lit/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp:37 + }; + union { // Test unnamed union. MSVC treats it as `int a; float b;` +int a; Here is a problem. `MicrosoftRecordLayoutBuilder` asserts every field or

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

aleksandr.urakov added inline comments. Comment at: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:186 +return lldb::eAccessProtected; + return lldb::eAccessNone; +} zturner wrote: > I wonder if this would be better as an `llvm_unreachable`. Being able to

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

aleksandr.urakov added inline comments. Comment at: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:696 + base_ast_type.GetOpaqueQualType(), access, + pdb_base_class->isVirtualBaseClass(), /*base_of_class*/ true); + base_classes.push_back(base_spec); ---

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

Hui added inline comments. Comment at: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:647-648 +auto member_ast_type = type_sp->GetLayoutCompilerType(); +if (!member_ast_type.IsCompleteType()) + member_ast_type.GetCompleteType(); +// CXX class type

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

Hui added inline comments. Comment at: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:271-272 +AccessType access = TranslateMemberAccess(udt->getAccess()); +if (access == lldb::eAccessNone && udt->isNested() && +udt->getClassParent()) { + access = GetDefaultA

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

Hui added inline comments. Comment at: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:246 +return ty ? ty->shared_from_this() : nullptr; + } break; case PDB_SymType::UDT: { zturner wrote: > This looks unusual. Did you clang-format it? Just tried clang-fo

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

Hui added inline comments. Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:588 +if (result->GetID() == type_uid) { + pdb->CompleteRecordTypeForPDBSymbol(*pdb_type, result); +} aleksandr.urakov wrote: > What are the advantages and disadvan

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

Hui added inline comments. Comment at: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:696 + base_ast_type.GetOpaqueQualType(), access, + pdb_base_class->isVirtualBaseClass(), /*base_of_class*/ true); + base_classes.push_back(base_spec); ale

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

Hui added inline comments. Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:587 +// only do this once. +if (result->GetID() == type_uid) { + pdb->CompleteRecordTypeForPDBSymbol(*pdb_type, result); aleksandr.urakov wrote: > I don't fully un

Re: [Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

Not at all. > On Jul 18, 2018, at 8:27 AM, Aleksandr Urakov via Phabricator > wrote: > > aleksandr.urakov added a comment. > > Don't you mind if I try to combine this with https://reviews.llvm.org/D49368? > To avoid doing the work twice. > > > https://reviews.llvm.org/D49410 > > > _

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

aleksandr.urakov added a comment. Don't you mind if I try to combine this with https://reviews.llvm.org/D49368? To avoid doing the work twice. https://reviews.llvm.org/D49410 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.ll

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

aleksandr.urakov added inline comments. Comment at: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:291-295 +if (udt->isConstType()) + clang_type = clang_type.AddConstModifier(); + +if (udt->isVolatileType()) + clang_type = clang_type.AddVolatileModifier(); -

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

zturner added inline comments. Comment at: lit/SymbolFile/PDB/class-layout.test:3 +RUN: clang-cl -m32 /Z7 /c /GS- %S/Inputs/ClassLayoutTest.cpp /o %T/ClassLayoutTest.cpp.obj +RUN: link %T/ClassLayoutTest.cpp.obj /DEBUG /nodefaultlib /ENTRY:main /OUT:%T/ClassLayoutTest.cpp.exe +

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

asmith updated this revision to Diff 155904. asmith added a comment. Adding missing inputs https://reviews.llvm.org/D49410 Files: lit/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp lit/SymbolFile/PDB/Inputs/PointerTypeTest.cpp lit/SymbolFile/PDB/class-layout.test lit/SymbolFile/PDB/pointers.

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

aleksandr.urakov added a comment. I think that this patch is more solid and covers more cases, than https://reviews.llvm.org/D49368, especially in `CreateLLDBTypeFromPDBType` part. But https://reviews.llvm.org/D49368 has also following: - Filling of a layout info. It allows use of packed types,

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

aleksandr.urakov added a comment. It seems that you forgot to add the testing `*.cpp` files from the `Input` directory to the patch... Repository: rL LLVM https://reviews.llvm.org/D49410 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

aleksandr.urakov added a comment. Ah, we need some way to synchronize our work :) I think in this case it will be better to appreciate the pros and cons of our patches, choose the better variant and improve that with the features available in another variant. I will investigate your patch to un

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

labath added a comment. I have no opinion on the implementation, but I like that you wrote a non-execution test for this, as this will enable us one day to run it on non-windows hosts too. Comment at: lit/SymbolFile/PDB/class-layout.test:50-53 +CHECK-DAG:_List *array[90];

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

asmith created this revision. asmith added reviewers: lldb-commits, aleksandr.urakov, rnk, zturner. Herald added a subscriber: llvm-commits. This is an alternative to https://reviews.llvm.org/D49368 Repository: rL LLVM https://reviews.llvm.org/D49410 Files: lit/SymbolFile/PDB/class-layout.