Add check on initial and boot values when loading GUCs

michaelpq · michaelpq · commit a73952b79563 · 2022-10-31T13:54:23.000+09:00
This commit adds a function to perform a cross-check between the initial value of the C declaration associated to a GUC and its actual boot value in assert-enabled builds. The purpose of this is to prevent anybody reading these C declarations from being fooled by mismatched values before they are loaded at program startup. The following rules apply depending on the GUC type: * bool - can be false, or same as boot_val. * int - can be 0, or same as the boot_val. * real - can be 0.0, or same as the boot_val. * string - can be NULL, or strcmp'd equal to the boot_val. * enum - equal to the boot_val. This is done for the system as well custom GUCs loaded by external modules, which may require extension developers to adapt the C declaration of the variables used by these GUCs (testing this change with some of my own modules has allowed me to catch some stupid typos, FWIW). This may finish by being a bad experiment depending on the feedbcak received, but let's see how it goes. Author: Peter Smith Reviewed-by: Nathan Bossart, Tom Lane, Michael Paquier, Justin Pryzby Discussion: https://postgr.es/m/CAHut+PtHE0XSfjjRQ6D4v7+dqzCw=d+1a64ujra4EX8aoc_Z+w@mail.gmail.com
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
@@ -1382,6 +1382,89 @@ check_GUC_name_for_parameter_acl(const char *name)
 	return false;
 }
 
+/*
+ * Routine in charge of checking that the initial value of a GUC is the
+ * same when declared and when loaded to prevent anybody looking at the
+ * C declarations of these GUCS from being fooled by mismatched values.
+ *
+ * The following validation rules apply:
+ * bool - can be false, otherwise must be same as the boot_val
+ * int  - can be 0, otherwise must be same as the boot_val
+ * real - can be 0.0, otherwise must be same as the boot_val
+ * string - can be NULL, otherwise must be strcmp equal to the boot_val
+ * enum - must be same as the boot_val
+ */
+#ifdef USE_ASSERT_CHECKING
+static bool
+check_GUC_init(struct config_generic *gconf)
+{
+	switch (gconf->vartype)
+	{
+		case PGC_BOOL:
+			{
+				struct config_bool *conf = (struct config_bool *) gconf;
+
+				if (*conf->variable && !conf->boot_val)
+				{
+					elog(LOG, "GUC (PGC_BOOL) %s, boot_val=%d, C-var=%d",
+						 conf->gen.name, conf->boot_val, *conf->variable);
+					return false;
+				}
+				break;
+			}
+		case PGC_INT:
+			{
+				struct config_int *conf = (struct config_int *) gconf;
+
+				if (*conf->variable != 0 && *conf->variable != conf->boot_val)
+				{
+					elog(LOG, "GUC (PGC_INT) %s, boot_val=%d, C-var=%d",
+						 conf->gen.name, conf->boot_val, *conf->variable);
+					return false;
+				}
+				break;
+			}
+		case PGC_REAL:
+			{
+				struct config_real *conf = (struct config_real *) gconf;
+
+				if (*conf->variable != 0.0 && *conf->variable != conf->boot_val)
+				{
+					elog(LOG, "GUC (PGC_REAL) %s, boot_val=%g, C-var=%g",
+						 conf->gen.name, conf->boot_val, *conf->variable);
+					return false;
+				}
+				break;
+			}
+		case PGC_STRING:
+			{
+				struct config_string *conf = (struct config_string *) gconf;
+
+				if (*conf->variable != NULL && strcmp(*conf->variable, conf->boot_val) != 0)
+				{
+					elog(LOG, "GUC (PGC_STRING) %s, boot_val=%s, C-var=%s",
+						 conf->gen.name, conf->boot_val ? conf->boot_val : "<null>", *conf->variable);
+					return false;
+				}
+				break;
+			}
+		case PGC_ENUM:
+			{
+				struct config_enum *conf = (struct config_enum *) gconf;
+
+				if (*conf->variable != conf->boot_val)
+				{
+					elog(LOG, "GUC (PGC_ENUM) %s, boot_val=%d, C-var=%d",
+						 conf->gen.name, conf->boot_val, *conf->variable);
+					return false;
+				}
+				break;
+			}
+	}
+
+	return true;
+}
+#endif
 
 /*
  * Initialize GUC options during program startup.
@@ -1413,6 +1496,9 @@ InitializeGUCOptions(void)
 	hash_seq_init(&status, guc_hashtab);
 	while ((hentry = (GUCHashEntry *) hash_seq_search(&status)) != NULL)
 	{
+		/* Check mapping between initial and default value */
+		Assert(check_GUC_init(hentry->gucvar));
+
 		InitializeOneGUCOption(hentry->gucvar);
 	}
 
@@ -4654,6 +4740,9 @@ define_custom_variable(struct config_generic *variable)
 	GUCHashEntry *hentry;
 	struct config_string *pHolder;
 
+	/* Check mapping between initial and default value */
+	Assert(check_GUC_init(variable));
+
 	/*
 	 * See if there's a placeholder by the same name.
 	 */
