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 <
revi...@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
>
>
>
>
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
