[cfe-dev] [lldb-dev] stable layout bug for imported record decls.

[Alexey Sidorin via cfe-dev]

(+ Gabor and Gabor)

Hi Lang,

We faced a very similar issue with record fields where import order can 
change the order of imported FieldDecls resulting in broken 
ASTRecordLayout. The patch for this issue is on review: 
https://reviews.llvm.org/D44100. It just reorders the fields after 
structure import is finished. CSA guys also reported the same problem 
with FriendDecls in the same review.The order of methods was not a 
problem for us but your report adds a new item to support. It looks like 
_all_ decls inside RecordDecl have to be reordered. I'll try to 
resurrect the patch this weekend (it is a bit outdated due to my 
workload, sorry) and add you as a reviewer so you can check if it solves 
the problem or not.

09.08.2018 20:46, Lang Hames via lldb-dev пишет:
> Hi clang-dev, lldb-dev,
>
> It looks like my clang commit r305850, which modified ASTImporter to 
> import method override tables from an external context, introduced a 
> new bug which manifests as incorrect vtable layouts for LLDB 
> expression code.
>
> The bug itself is fairly straightforward. In r305850 I introduced the 
> following method, which is called from ASTNodeImporter::VisitFunctionDecl:
>
> void ASTNodeImporter::ImportOverrides(CXXMethodDecl *ToMethod,
>             CXXMethodDecl *FromMethod) {
>   for (auto *FromOverriddenMethod : FromMethod->overridden_methods())
> ToMethod->addOverriddenMethod(
> cast<CXXMethodDecl>(Importer.Import(const_cast<CXXMethodDecl*>(
>                   FromOverriddenMethod))));
> }
>
> This will produce the correct override table, but can also cause 
> methods in the Base class to be visited in the wrong order. Consider:
>
> class Base {
> public:
>   virtual void bar() {}
>   virtual void foo() {}
> };
>
> class Derived : public Base {
> public:
>   void foo() override {}
> };
>
> If Derived is imported into a new context before Base, then the 
> importer will visit Derived::foo, and (via ImportOverrides) 
> immediately import Base::foo, but this happens before Base::bar is 
> imported. As a consequence, the decl order on the imported Base class 
> will end up being [ foo, bar ], instead of [ bar, foo ]. In LLDB 
> expression evaluation this manifests as an incorrect vtable layout for 
> Base, with foo occupying the first slot.
>
> I am looking for suggestions on the right way to fix this. A brute 
> force solution might be to always have 
> ASTNodeImporter::VisitRecordDecl visit all base classes, then all 
> virtual methods, which would ensure they are visited in the original 
> decl order. However I do not know whether this covers all paths by 
> which a CXXRecordDecl might be imported, nor whether the performance 
> of this solution would be acceptable (it seems like it would preclude 
> a lot of laziness).
>
> Alternatively we might be able to associate an index with each 
> imported decl and sort on that when we complete the type, but that 
> would leave imported decls in the wrong order until the type was 
> complete, and since I do not know all the use cases for the importer 
> I'm concerned people may rely on the decl order before type is complete.
>
> Any insight from ASTImporter experts would be greatly appreciated. :)
>
> Cheers,
> Lang.
>
>
> _______________________________________________
> lldb-dev mailing list
> lldb-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20180810/9c658450/attachment.html>