Avoid copying undefined data in _readA_Const().

tglsfdc · tglsfdc · commit e060cd59fabd · 2023-03-19T15:36:16.000-04:00
nodeRead() will have created a Node struct that's only allocated big enough for the specific node type, so copying sizeof(union ValUnion) can be copying too much. This provokes valgrind complaints, and with very bad luck could perhaps result in SIGSEGV. While at it, tidy up _equalA_Const to avoid duplicate checks of isnull. Per report from Alexander Lakhin. This code is new as of a6bc330, so no need to back-patch. Discussion: https://postgr.es/m/4995256b-cc65-170e-0b22-60ad2cd535f1@gmail.com
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
@@ -133,13 +133,11 @@ _equalExtensibleNode(const ExtensibleNode *a, const ExtensibleNode *b)
 static bool
 _equalA_Const(const A_Const *a, const A_Const *b)
 {
-	/*
-	 * Hack for in-line val field.  Also val is not valid is isnull is true.
-	 */
-	if (!a->isnull && !b->isnull &&
+	COMPARE_SCALAR_FIELD(isnull);
+	/* Hack for in-line val field.  Also val is not valid if isnull is true */
+	if (!a->isnull &&
 		!equal(&a->val, &b->val))
 		return false;
-	COMPARE_SCALAR_FIELD(isnull);
 	COMPARE_LOCATION_FIELD(location);
 
 	return true;
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
@@ -305,14 +305,37 @@ _readA_Const(void)
 {
 	READ_LOCALS(A_Const);
 
+	/* We expect either NULL or :val here */
 	token = pg_strtok(&length);
 	if (length == 4 && strncmp(token, "NULL", 4) == 0)
 		local_node->isnull = true;
 	else
 	{
 		union ValUnion *tmp = nodeRead(NULL, 0);
 
-		memcpy(&local_node->val, tmp, sizeof(*tmp));
+		/* To forestall valgrind complaints, copy only the valid data */
+		switch (nodeTag(tmp))
+		{
+			case T_Integer:
+				memcpy(&local_node->val, tmp, sizeof(Integer));
+				break;
+			case T_Float:
+				memcpy(&local_node->val, tmp, sizeof(Float));
+				break;
+			case T_Boolean:
+				memcpy(&local_node->val, tmp, sizeof(Boolean));
+				break;
+			case T_String:
+				memcpy(&local_node->val, tmp, sizeof(String));
+				break;
+			case T_BitString:
+				memcpy(&local_node->val, tmp, sizeof(BitString));
+				break;
+			default:
+				elog(ERROR, "unrecognized node type: %d",
+					 (int) nodeTag(tmp));
+				break;
+		}
 	}
 
 	READ_LOCATION_FIELD(location);
