Add VRT_MAJOR_VERSION and VRT_MINOR_VERSION as discussed on the Stockholm VDD.

Renovate and simplify the compile-time binding between VMODs, VCC
and varnishd.

bin/varnishd/cache/cache_vrt_vmod.c

@@ -39,7 +39,6 @@
 
 #include "vcli_priv.h"
 #include "vrt.h"
-#include "vmod_abi.h"
 
 /*--------------------------------------------------------------------
  * Modules stuff
@@ -64,10 +63,10 @@ static VTAILQ_HEAD(,vmod)	vmods = VTAILQ_HEAD_INITIALIZER(vmods);
 
 int
 VRT_Vmod_Init(void **hdl, void *ptr, int len, const char *nm,
-    const char *path, struct cli *cli)
+    const char *path, const char *file_id, struct cli *cli)
 {
 	struct vmod *v;
-	void *x, *y, *z, *w;
+	const struct vmod_data *d;
 	char buf[256];
 	void *dlhdl;
 
@@ -90,50 +89,37 @@ VRT_Vmod_Init(void **hdl, void *ptr, int len, const char *nm,
 
 		v->hdl = dlhdl;
 
-		bprintf(buf, "Vmod_%s_Name", nm);
-		x = dlsym(v->hdl, buf);
-		bprintf(buf, "Vmod_%s_Len", nm);
-		y = dlsym(v->hdl, buf);
-		bprintf(buf, "Vmod_%s_Func", nm);
-		z = dlsym(v->hdl, buf);
-		bprintf(buf, "Vmod_%s_ABI", nm);
-		w = dlsym(v->hdl, buf);
-		if (x == NULL || y == NULL || z == NULL || w == NULL) {
+		bprintf(buf, "Vmod_%s_Data", nm);
+		d = dlsym(v->hdl, buf);
+		if (d == NULL ||
+		    d->file_id == NULL ||
+		    strcmp(d->file_id, file_id)) {
 			VCLI_Out(cli, "Loading VMOD %s from %s:\n", nm, path);
-			VCLI_Out(cli, "VMOD symbols not found\n");
-			VCLI_Out(cli, "Check relative pathnames.\n");
+			VCLI_Out(cli,
+			    "This is no longer the same file seen by"
+			    " the VCL-compiler.\n");
 			(void)dlclose(v->hdl);
 			FREE_OBJ(v);
 			return (1);
 		}
-		AN(x);
-		AN(y);
-		AN(z);
-		AN(w);
-		if (strcmp(x, nm)) {
+		if (d->vrt_major != VRT_MAJOR_VERSION ||
+		    d->vrt_minor > VRT_MINOR_VERSION ||
+		    d->name == NULL ||
+		    strcmp(d->name, nm) ||
+		    d->func == NULL ||
+		    d->func_len <= 0 ||
+		    d->proto == NULL ||
+		    d->spec == NULL ||
+		    d->abi == NULL) {
 			VCLI_Out(cli, "Loading VMOD %s from %s:\n", nm, path);
-			VCLI_Out(cli, "File contain wrong VMOD (\"%s\")\n",
-			    (char *) x);
-			VCLI_Out(cli, "Check relative pathnames ?.\n");
+			VCLI_Out(cli, "VMOD data is mangled.\n");
 			(void)dlclose(v->hdl);
 			FREE_OBJ(v);
 			return (1);
 		}
 
-		if (strcmp(w, VMOD_ABI_Version)) {
-			VCLI_Out(cli, "Loading VMOD %s from %s:\n", nm, path);
-			VCLI_Out(cli, "VMOD ABI (%s)", (char*)w);
-			VCLI_Out(cli, " incompatible with varnish ABI (%s)\n",
-			    VMOD_ABI_Version);
-			(void)dlclose(v->hdl);
-			FREE_OBJ(v);
-			return (1);
-		}
-
-		// XXX: Check w for ABI version compatibility
-
-		v->funclen = *(const int *)y;
-		v->funcs = z;
+		v->funclen = d->func_len;
+		v->funcs = d->func;
 
 		REPLACE(v->nm, nm);
 		REPLACE(v->path, path);

bin/varnishd/flint.lnt

@@ -58,18 +58,8 @@
 
 // Stuff in VMODs which is used through dl*(3) functions
 -esym(754, Vmod_*_Func::*)
--esym(765, Vmod_*_Func)
--esym(552, Vmod_*_Func)
--esym(765, Vmod_*_Len)
--esym(714, Vmod_*_Len)
--esym(765, Vmod_*_Name)
--esym(714, Vmod_*_Name)
--esym(765, Vmod_*_Proto)
--esym(714, Vmod_*_Proto)
--esym(765, Vmod_*_Spec)
--esym(714, Vmod_*_Spec)
--esym(765, Vmod_*_ABI)
--esym(714, Vmod_*_ABI)
+-esym(714, Vmod_*_Data)
+-esym(765, Vmod_*_Data)
 
 
 //-sem (pthread_mutex_lock, thread_lock)

include/vrt.h

@@ -26,11 +26,26 @@
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  *
- * Runtime support for compiled VCL programs.
+ * Runtime support for compiled VCL programs and VMODs.
  *
- * XXX: When this file is changed, lib/libvcc/generate.py *MUST* be rerun.
+ * NB: When this file is changed, lib/libvcc/generate.py *MUST* be rerun.
  */
 
