[Lldb-commits] [PATCH] D56126: [NativePDB] Add basic support of methods recostruction in AST

[Aleksandr Urakov via lldb-commits]

Yes, right! There's x64 fastcall for everything. Ok, I'll remove it later,
thanks!

Am So., 30. Dez. 2018, 09:26 hat Zachary Turner <zturner at google.com>
geschrieben:

> Sorry, this comment was supposed to be deleted after I realized i was
> wrong.
>
> On Sat, Dec 29, 2018 at 9:12 PM Aleksandr Urakov via Phabricator <
> reviews at reviews.llvm.org> wrote:
>
>> aleksandr.urakov marked an inline comment as done.
>> aleksandr.urakov added inline comments.
>>
>>
>> ================
>> Comment at: lit/SymbolFile/NativePDB/ast-methods.cpp:28
>> +// CHECK: |-CXXRecordDecl {{.*}} struct Struct definition
>> +// CHECK: | |-CXXMethodDecl {{.*}} simple_method 'void ()
>> __attribute__((thiscall))'
>> +// CHECK: | |-CXXMethodDecl {{.*}} virtual_method 'void ()
>> __attribute__((thiscall))' virtual
>> ----------------
>> zturner wrote:
>> > I think this is wrong.  If I compile this file myself and look at the
>> debug info, the debug info says it has cdecl calling convention.
>> >
>> >
>> > ```
>> > D:\src\llvmbuild\ninja-x64>bin\llvm-pdbutil.exe dump -types foo.pdb |
>> grep -C 5 simple_method
>> >    0x106D | LF_METHODLIST [size = 20]
>> >             - Method [type = 0x106B, vftable offset = -1, attrs =
>> public compiler-generated]
>> >             - Method [type = 0x106C, vftable offset = -1, attrs =
>> public compiler-generated]
>> >    0x106E | LF_FIELDLIST [size = 152]
>> >             - LF_VFUNCTAB type = 0x1056
>> >             - LF_ONEMETHOD [name = `simple_method`]
>> >               type = 0x1059, vftable offset = -1, attrs = public
>> >             - LF_ONEMETHOD [name = `virtual_method`]
>> >               type = 0x1059, vftable offset = 0, attrs = public intro
>> virtual
>> >             - LF_ONEMETHOD [name = `static_method`]
>> >               type = 0x105A, vftable offset = -1, attrs = public static
>> >
>> > D:\src\llvmbuild\ninja-x64>bin\llvm-pdbutil.exe dump -types
>> -type-index=0x1059 foo.pdb
>> >
>> >
>> >                      Types (TPI Stream)
>> > ============================================================
>> >   Showing 1 records.
>> >    0x1059 | LF_MFUNCTION [size = 28]
>> >             return type = 0x0003 (void), # args = 0, param list = 0x1011
>> >             class type = 0x1057, this type = 0x1058, this adjust = 0
>> >             calling conv = cdecl, options = None
>> > ```
>> >
>> > Also, if I use clang-cl and pass the `-ast-dump` option, it will look
>> like this:
>> >
>> > ```
>> > | |-CXXMethodDecl 0x23869a32c38 <line:2:3, col:25> col:8 simple_method
>> 'void ()'
>> > | | `-CompoundStmt 0x23869a336f0 <col:24, col:25>
>> > ```
>> >
>> > So it doesn't have `__attribute__((thiscall))` as you do in the test.
>> Could you double check this?
>> Hm... I tested it on x86, and I had there a "thiscall" attribute. It
>> seems that on x64 it is not so. I think it's because the struct contains no
>> fields and the methods doesn't use them - they don't need a "this" pointer.
>> But I can't understand, why is there still "thiscall" on x86.
>> Unfortunately, I can check it only two weeks later, but thanks for catching
>> that!
>
> Ahh that explains it.  X64 just uses one calling convention and doesn’t
> distinguish between thiscall (the compiler will even ignore the keyword).
> I think we can just remove this from the test and it should be fine
>
>
>
>>
>>
>> ================
>> Comment at: source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp:669-672
>> +    MemberFunctionRecord mfr;
>> +    llvm::cantFail(
>> +        TypeDeserializer::deserializeAs<MemberFunctionRecord>(cvt, mfr));
>> +    return CreateFunctionType(mfr.ArgumentList, mfr.ReturnType,
>> mfr.CallConv);
>> ----------------
>> zturner wrote:
>> > I think there is a problem here.
>> I can't understand, why?..
>>
>>
>> Repository:
>>   rLLDB LLDB
>>
>> CHANGES SINCE LAST ACTION
>>   https://reviews.llvm.org/D56126/new/
>>
>> https://reviews.llvm.org/D56126
>>
>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20181230/519616ff/attachment.html>