Avoid copying undefined data in _readA_Const().

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 a6bc33019,
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
index b8a0452565f5b61ee966d8b1a14912a310539fb8..06d498f02962714747c05e65e5fc69e5125921dc 100644 (file)
--- 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
index f3629cdfd14b876f8b10bcd93edc05384b33a600..597e5b3ea8b94d055693d9a698f7b16844200fae 100644 (file)
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -305,6 +305,7 @@ _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;
@@ -312,7 +313,29 @@ _readA_Const(void)
    {
        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);