[master] ae87e52f9 vcc: check for impossible subs at compile time

[Nils Goroll]

commit ae87e52f926b8248622e72851573129ccc3ef605
Author: Nils Goroll <nils.goroll at uplex.de>
Date:   Sat Feb 6 14:25:37 2021 +0100

    vcc: check for impossible subs at compile time
    
    We call those subs impossible which use a combination of variables not
    being available together from any of the built-in subroutines (aka
    methods), for example::
    
            sub foo {
                    set req.http.impossible = beresp.reason;
            }
    
    For this example, there is no built-in subroutine for which both req
    and beresp were available, so it can not possibly be valid in any
    context.
    
    As long as subs were actually called directly or indirectly from a
    built-in sub, VCC did already detect the problem. For the above
    example, if `sub vcl_recv { call foo; }` existed, VCC would complain
    about beresp.reason not being accessible from vcl_recv.
    
    Except, when vcc_err_unref was disabled, the example would pass
    without error, and likewise it will with calls which we are about to
    add.
    
    We now add a check to detect impossible subroutines. It happens in a
    second walk over the VCL's SUB symbols in order to keep the more
    precise error messages for calls rooting in one of the built-in subs.
    
    This change also changes the order of error reporting slightly and
    incorporates, indirectly, the vcc_CheckUses() check moved in
    ed36b6389f9dbf6962e726f0df34d59132d9241a.
    
    v00020.vtc now has the "Impossible Subroutine with vcc_err_unref off"
    check, while the newly added m00053.vtc has been changed to test
    failure of the impossible sub at compile time rather than at runtime.
    
    This change was triggered by feedback from Dridi

diff --git a/bin/varnishtest/tests/v00020.vtc b/bin/varnishtest/tests/v00020.vtc
index 068135836..c9d41647b 100644
--- a/bin/varnishtest/tests/v00020.vtc
+++ b/bin/varnishtest/tests/v00020.vtc
@@ -246,6 +246,17 @@ varnish v1 -errvcl {Not available in subroutine 'vcl_recv'.} {
 	}
 }
 
+varnish v1 -cliok "param.set vcc_err_unref off"
+
+varnish v1 -errvcl {Impossible Subroutine} {
+	backend b { .host = "127.0.0.1"; }
+	sub foo {
+		set req.http.foo = 100 + beresp.status;
+	}
+}
+
+varnish v1 -cliok "param.set vcc_err_unref on"
+
 varnish v1 -errvcl {
 ('<vcl.inline>' Line 4 Pos 44) -- (Pos 49)
         sub foo { set req.http.foo = 100 + beresp.status; }
diff --git a/lib/libvcc/vcc_xref.c b/lib/libvcc/vcc_xref.c
index 091874895..c2e63ed76 100644
--- a/lib/libvcc/vcc_xref.c
+++ b/lib/libvcc/vcc_xref.c
@@ -140,10 +140,6 @@ vcc_AddUses(struct vcc *tl, const struct token *t1, const struct token *t2,
 		WRONG("wrong xref use");
 
 	VTAILQ_INSERT_TAIL(&tl->curproc->uses, pu, list);
-
-	if (pu->mask == 0)
-		if (vcc_CheckUses(tl))
-			AN(tl->err);
 }
 
 void
@@ -355,11 +351,34 @@ vcc_checkuses(struct vcc *tl, const struct symbol *sym)
 	}
 }
 
+/*
+ * Used from a second symbol walk because vcc_checkuses is more precise for
+ * subroutines called from methods. We catch here subs used for dynamic calls
+ * and with vcc_err_unref = off
+ */
+static void
+vcc_checkpossible(struct vcc *tl, const struct symbol *sym)
+{
+	struct proc *p;
+
+	p = sym->proc;
+	AN(p);
+
+	if (p->okmask != 0)
+		return;
+
+	VSB_cat(tl->sb, "Impossible Subroutine");
+	vcc_ErrWhere(tl, p->name);
+}
+
 int
 vcc_CheckUses(struct vcc *tl)
 {
 
 	VCC_WalkSymbols(tl, vcc_checkuses, SYM_MAIN, SYM_SUB);
+	if (tl->err)
+		return (tl->err);
+	VCC_WalkSymbols(tl, vcc_checkpossible, SYM_MAIN, SYM_SUB);
 	return (tl->err);
 }