[Alvaro Herrera <alvherre@2ndquadrant.com>]

Hello

I just came across a crash while debugging some corrupted system
catalogs; pg_identify_object_as_address fails to cope with some NULL
input, causing a crash.  Attached patch fixes it.  Naturally, the output
array will contain NULL members in the output, but that's better than
crashing ...

(The first hunk is purely speculative; I haven't seen anything that
requires that.  The actual fix is in the other hunks.  But seems better
to be defensive.)

The crash can be reproduced thusly

create function f() returns int language plpgsql as $$ begin return 1; end; $$;
update pg_proc set pronamespace = 9999 where proname = 'f' returning oid \gset
select * from pg_identify_object_as_address('pg_proc'::regclass, :oid, 0);

After the patch, the last line returns:

   type   | object_names | object_args 
----------+--------------+-------------
 function | {NULL,f}     | {}

where the NULL obviously corresponds to the bogus pg_namespace OID being
referenced.

The patch is on 9.6.  I checked 10 and it applies fine there.  My
intention is to apply to all branches since 9.5.

-- 
Álvaro Herrera                PostgreSQL Expert, https://www.2ndQuadrant.com/

Attachments:

pg_identify_object_as_address-crash.patch (text/x-diff, 1.5K) download
diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index bd55dfd08e3..479087592da 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -3576,7 +3576,10 @@ pg_identify_object_as_address(PG_FUNCTION_ARGS)
 	pfree(identity);
 
 	/* object_names */
-	values[1] = PointerGetDatum(strlist_to_textarray(names));
+	if (names != NIL)
+		values[1] = PointerGetDatum(strlist_to_textarray(names));
+	else
+		values[1] = PointerGetDatum(construct_empty_array(TEXTOID));
 	nulls[1] = false;
 
 	/* object_args */
@@ -4803,10 +4806,12 @@ strlist_to_textarray(List *list)
 {
 	ArrayType  *arr;
 	Datum	   *datums;
+	bool	   *nulls;
 	int			j = 0;
 	ListCell   *cell;
 	MemoryContext memcxt;
 	MemoryContext oldcxt;
+	int			one = 1;
 
 	memcxt = AllocSetContextCreate(CurrentMemoryContext,
 								   "strlist to array",
@@ -4814,17 +4819,24 @@ strlist_to_textarray(List *list)
 	oldcxt = MemoryContextSwitchTo(memcxt);
 
 	datums = palloc(sizeof(text *) * list_length(list));
+	nulls = palloc(sizeof(bool) * list_length(list));
 	foreach(cell, list)
 	{
 		char	   *name = lfirst(cell);
 
-		datums[j++] = CStringGetTextDatum(name);
+		if (name)
+		{
+			nulls[j] = false;
+			datums[j++] = CStringGetTextDatum(name);
+		}
+		else
+			nulls[j++] = true;
 	}
 
 	MemoryContextSwitchTo(oldcxt);
 
-	arr = construct_array(datums, list_length(list),
-						  TEXTOID, -1, false, 'i');
+	arr = construct_md_array(datums, nulls, 1, &j,
+							 &one, TEXTOID, -1, false, 'i');
 	MemoryContextDelete(memcxt);
 
 	return arr;