+/***********************************************************************
+ * Major and minor VRT API versions.
+ *
+ * Whenever something is added, increment MINOR version
+ * Whenever something is deleted or changed in a way which is not
+ * binary/load-time compatible, increment MAJOR version
+ */
+
+#define VRT_MAJOR_VERSION	1U
+
+#define VRT_MINOR_VERSION	1U
+
+
+/***********************************************************************/
+
 struct req;
 struct busyobj;
 struct vsl_log;
@@ -89,6 +104,22 @@ struct vrt_ctx {
 
 /***********************************************************************/
 
+struct vmod_data {
+	/* The version/id fields must be first, they protect the rest */
+	unsigned			vrt_major;
+	unsigned			vrt_minor;
+	const char			*file_id;
+
+	const char			*name;
+	const void			*func;
+	int				func_len;
+	const char			*proto;
+	const char			* const *spec;
+	const char			*abi;
+};
+
+/***********************************************************************/
+
 enum gethdr_e { HDR_REQ, HDR_RESP, HDR_OBJ, HDR_BEREQ, HDR_BERESP };
 
 struct gethdr_s {
@@ -230,7 +261,7 @@ int VRT_VSA_GetPtr(const struct suckaddr *sua, const unsigned char ** dst);
 
 /* VMOD/Modules related */
 int VRT_Vmod_Init(void **hdl, void *ptr, int len, const char *nm,
-    const char *path, struct cli *cli);
+    const char *path, const char *file_id, struct cli *cli);
 void VRT_Vmod_Fini(void **hdl);
 
 struct vmod_priv;

lib/libvcc/vcc_vmod.c

@@ -35,6 +35,7 @@
 #include "vcc_compile.h"
 
 #include "vmod_abi.h"
+#include "vrt.h"
 
 void
 vcc_ParseImport(struct vcc *tl)
@@ -44,14 +45,12 @@ vcc_ParseImport(struct vcc *tl)
 	char buf[256];
 	struct token *mod, *t1;
 	struct inifin *ifp;
-	const char *modname;
-	const char *proto;
-	const char *abi;
-	const char **spec;
+	const char * const *spec;
 	struct symbol *sym;
 	const struct symbol *osym;
 	const char *p;
 	// int *modlen;
+	const struct vmod_data *vmd;
 
 	t1 = tl->t;
 	SkipToken(tl, ID);		/* "import" */
@@ -62,7 +61,7 @@ vcc_ParseImport(struct vcc *tl)
 
 	osym = VCC_FindSymbol(tl, mod, SYM_NONE);
 	if (osym != NULL && osym->kind != SYM_VMOD) {
-		VSB_printf(tl->sb, "Module %.*s conflics with other symbol.\n",
+		VSB_printf(tl->sb, "Module %.*s conflicts with other symbol.\n",
 		    PF(mod));
 		vcc_ErrWhere2(tl, t1, tl->t);
 		return;
@@ -104,77 +103,88 @@ vcc_ParseImport(struct vcc *tl)
 		bprintf(fn, "%s/libvmod_%.*s.so", tl->vmod_dir, PF(mod));
 	}
 
-	ifp = New_IniFin(tl);
-
-	VSB_printf(ifp->ini, "\tif (VRT_Vmod_Init(&VGC_vmod_%.*s,\n", PF(mod));
-	VSB_printf(ifp->ini, "\t    &Vmod_%.*s_Func,\n", PF(mod));
-	VSB_printf(ifp->ini, "\t    sizeof(Vmod_%.*s_Func),\n", PF(mod));
-	VSB_printf(ifp->ini, "\t    \"%.*s\",\n", PF(mod));
-	VSB_printf(ifp->ini, "\t    ");
-	EncString(ifp->ini, fn, NULL, 0);
-	VSB_printf(ifp->ini, ",\n\t    ");
-	VSB_printf(ifp->ini, "cli))\n");
-	VSB_printf(ifp->ini, "\t\treturn(1);");
-
-	/* XXX: zero the function pointer structure ?*/
-	VSB_printf(ifp->fin, "\tVRT_priv_fini(&vmod_priv_%.*s);", PF(mod));
-	VSB_printf(ifp->fin, "\n\tVRT_Vmod_Fini(&VGC_vmod_%.*s);", PF(mod));
-
-	ifp = NULL;
-
 	SkipToken(tl, ';');
 
 	hdl = dlopen(fn, RTLD_NOW | RTLD_LOCAL);
 	if (hdl == NULL) {
-		VSB_printf(tl->sb, "Could not load module %.*s\n\t%s\n\t%s\n",
-		    PF(mod), fn, dlerror());
+		VSB_printf(tl->sb, "Could not load VMOD %.*s\n", PF(mod));
+		VSB_printf(tl->sb, "\tFile name: %s\n", fn);
+		VSB_printf(tl->sb, "\tdlerror:: %s\n", dlerror());
 		vcc_ErrWhere(tl, mod);
 		return;
 	}
 
-	bprintf(buf, "Vmod_%.*s_Name", PF(mod));
-	modname = dlsym(hdl, buf);
-	if (modname == NULL) {
-		VSB_printf(tl->sb, "Could not load module %.*s\n\t%s\n\t%s\n",
-		    PF(mod), fn, "Symbol Vmod_Name not found");
+	bprintf(buf, "Vmod_%.*s_Data", PF(mod));
+	vmd = dlsym(hdl, buf);
+	if (vmd == NULL) {
+		VSB_printf(tl->sb, "Malformed VMOD %.*s\n", PF(mod));
+		VSB_printf(tl->sb, "\tFile name: %s\n", fn);
+		VSB_printf(tl->sb, "\t(no Vmod_Data symbol)\n");
 		vcc_ErrWhere(tl, mod);
 		return;
 	}
-	if (!vcc_IdIs(mod, modname)) {
-		VSB_printf(tl->sb, "Could not load module %.*s\n\t%s\n",
-		    PF(mod), fn);
-		VSB_printf(tl->sb, "\tModule has wrong name: <%s>\n", modname);
+	if (vmd->vrt_major != VRT_MAJOR_VERSION ||
+	    vmd->vrt_minor > VRT_MINOR_VERSION) {
+		VSB_printf(tl->sb, "Incompatible VMOD %.*s\n", PF(mod));
+		VSB_printf(tl->sb, "\tFile name: %s\n", fn);
+		VSB_printf(tl->sb, "\tVMOD version %u.%u\n",
+		    vmd->vrt_major, vmd->vrt_minor);
+		VSB_printf(tl->sb, "\tvarnishd version %u.%u\n",
+		    VRT_MAJOR_VERSION, VRT_MINOR_VERSION);
 		vcc_ErrWhere(tl, mod);
 		return;
 	}
-
-	bprintf(buf, "Vmod_%.*s_ABI", PF(mod));
-	abi = dlsym(hdl, buf);
-	if (abi == NULL || strcmp(abi, VMOD_ABI_Version) != 0) {
-		VSB_printf(tl->sb, "Could not load module %.*s\n\t%s\n",
-		    PF(mod), fn);
-		VSB_printf(tl->sb, "\tABI mismatch, expected <%s>, got <%s>\n",
-			   VMOD_ABI_Version, abi);
+	if (vmd->name == NULL ||
+	    vmd->func == NULL ||
+	    vmd->func_len <= 0 ||
+	    vmd->proto == NULL ||
+	    vmd->abi == NULL) {
+		VSB_printf(tl->sb, "Mangled VMOD %.*s\n", PF(mod));
+		VSB_printf(tl->sb, "\tFile name: %s\n", fn);
+		VSB_printf(tl->sb, "\tInconsistent metadata\n");
 		vcc_ErrWhere(tl, mod);
 		return;
 	}
 
-	bprintf(buf, "Vmod_%.*s_Proto", PF(mod));
-	proto = dlsym(hdl, buf);
-	if (proto == NULL) {
-		VSB_printf(tl->sb, "Could not load module %.*s\n\t%s\n\t%s\n",
-		    PF(mod), fn, "Symbol Vmod_Proto not found");
+	if (!vcc_IdIs(mod, vmd->name)) {
+		VSB_printf(tl->sb, "Wrong VMOD file %.*s\n", PF(mod));
+		VSB_printf(tl->sb, "\tFile name: %s\n", fn);
+		VSB_printf(tl->sb, "\tContains vmod \"%s\"\n", vmd->name);
 		vcc_ErrWhere(tl, mod);
 		return;
 	}
-	bprintf(buf, "Vmod_%.*s_Spec", PF(mod));
-	spec = dlsym(hdl, buf);
-	if (spec == NULL) {
-		VSB_printf(tl->sb, "Could not load module %.*s\n\t%s\n\t%s\n",
-		    PF(mod), fn, "Symbol Vmod_Spec not found");
+
+	if (strcmp(vmd->abi, VMOD_ABI_Version) != 0) {
+		VSB_printf(tl->sb, "Incompatible VMOD %.*s\n", PF(mod));
+		VSB_printf(tl->sb, "\tFile name: %s\n", fn);
+		VSB_printf(tl->sb, "\tABI mismatch, expected <%s>, got <%s>\n",
+			   VMOD_ABI_Version, vmd->abi);
 		vcc_ErrWhere(tl, mod);
 		return;
 	}
+
+	ifp = New_IniFin(tl);
+
+	VSB_printf(ifp->ini, "\tif (VRT_Vmod_Init(&VGC_vmod_%.*s,\n", PF(mod));
+	VSB_printf(ifp->ini, "\t    &Vmod_%.*s_Func,\n", PF(mod));
+	VSB_printf(ifp->ini, "\t    sizeof(Vmod_%.*s_Func),\n", PF(mod));
+	VSB_printf(ifp->ini, "\t    \"%.*s\",\n", PF(mod));
+	VSB_printf(ifp->ini, "\t    ");
+	EncString(ifp->ini, fn, NULL, 0);
+	VSB_printf(ifp->ini, ",\n");
+	AN(vmd);
+	AN(vmd->file_id);
+	VSB_printf(ifp->ini, "\t    \"%s\",\n", vmd->file_id);
+	VSB_printf(ifp->ini, "\t    cli))\n");
+	VSB_printf(ifp->ini, "\t\treturn(1);");
+
+	/* XXX: zero the function pointer structure ?*/
+	VSB_printf(ifp->fin, "\tVRT_priv_fini(&vmod_priv_%.*s);", PF(mod));
+	VSB_printf(ifp->fin, "\n\tVRT_Vmod_Fini(&VGC_vmod_%.*s);", PF(mod));
+
+	ifp = NULL;
+
+	spec = vmd->spec;
 	for (; *spec != NULL; spec++) {
 		p = *spec;
 		if (!strcmp(p, "OBJ")) {
@@ -208,6 +218,6 @@ vcc_ParseImport(struct vcc *tl)
 	Fh(tl, 0, "\n/* --- BEGIN VMOD %.*s --- */\n\n", PF(mod));
 	Fh(tl, 0, "static void *VGC_vmod_%.*s;\n", PF(mod));
 	Fh(tl, 0, "static struct vmod_priv vmod_priv_%.*s;\n", PF(mod));
-	Fh(tl, 0, "\n%s\n", proto);
+	Fh(tl, 0, "\n%s\n", vmd->proto);
 	Fh(tl, 0, "\n/* --- END VMOD %.*s --- */\n\n", PF(mod));
 }

lib/libvcc/vmodtool.py

@@ -40,6 +40,7 @@ import sys
 import re
 import optparse
 import unittest
+import random
 from os import unlink
 from os.path import dirname, realpath, exists
 from pprint import pprint, pformat
@@ -196,29 +197,17 @@ class Vmod(object):
 				fo.write(j + "\n")
 
 	def c_vmod(self, fo):
-		fo.write('extern const char Vmod_' + self.nam + '_Name[];\n')
-		fo.write('const char Vmod_' + self.nam + '_Name[] =')
-		fo.write(' \"' + self.nam + '";\n')
-		fo.write("\n")
 
 		cs = self.c_struct()
 		fo.write(cs + ';\n')
 
-		vfn = 'Vmod_' + self.nam + '_Func'
+		vfn = 'Vmod_%s_Func' % self.nam
 
-		fo.write("extern const struct " + vfn + " " + vfn + ';\n')
-		fo.write("const struct " + vfn + " " + vfn + ' =')
+		fo.write("\nstatic const struct %s Vmod_Func =" % vfn)
 		fo.write(self.c_initializer())
 		fo.write("\n")
 
-		fo.write("\n")
-		fo.write("extern const int Vmod_" + self.nam + '_Len;\n')
-		fo.write("const int Vmod_" + self.nam + '_Len =')
-		fo.write(" sizeof(Vmod_" + self.nam + "_Func);\n")
-		fo.write("\n")
-
-		fo.write("extern const char Vmod_" + self.nam + "_Proto[];\n")
-		fo.write("const char Vmod_" + self.nam + "_Proto[] =\n")
+		fo.write("\nstatic const char Vmod_Proto[] =\n")
 		for t in self.c_typedefs_():
 			for i in lwrap(t, w=64):
 				fo.write('\t"' + i + '\\n"\n')
@@ -230,12 +219,28 @@ class Vmod(object):
 		fo.write(self.c_strspec())
 
 		fo.write("\n")
-		fo.write('extern const char Vmod_' + self.nam + '_ABI[];\n')
-		fo.write('const char Vmod_' + self.nam + '_ABI[] =')
-		fo.write(' VMOD_ABI_Version;\n')
-		#fo.write("\n")
-		#fo.write('const void * const Vmod_' + self.nam + '_Id =')
-		#fo.write(' &Vmod_' + self.nam + '_Id;\n')
+
+		nm = "Vmod_" + self.nam + "_Data"
+		fo.write("extern const struct vmod_data " + nm + ";\n\n")
+		fo.write("const struct vmod_data " + nm + " = {\n")
+		fo.write("\t.vrt_major = VRT_MAJOR_VERSION,\n");
+		fo.write("\t.vrt_minor = VRT_MINOR_VERSION,\n");
+		fo.write("\t.name = \"%s\",\n" % self.nam)
+		fo.write("\t.func = &Vmod_Func,\n")
+		fo.write("\t.func_len = sizeof(Vmod_Func),\n")
+		fo.write("\t.proto = Vmod_Proto,\n")
+		fo.write("\t.spec = Vmod_Spec,\n")
+		fo.write("\t.abi = VMOD_ABI_Version,\n")
+
+		# NB: Sort of hackish:
+		# Fill file_id with random stuff, so we can tell if
+		# VCC and VRT_Vmod_Init() dlopens the same file
+		#
+		fo.write("\t.file_id = \"")
+		for i in range(32):
+			fo.write("%c" % random.randint(0x23,0x5b))
+		fo.write("\",\n")
+		fo.write("};\n")
 
 	def c_initializer(self):
 		s = '{\n'
@@ -269,8 +274,7 @@ class Vmod(object):
 		return s
 
 	def c_strspec(self):
-		s = "const char * const Vmod_" + self.nam + "_Spec[]"
-		s = "extern " + s + ";\n" + s + " = {\n"
+		s = "static const char * const Vmod_Spec[] = {\n"
 
 		for o in self.objs:
 			s += o.c_strspec(self.nam) + ",\n\n"