[DTrace-devel] [PATCH] Set storage for variables when type is assigned

[eugene.loh at oracle.com]

From: Eugene Loh <eugene.loh at oracle.com>

A commit ("Change storage for global variables") changed storage of
global variables, which no longer need to have 8-byte size.  The parser
now calls dt_ident_set_storage() for each variable, in dt_node_decl() if
the variable is declared explicitly or in dt_xcook_ident() if declared
implicitly (based on its first assignment).  The variable type is not
known in dt_xcook_ident(), however, and so correct storage cannot be set.

Consolidate setting storage to dt_ident_type_assign().  Add a test.

Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
---
 libdtrace/dt_ident.c                     |  5 ++
 libdtrace/dt_parser.c                    |  7 --
 test/unittest/struct/tst.implicit_decl.d | 91 ++++++++++++++++++++++++
 test/unittest/struct/tst.implicit_decl.r |  2 +
 4 files changed, 98 insertions(+), 7 deletions(-)
 create mode 100644 test/unittest/struct/tst.implicit_decl.d
 create mode 100644 test/unittest/struct/tst.implicit_decl.r

diff --git a/libdtrace/dt_ident.c b/libdtrace/dt_ident.c
index 48a97954..8a2f7f3c 100644
--- a/libdtrace/dt_ident.c
+++ b/libdtrace/dt_ident.c
@@ -1024,6 +1024,11 @@ dt_ident_type_assign(dt_ident_t *idp, ctf_file_t *fp, ctf_id_t type)
 {
 	idp->di_ctfp = fp;
 	idp->di_type = type;
+
+	/* set the offset and size */
+	assert(idp->di_offset == -1);
+	if (idp->di_hash)
+		dt_ident_set_storage(idp, 8, ctf_type_size(fp, type));
 }
 
 dt_ident_t *
diff --git a/libdtrace/dt_parser.c b/libdtrace/dt_parser.c
index 948ef2d8..482d0a31 100644
--- a/libdtrace/dt_parser.c
+++ b/libdtrace/dt_parser.c
@@ -1661,9 +1661,6 @@ dt_node_decl(void)
 			if (idp == NULL)
 				longjmp(yypcb->pcb_jmpbuf, EDT_NOMEM);
 
-			dt_ident_set_storage(idp, 8,
-					     ctf_type_size(dtt.dtt_ctfp, type));
-
 			dt_ident_type_assign(idp, dtt.dtt_ctfp, dtt.dtt_type);
 
 			/*
@@ -2754,10 +2751,6 @@ dt_xcook_ident(dt_node_t *dnp, dt_idhash_t *dhp, uint_t idkind, int create)
 		if (idp == NULL)
 			longjmp(yypcb->pcb_jmpbuf, EDT_NOMEM);
 
-		/* aggregation storage size is set later, in dt_cg.c */
-		if (idkind != DT_IDENT_AGG)
-			dt_ident_set_storage(idp, 8, 8);
-
 		/*
 		 * Arrays and aggregations are not cooked individually. They
 		 * have dynamic types and must be referenced using operator [].
diff --git a/test/unittest/struct/tst.implicit_decl.d b/test/unittest/struct/tst.implicit_decl.d
new file mode 100644
index 00000000..2a24c840
--- /dev/null
+++ b/test/unittest/struct/tst.implicit_decl.d
@@ -0,0 +1,91 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
+ * Licensed under the Universal Permissive License v 1.0 as shown at
+ * http://oss.oracle.com/licenses/upl.
+ */
+
+/*
+ * ASSERTION: Verify assignments when structs are implicitly declared.
+ *
+ * SECTION: Structs and Unions/Structs
+ */
+
+#pragma D option quiet
+
+/*
+ * Declare some structs, including a struct of structs.
+ */
+
+struct coords {
+	int x, y;
+};
+struct particle {
+	struct coords position;
+	struct coords velocity;
+};
+
+/*
+ * Declare some global (uppercase) and clause-local (lowercase) variables.
+ * In each trio:
+ *   - The first two variables are ignored;
+ *       they are declared just so we can test nonzero offsets.
+ *   - The third variable is initialized.
+ */
+     struct coords   P, Q, R;
+this struct coords   p, q, r;
+     struct particle A, B, C;
+this struct particle a, b, c;
+
+BEGIN {
+	/* Initialize the third variables. */
+	      R.x          = 11;
+	      R.y          = 12;
+	      C.position.x = 13;
+	      C.position.y = 14;
+	      C.velocity.x = 15;
+	      C.velocity.y = 16;
+	this->r.x          = 21;
+	this->r.y          = 22;
+	this->c.position.x = 23;
+	this->c.position.y = 24;
+	this->c.velocity.x = 25;
+	this->c.velocity.y = 26;
+
+	/*
+	 * structs S/T/D/E/s/t/d/e are declared implicitly
+	 * via their first assignments
+	 */
+
+	      S          =       R;
+	this->s          = this->r;
+	      T          = this->s;
+	this->t          =       S;
+
+	      D          =       C;
+	this->d          = this->c;
+	      E          = this->d;
+	this->e          =       D;
+
+	      E.velocity = this->t;
+	this->e.velocity =       T;
+
+	/* print results */
+
+	printf("%d %d %d %d %d %d %d %d\n",
+	          E.position.x,
+	          E.position.y,
+	          E.velocity.x,
+	          E.velocity.y,
+	    this->e.position.x,
+	    this->e.position.y,
+	    this->e.velocity.x,
+	    this->e.velocity.y);
+
+	exit(0);
+}
+
+ERROR
+{
+	exit(1);
+}
diff --git a/test/unittest/struct/tst.implicit_decl.r b/test/unittest/struct/tst.implicit_decl.r
new file mode 100644
index 00000000..01909934
--- /dev/null
+++ b/test/unittest/struct/tst.implicit_decl.r
@@ -0,0 +1,2 @@
+23 24 11 12 13 14 21 22
+
-- 
2.18.4