connorkuehl added inline comments.

================
Comment at: clang/lib/AST/DeclBase.cpp:1262
+  // The last one in the chain should have a null next!
+  PrevDecl->NextInContextAndBits.setPointer(nullptr);
+
----------------
Apologies that this is included. I've left a comment on 
`clang/lib/AST/RecordFieldReorganizer.cpp` describing our issues with this 
code. I also mention experimenting with fixing it in that comment as well, and 
in those experiments we've been able to remove this change from BuildDeclChain. 
We're just waiting for further advice before we change the code too rapidly.


================
Comment at: clang/lib/AST/RecordFieldReorganizer.cpp:58
+  Decl *First, *Last;
+  std::tie(First, Last) = DeclContext::BuildDeclChain(
+      NewFieldOrder, D->hasLoadedFieldsFromExternalStorage());
----------------
This part of the code where we rebuild the new DeclChain with the new order of 
fields is resulting in a Clang compiler that segfaults when building the Linux 
kernel.

Any advice with how we can safely manage the DeclContext field chain is greatly 
appreciated here.

In our experimentation (which isn't present in this diff here) we thought our 
best bet to was to save the LastDecl's next pointer before BuildDeclChain is 
called and then we set the NEW LastDecl's next pointer to it in an effort to 
preserve whatever context chaining is happening.

Truthfully, we don't fully understand the machinery of the DeclContext or why 
our assumptions about this linked list of fields are incorrect.

More troubleshooting information regarding this particular instance is on our 
github issue: https://github.com/clang-randstruct/llvm-project/issues/42







Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59254/new/

https://reviews.llvm.org/D59254



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to