Change PointerGetDatum() back to a macro

The argument was marked as "const void *X", but that might rightly
give the compiler the idea that *X cannot be modified through the
resulting Datum, and make incorrect optimizations based on that. Some
functions use pointer Datums to pass output arguments, like GIN
support functions. Coverity started to complain after commit
6f5ad00ab7 that there's dead code in ginExtractEntries(), because it
didn't see that it passes PointerGetDatum(&nentries) to a function
that sets it.

This issue goes back to commit c8b2ef05f481 (version 16), which
changed PointerGetDatum() from a macro to a static inline function.
This commit changes it back to a macro, but uses a trick with a dummy
conditional expression to still produce a compiler error if you try to
pass a non-pointer as the argument.

Even though this goes back to v16, I'm only committing this to
'master' for now, to verify that this silences the Coverity warning.
If this works, we might want to introduce separate const and non-const
versions of PointerGetDatum() instead of this, but that's a bigger
patch.  It's also not decided yet whether to back-patch this (or some
other fix), given that we haven't yet seen any hard evidence of
compilers actually producing buggy code because of this.

Discussion: https://www.postgresql.org/message-id/342012.1776017102@sss.pgh.pa.us

diff --git a/src/include/postgres.h b/src/include/postgres.h
index 046ba678faf601af80f8e6d0585f4d3c7643e4bc..87f6bd494993070e6359dcb42fd464c53797ee84 100644 (file)
--- a/src/include/postgres.h
+++ b/src/include/postgres.h
@@ -337,12 +337,22 @@ DatumGetPointer(Datum X)
 /*
  * PointerGetDatum
  *             Returns datum representation for a pointer.
+ *
+ * This used to be defined as "static inline Datum PointerGetDatum(const void
+ * *X) ", but it had the problem that the compiler would see the const
+ * attribute, and could rightly assume that the function won't modify *X.
+ * While PointerGetDatum() itself doesn't modify *X, the resulting Datum could
+ * later be passed to a function that converts it back to a non-const pointer
+ * and modifies it.  Most functions don't modify their arguments passed by
+ * reference - that would be very bogus for any operators or functions exposed
+ * in SQL - but some functions like GIN support functions do have output
+ * arguments that are pointer Datums.
+ *
+ * The odd-looking "true ? (X) : NULL" conditional expression has the effect
+ * of producing a compiler error if X is not a pointer.
  */
-static inline Datum
-PointerGetDatum(const void *X)
-{
-       return (Datum) (uintptr_t) X;
-}
+#define PointerGetDatum(X) \
+       ((Datum) (uintptr_t) (true ? (X) : NULL))
 
 /*
  * DatumGetCString
@@ -361,6 +371,9 @@ DatumGetCString(Datum X)
  * CStringGetDatum
  *             Returns datum representation for a C string (null-terminated string).
  *
+ * We assume that the resulting Datum is not used to modify the string, hence
+ * the argument can be marked as const.
+ *
  * Note: C string is not a full-fledged Postgres type at present,
  * but type output functions use this conversion for their outputs.
  * Note: CString is pass-by-reference; caller must ensure the pointed-to