From 42f7166f6b7578ae4fc5d65f3a7597291b364075 Mon Sep 17 00:00:00 2001
From: Giuliano Belinassi <gbelinassi@suse.de>
Date: Fri, 16 Aug 2024 17:17:27 -0300
Subject: [PATCH] Fix CompleteDefinition of struct being dragged into output
 because of union field

On the following example:
```
struct ll {
  union {
    unsigned long key;
    void *key_ptr;
  };
  struct ll *next;
};
```
Even if we don't need the full definition of `ll` it is dragged into
the output because when analyzing this `union` field it tries to go
up and set `struct ll` as having its body necessary once there was
no way for it to know that it went there as a consequence of adding
`struct ll` to the closure, rather than it being there because of
a foward union declaration inside a struct.  This commit fixes
this by pushing the Decls into a stack as a way to know which
declarations we are currently analyzing.

Signed-off-by: Giuliano Belinassi <gbelinassi@suse.de>
---
 libcextract/Closure.cpp           | 15 +++++++++++++--
 libcextract/Closure.hh            | 15 ++++++++++++++-
 testsuite/small/record-nested-5.c | 20 ++++++++++++++++++++
 3 files changed, 47 insertions(+), 3 deletions(-)
 create mode 100644 testsuite/small/record-nested-5.c

diff --git a/libcextract/Closure.cpp b/libcextract/Closure.cpp
index dfc428f..f0b3e6d 100644
--- a/libcextract/Closure.cpp
+++ b/libcextract/Closure.cpp
@@ -87,7 +87,10 @@ bool DeclClosureVisitor::TraverseDecl(Decl *decl)
 {
   DO_NOT_RUN_IF_ALREADY_ANALYZED(decl);
   Mark_As_Analyzed(decl);
-  return RecursiveASTVisitor::TraverseDecl(decl);
+  Stack.push_back(decl);
+  bool ret = RecursiveASTVisitor::TraverseDecl(decl);
+  Stack.pop_back();
+  return ret;
 }
 
 bool DeclClosureVisitor::VisitFunctionDecl(FunctionDecl *decl)
@@ -506,7 +509,15 @@ bool DeclClosureVisitor::ParentRecordDeclHelper(TagDecl *decl)
   /* In case this references a struct/union defined inside a struct (nested
      struct), then we also need to analyze the parent struct.  */
   RecordDecl *parent = dyn_cast<RecordDecl>(decl->getLexicalDeclContext());
-  if (parent) {
+  Decl *analysis_parent = nullptr;
+
+  /* Check if we are not coming from an analysis from the RecordDecl, i.e. from
+     a stack perspective the Decl that is at the bottom of the Decl.  */
+  if (Stack.size() >= 2) {
+    analysis_parent = Stack[Stack.size() - 2];
+  }
+
+  if (parent && analysis_parent != parent) {
     /* If the parent struct is flagged as not needing a complete definition
        then we need to set it to true, else the nested struct won't be
        output as of only a partial definition of the parent struct is
diff --git a/libcextract/Closure.hh b/libcextract/Closure.hh
index 32e8ea6..91100aa 100644
--- a/libcextract/Closure.hh
+++ b/libcextract/Closure.hh
@@ -140,7 +140,10 @@ class DeclClosureVisitor : public RecursiveASTVisitor<DeclClosureVisitor>
   public:
   DeclClosureVisitor(ASTUnit *ast)
     : RecursiveASTVisitor(),
-      AST(ast)
+      AST(ast),
+      Closure(),
+      AnalyzedDecls(),
+      Stack()
   {
   }
 
@@ -273,4 +276,14 @@ class DeclClosureVisitor : public RecursiveASTVisitor<DeclClosureVisitor>
 
   /** The set of all analyzed Decls.  */
   std::unordered_set<Decl *> AnalyzedDecls;
+
+  /** Stack of Decls.  Implement using a vector because we may need to access
+      the second element on the top, and we also need its continuity.  */
+  llvm::SmallVector<Decl *, 128> Stack;
+
+  /** Return what is on top of our stack.  */
+  inline Decl *Stack_Top(void)
+  {
+    return Stack[Stack.size() - 1];
+  }
 };
diff --git a/testsuite/small/record-nested-5.c b/testsuite/small/record-nested-5.c
new file mode 100644
index 0000000..a0f248c
--- /dev/null
+++ b/testsuite/small/record-nested-5.c
@@ -0,0 +1,20 @@
+/* { dg-options "-DCE_EXTRACT_FUNCTIONS=f -DCE_NO_EXTERNALIZATION" }*/
+
+struct ll;
+
+int get(struct ll *);
+
+struct ll {
+  union {
+    unsigned long key;
+    void *key_ptr;
+  };
+  struct ll *next;
+};
+
+int f(struct ll *l)
+{
+  return get(l);
+}
+
+/* { dg-final { scan-tree-dump-not "struct ll *{" } } */