[master] 06696692f Support dynamic SUBs with VRT_call() / VRT_check_call()
Nils Goroll
nils.goroll at uplex.de
Mon Feb 8 17:52:04 UTC 2021
commit 06696692fd9fa24b799c44cd6b83fc620cc6a30b
Author: Nils Goroll <nils.goroll at uplex.de>
Date: Sat Feb 6 15:21:02 2021 +0100
Support dynamic SUBs with VRT_call() / VRT_check_call()
This allows for VCL subs to be called dynamically at runtime: vmods
may store references to VCL_SUBs and call them some time later.
Calling a VCL_SUB pointer requires to conduct two fundamental checks
also at runtime, which previously were implemented by VCC at compile
time only:
* checking the allowed context
* checking for recursive calls
The foundation for both has been laid in previous commits and has
been made available through VPI_Call_*().
Note that we do not change the VCC checks in any way for static calls,
we only add runtime checks for dynamic calls.
This commit adds a VRT interface for dynamic calls and changes VGC to
ensure proper runtime checking (as suggested by phk).
We add to the vcl_func_f signature
- a vcl_func_call_e argument denoting the type of call and
- a vcl_func_fail_e value argument to optionally return a failure code
On vcl_func_call_e:
VSUB_CHECK allows runtime callers to preflight a "can this sub be
called" check, which is the basis for VRT_check_call().
VSUB_DYNAMIC must be used by any calls outside VGC.
VSUB_STATIC is for VGC only and, strictly speaking, would not be
required. It is added for clarity and safety and used for "call"
actions from VCL.
On vcl_func_fail_e:
If the argument is present, any error will be returned in
it. Otherwise, the generated code fails the VCL.
On the implementation:
To minimize the overhead for runtime context and recursion checks, we
only add them where needed.
In previous commits, we changed the VCC walks over SUBs to ensure that
any dynamically referenced SUBs and all SUBs reachable from these via
"call" can be identified by having more references than calls
(p->sym->nref > p->called).
So we now know, at compile time, for which SUBs we need to add runtime
checks. Read
https://github.com/varnishcache/varnish-cache/pull/3163#issuecomment-770266098
for more details.
Depending on whether the SUB is dynamically callable or not, we either
add runtime checks or keep the code identical to before (except for a
changed signature and some assertions).
For the dynamic case, we need to wrap the actual SUB in order to clear
the recursion check bit when it returns. Note that this happens
independend of whether the actuall call is static or dynamic - because
it has to. The recursion check for dynamic calls requires any previous
static calls to be registered.
The VGC for the dynamic case looks like this (slightly edited for
clarity):
static void
VGC_function_foo_checked(VRT_CTX)
{
assert(ctx->method & (VCL_MET_DELIVER|VCL_MET_SYNTH));
{ { VPI_count(ctx, 1);
// ... as before
void v_matchproto_(vcl_func_f)
VGC_function_foo(VRT_CTX, enum vcl_func_call_e call,
enum vcl_func_fail_e *failp)
{
enum vcl_func_fail_e fail;
fail = VPI_Call_Check(ctx, &VCL_conf, 0x300, 0);
if (failp)
*failp = fail;
else if (fail == VSUB_E_METHOD)
VRT_fail(ctx, "call to \"sub foo{}\" not allowed from here");
else if (fail == VSUB_E_RECURSE)
VRT_fail(ctx, "Recursive call to \"sub foo{}\" not allowed from here");
else
assert(fail == VSUB_E_OK);
if (fail != VSUB_E_OK || call == VSUB_CHECK)
return;
VPI_Call_Begin(ctx, 0);
VGC_function_foo_checked(ctx);
VPI_Call_End(ctx, 0);
}
The actual function body remains unchanged, but is contained in the
static function named "*_checked". The vcl_func_f is now a wrapper.
The wrapper
- checks for context errors using VPI_Call_Check()
- either returns any error in the failp argument or fails the VCL
- encoses the call to the "_checked" function with VPI_Call_Begin /
VPI_Call_End which set/clear the recursion marker bit.
diff --git a/bin/varnishd/cache/cache_vrt_vcl.c b/bin/varnishd/cache/cache_vrt_vcl.c
index b474ea4e9..8d20c7848 100644
--- a/bin/varnishd/cache/cache_vrt_vcl.c
+++ b/bin/varnishd/cache/cache_vrt_vcl.c
@@ -487,7 +487,7 @@ vcl_call_method(struct worker *wrk, struct req *req, struct busyobj *bo,
wrk->seen_methods |= method;
AN(ctx.vsl);
VSLb(ctx.vsl, SLT_VCL_call, "%s", VCL_Method_Name(method));
- func(&ctx);
+ func(&ctx, VSUB_STATIC, NULL);
VSLb(ctx.vsl, SLT_VCL_return, "%s", VCL_Return_Name(wrk->handling));
wrk->cur_method |= 1; // Magic marker
if (wrk->handling == VCL_RET_FAIL)
@@ -517,3 +517,51 @@ VCL_##func##_method(struct vcl *vcl, struct worker *wrk, \
}
#include "tbl/vcl_returns.h"
+
+/*--------------------------------------------------------------------
+ */
+
+VCL_STRING
+VRT_check_call(VRT_CTX, VCL_SUB sub)
+{
+ VCL_STRING err = NULL;
+ enum vcl_func_fail_e fail;
+
+ CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
+ CHECK_OBJ_NOTNULL(sub, VCL_SUB_MAGIC);
+
+ AN(sub->func);
+ sub->func(ctx, VSUB_CHECK, &fail);
+
+ switch (fail) {
+ case VSUB_E_OK:
+ break;
+ case VSUB_E_METHOD:
+ err = WS_Printf(ctx->ws, "Dynamic call to \"sub %s{}\""
+ " not allowed from here", sub->name);
+ if (err == NULL)
+ err = "Dynamic call not allowed and workspace overflow";
+ break;
+ case VSUB_E_RECURSE:
+ err = WS_Printf(ctx->ws, "Recursive dynamic call to"
+ " \"sub %s{}\"", sub->name);
+ if (err == NULL)
+ err = "Recursive dynamic call and workspace overflow";
+ break;
+ default:
+ INCOMPL();
+ }
+
+ return (err);
+}
+
+VCL_VOID
+VRT_call(VRT_CTX, VCL_SUB sub)
+{
+
+ CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
+ CHECK_OBJ_NOTNULL(sub, VCL_SUB_MAGIC);
+
+ AN(sub->func);
+ sub->func(ctx, VSUB_DYNAMIC, NULL);
+}
diff --git a/doc/changes.rst b/doc/changes.rst
index 4b7c61886..e98abd650 100644
--- a/doc/changes.rst
+++ b/doc/changes.rst
@@ -56,6 +56,19 @@ Varnish Cache Next (2021-03-15)
* All shard ``Error`` and ``Notice`` messages now use the unified
prefix ``vmod_directors: shard %s``.
+* The ``VCL_SUB`` data type is now supported for VMODs to save
+ references to subroutines to be called later using
+ ``VRT_call()``. Calls from a wrong context (e.g. calling a
+ subroutine accessing ``req`` from the backend side) and recursive
+ calls fail the VCL.
+
+* ``VRT_check_call()`` can be used to check if a ``VRT_call()`` would
+ succeed in order to avoid the potential VCL failure in case it would
+ not.
+
+ It returns ``NULL`` if ``VRT_call()`` would make the call or an
+ error string why not.
+
================================
Varnish Cache 6.5.1 (2020-09-25)
================================
diff --git a/doc/sphinx/reference/vmod.rst b/doc/sphinx/reference/vmod.rst
index f9d49d01c..8eb6baa8a 100644
--- a/doc/sphinx/reference/vmod.rst
+++ b/doc/sphinx/reference/vmod.rst
@@ -460,6 +460,26 @@ TIME
An absolute time, as in 1284401161.
+VCL_SUB
+ C-type: ``const struct vcl_sub *``
+
+ Opaque handle on a VCL subroutine.
+
+ References to subroutines can be passed into VMODs as
+ arguments and called later through ``VRT_call()``. The scope
+ strictly is the VCL: vmods must ensure that ``VCL_SUB``
+ references never be called from a different VCL.
+
+ ``VRT_call()`` fails the VCL for recursive calls and when the
+ ``VCL_SUB`` can not be called from the current context
+ (e.g. calling a subroutine accessing ``req`` from the backend
+ side).
+
+ ``VRT_check_call()`` can be used to check if a ``VRT_call()``
+ would succeed in order to avoid the potential VCL failure. It
+ returns ``NULL`` if ``VRT_call()`` would make the call or an
+ error string why not.
+
VOID
C-type: ``void``
diff --git a/include/vrt.h b/include/vrt.h
index ffb6d2abc..4071756a7 100644
--- a/include/vrt.h
+++ b/include/vrt.h
@@ -70,6 +70,8 @@
* VRT_re_fini removed
* VRT_re_match signature changed
* VRT_regsub signature changed
+ * VRT_call() added
+ * VRT_check_call() added
* 12.0 (2020-09-15)
* Added VRT_DirectorResolve()
* Added VCL_STRING VRT_BLOB_string(VRT_CTX, VCL_BLOB)
@@ -339,13 +341,19 @@ struct vrt_ctx {
#define VRT_CTX const struct vrt_ctx *ctx
void VRT_CTX_Assert(VRT_CTX);
+enum vcl_func_call_e {
+ VSUB_STATIC, // VCL "call" action, only allowed from VCC
+ VSUB_DYNAMIC, // VRT_call()
+ VSUB_CHECK // VRT_check_call()
+};
+
enum vcl_func_fail_e {
VSUB_E_OK,
VSUB_E_RECURSE, // call would recurse
VSUB_E_METHOD // can not be called from this method
};
-typedef void vcl_func_f(VRT_CTX);
+typedef void vcl_func_f(VRT_CTX, enum vcl_func_call_e, enum vcl_func_fail_e *);
/***********************************************************************
* This is the interface structure to a compiled VMOD
@@ -416,6 +424,10 @@ VCL_STRING VRT_StrandsWS(struct ws *, const char *, VCL_STRANDS);
VCL_STRING VRT_CollectStrands(VRT_CTX, VCL_STRANDS);
VCL_STRING VRT_UpperLowerStrands(VRT_CTX, VCL_STRANDS s, int up);
+/* VCL_SUB */
+VCL_STRING VRT_check_call(VRT_CTX, VCL_SUB);
+VCL_VOID VRT_call(VRT_CTX, VCL_SUB);
+
/* Functions to turn types into canonical strings */
VCL_STRING VRT_BACKEND_string(VCL_BACKEND);
diff --git a/lib/libvcc/vcc_action.c b/lib/libvcc/vcc_action.c
index 45eb278ca..4a7ee7de2 100644
--- a/lib/libvcc/vcc_action.c
+++ b/lib/libvcc/vcc_action.c
@@ -53,7 +53,8 @@ vcc_act_call(struct vcc *tl, struct token *t, struct symbol *sym)
if (sym != NULL) {
vcc_AddCall(tl, t0, sym);
VCC_GlobalSymbol(sym, SUB, "VGC_function");
- Fb(tl, 1, "%s(ctx);\n", sym->lname);
+
+ Fb(tl, 1, "%s(ctx, VSUB_STATIC, NULL);\n", sym->lname);
SkipToken(tl, ';');
}
}
diff --git a/lib/libvcc/vcc_compile.c b/lib/libvcc/vcc_compile.c
index e9fb25b8b..df20b6e45 100644
--- a/lib/libvcc/vcc_compile.c
+++ b/lib/libvcc/vcc_compile.c
@@ -171,8 +171,10 @@ static void
vcc_EmitProc(struct vcc *tl, struct proc *p)
{
struct vsb *vsbm;
- unsigned mask;
+ unsigned mask, nsub;
const char *maskcmp;
+ const char *cc_adv;
+ int dyn = (p->sym->nref > p->called);
AN(p->okmask);
AZ(VSB_finish(p->cname));
@@ -188,6 +190,13 @@ vcc_EmitProc(struct vcc *tl, struct proc *p)
maskcmp = "&";
}
+ if ((mask & VCL_MET_TASK_H) && (mask & ~VCL_MET_TASK_H) == 0)
+ cc_adv = "v_dont_optimize ";
+ else
+ cc_adv = "";
+
+ nsub = tl->nsub++;
+
Fh(tl, 1, "vcl_func_f %s;\n", VSB_data(p->cname));
Fh(tl, 1, "const struct vcl_sub sub_%s[1] = {{\n",
VSB_data(p->cname));
@@ -196,14 +205,23 @@ vcc_EmitProc(struct vcc *tl, struct proc *p)
Fh(tl, 1, "\t.name\t\t= \"%.*s\",\n", PF(p->name));
Fh(tl, 1, "\t.vcl_conf\t= &VCL_conf,\n");
Fh(tl, 1, "\t.func\t\t= %s,\n", VSB_data(p->cname));
- Fh(tl, 1, "\t.n\t\t= %d,\n", tl->nsub++);
+ Fh(tl, 1, "\t.n\t\t= %d,\n", nsub);
Fh(tl, 1, "\t.nref\t\t= %d,\n", p->sym->nref);
Fh(tl, 1, "\t.called\t\t= %d\n", p->called);
Fh(tl, 1, "}};\n");
- Fc(tl, 1, "\nvoid %sv_matchproto_(vcl_func_f)\n",
- ((mask & VCL_MET_TASK_H) && (mask & ~VCL_MET_TASK_H) == 0) ?
- "v_dont_optimize " : "");
- Fc(tl, 1, "%s(VRT_CTX)\n{\n", VSB_data(p->cname));
+
+ if (dyn) {
+ Fc(tl, 1, "\nstatic inline void %s\n", cc_adv);
+ Fc(tl, 1, "%s_checked(VRT_CTX)\n{\n", VSB_data(p->cname));
+ } else {
+ Fc(tl, 1, "\nvoid %sv_matchproto_(vcl_func_f)\n", cc_adv);
+ Fc(tl, 1, "%s(VRT_CTX, enum vcl_func_call_e call,\n",
+ VSB_data(p->cname));
+ Fc(tl, 1, " enum vcl_func_fail_e *failp)\n{\n");
+ Fc(tl, 1, " assert(call == VSUB_STATIC);\n");
+ Fc(tl, 1, " assert(failp == NULL);\n");
+ }
+
vsbm = VSB_new_auto();
AN(vsbm);
vcc_vcl_met2c(vsbm, mask);
@@ -213,6 +231,36 @@ vcc_EmitProc(struct vcc *tl, struct proc *p)
Fc(tl, 1, "%s\n%s}\n", VSB_data(p->prologue), VSB_data(p->body));
VSB_destroy(&p->body);
VSB_destroy(&p->prologue);
+
+ if (! dyn) {
+ VSB_destroy(&p->cname);
+ return;
+ }
+
+ /* wrapper to call the actual (_checked) function */
+ Fc(tl, 1, "\nvoid v_matchproto_(vcl_func_f)\n");
+ Fc(tl, 1, "%s(VRT_CTX, enum vcl_func_call_e call,\n",
+ VSB_data(p->cname));
+ Fc(tl, 1, " enum vcl_func_fail_e *failp)\n{\n");
+ Fc(tl, 1, " enum vcl_func_fail_e fail;\n\n");
+ Fc(tl, 1, " fail = VPI_Call_Check(ctx, &VCL_conf, 0x%x, %d);\n",
+ mask, nsub);
+ Fc(tl, 1, " if (failp)\n");
+ Fc(tl, 1, " *failp = fail;\n");
+ Fc(tl, 1, " else if (fail == VSUB_E_METHOD)\n");
+ Fc(tl, 1, " VRT_fail(ctx, \"call to \\\"sub %.*s{}\\\""
+ " not allowed from here\");\n", PF(p->name));
+ Fc(tl, 1, " else if (fail == VSUB_E_RECURSE)\n");
+ Fc(tl, 1, " VRT_fail(ctx, \"Recursive call to "
+ "\\\"sub %.*s{}\\\"\");\n", PF(p->name));
+ Fc(tl, 1, " else\n");
+ Fc(tl, 1, " assert (fail == VSUB_E_OK);\n");
+ Fc(tl, 1, " if (fail != VSUB_E_OK || call == VSUB_CHECK)\n");
+ Fc(tl, 1, " return;\n");
+ Fc(tl, 1, " VPI_Call_Begin(ctx, %d);\n", nsub);
+ Fc(tl, 1, " %s_checked(ctx);\n", VSB_data(p->cname));
+ Fc(tl, 1, " VPI_Call_End(ctx, %d);\n", nsub);
+ Fc(tl, 1, "}\n");
VSB_destroy(&p->cname);
}
@@ -784,13 +832,13 @@ vcc_CompileSource(struct vcc *tl, struct source *sp, const char *jfile)
/* Tie vcl_init/fini in */
ifp = New_IniFin(tl);
- VSB_cat(ifp->ini, "\tVGC_function_vcl_init(ctx);\n");
+ VSB_cat(ifp->ini, "\tVGC_function_vcl_init(ctx, VSUB_STATIC, NULL);\n");
/*
* Because the failure could be half way into vcl_init{} so vcl_fini{}
* must always be called, also on failure.
*/
ifp->ignore_errors = 1;
- VSB_cat(ifp->fin, "\t\tVGC_function_vcl_fini(ctx);\n");
+ VSB_cat(ifp->fin, "\t\tVGC_function_vcl_fini(ctx, VSUB_STATIC, NULL);\n");
VSB_cat(ifp->fin, "\t\t\tVPI_vcl_fini(ctx);");
/* Emit method functions */
diff --git a/lib/libvcc/vmodtool.py b/lib/libvcc/vmodtool.py
index 8bc43a807..126fc8306 100755
--- a/lib/libvcc/vmodtool.py
+++ b/lib/libvcc/vmodtool.py
@@ -115,6 +115,7 @@ CTYPES = {
'STRANDS': "VCL_STRANDS",
'STRING': "VCL_STRING",
'STRING_LIST': "const char *, ...",
+ 'SUB': "VCL_SUB",
'TIME': "VCL_TIME",
'VOID': "VCL_VOID",
}
More information about the varnish-commit
mailing list