[PATCH] D59254: [RFC] Implementation of Clang randstruct

Tue Mar 12 08:10:15 PDT 2019

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