[DTrace-devel] [PATCH v2] 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 | 85 ++++++++++++++++++++++++
 test/unittest/struct/tst.implicit_decl.r |  2 +
 4 files changed, 92 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..5b135837
--- /dev/null
+++ b/test/unittest/struct/tst.implicit_decl.d
@@ -0,0 +1,85 @@
+/*
+ * 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.
+ * The "dummy" variables are declared just so we can test nonzero offsets.
+ */
+     struct coords   C_DUMMY, C_DECLD;
+this struct coords   c_dummy, c_decld;
+     struct particle P_DUMMY, P_DECLD;
+this struct particle p_dummy, p_decld;
+
+BEGIN {
+	/* Initialize the declared variables. */
+	      C_DECLD.x          = 11;
+	      C_DECLD.y          = 12;
+	      P_DECLD.position.x = 13;
+	      P_DECLD.position.y = 14;
+	      P_DECLD.velocity.x = 15;
+	      P_DECLD.velocity.y = 16;
+	this->c_decld.x          = 21;
+	this->c_decld.y          = 22;
+	this->p_decld.position.x = 23;
+	this->p_decld.position.y = 24;
+	this->p_decld.velocity.x = 25;
+	this->p_decld.velocity.y = 26;
+
+	/* These structs are declared implicitly via their first assignments. */
+
+	      C_IMPL1          =       C_DECLD;
+	this->c_impl1          = this->c_decld;
+	      C_IMPL2          = this->c_impl1;
+	this->c_impl2          =       C_IMPL1;
+
+	      P_IMPL1          =       P_DECLD;
+	this->p_impl1          = this->p_decld;
+	      P_IMPL2          = this->p_impl1;
+	this->p_impl2          =       P_IMPL1;
+
+	      P_IMPL2.velocity = this->c_impl2;
+	this->p_impl2.velocity =       C_IMPL2;
+
+	/* print results */
+
+	printf("%d %d %d %d %d %d %d %d\n",
+	          P_IMPL2.position.x,
+	          P_IMPL2.position.y,
+	          P_IMPL2.velocity.x,
+	          P_IMPL2.velocity.y,
+	    this->p_impl2.position.x,
+	    this->p_impl2.position.y,
+	    this->p_impl2.velocity.x,
+	    this->p_impl2.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