- 15 Feb, 2021 8 commits
-
-
Poul-Henning Kamp authored
-
Poul-Henning Kamp authored
-
Poul-Henning Kamp authored
-
Guillaume Quintard authored
-
Nils Goroll authored
that is, call vmod.returning_sub(); in VCL. Background: The existing call sub; is special in that it instantiates "sub", if it does not exist already, enabling use-before-definition semantics. The doctrine of this change is to preserve this behaviour and to not make existing, static calls any less efficient. Implementation: To support calling both literal SUBs as well as SUB expressions, we peek into the symbol table to check if the called expression is a function, based on the knowledge that, as of now, only functions can have a non-literal SUB return. We also know that no other expression can yield a SUB. So if peeking ahead tells us that a function follows, we call into expression evaluation. Otherwise, we expect a literal SUB as before. Null check: For dynamic SUB calls from vmods via VRT_call(), we require the SUB argument be non-NULL. But we can not for call: To signal error with a SUB return, a vmod function needs to be allowed to return NULL, otherwise it would need to have a dummy return value available for the VRT_fail() case. So we need to check for NULL in VGC. Alternatives considered: - We could make expression evaluation return SUB also for literal SUB symbols, turning all calls into sub->func(ctx, ...) C statements. I tried this path and it resulted in a change of behaviour with cc_command="clang -g -O2 ...": Where, previously, sub code was inlined, it would now generate an explicit C function call always, despite the fact that the C function is known at compile time (because all named VCL_SUB structs are). So this option was dismissed for violating the doctrine. - The null check could go to VPI, via a VPI_call(), basically identical to VRT_call(), but checking for NULL. This option was dismissed because it would foreclose the requirement to allow for SUB arguments in the future. Acknowledgements: reza mentioned the idea of call SUB at one point, and I can not remember if I had it before or not, so when in doubt, it was his idea. Dridi helped with ideas on the implementation and spotted a bug.
-
Dridi Boukelmoune authored
To control the threshold used to warn.
-
Dridi Boukelmoune authored
As a recurring pitfall sometimes hard to identify, it deserves its own notice. The accounting of variants reuses the worker's strangelove field since it doesn't conflict where other parts of the code base use it.
-
Poul-Henning Kamp authored
-
- 12 Feb, 2021 5 commits
-
-
Poul-Henning Kamp authored
-
Poul-Henning Kamp authored
and protect this pattern in the test-case.
-
Poul-Henning Kamp authored
-
Poul-Henning Kamp authored
-
Poul-Henning Kamp authored
a comment why it is not necessary. Only call vcl_hash{} if the initial hash is still intact when we get there.
-
- 11 Feb, 2021 5 commits
-
-
Poul-Henning Kamp authored
This lifts internal restrictions on the hashkey production, but does not change how things work from a VCL perspective.
-
Poul-Henning Kamp authored
-
Poul-Henning Kamp authored
-
Poul-Henning Kamp authored
-
Poul-Henning Kamp authored
expect resp.http.content-length -le 1024
-
- 10 Feb, 2021 5 commits
-
-
Dridi Boukelmoune authored
And use that for logging purposes when a successfully opened h2 session ends. RX_JUNK is still the default session close reason when existing reasons aren't accurate enough. Fixes #3393
-
Dridi Boukelmoune authored
In the same fashion as include/tbl/params.h for legibility.
-
Nils Goroll authored
with manual polish for cstyle and un-doing some return ((x)).
-
Nils Goroll authored
-
Nils Goroll authored
Either flexelint or me, probabbly the latter, do not understand the code. As far as I am concerned, I would have thought that flexelint could follow for AN(y); REPLACE(x, y); x being guranteed to be non-NULL This reverts commit 897e5313. This reverts commit 6f4d0ec1.
-
- 09 Feb, 2021 9 commits
-
-
Nils Goroll authored
-
Nils Goroll authored
-
Nils Goroll authored
to slim down one of my PRs with a trivial addition which, I hope, will not be controversial.
-
Nils Goroll authored
-
Nils Goroll authored
-
Nils Goroll authored
-
Nils Goroll authored
Missing include for REPLACE() / memset() added manually.
-
Nils Goroll authored
After edb0ca98, the ladybug understands our code better.
-
Nils Goroll authored
Move patches there which are not intended to be applied again, but which we used once and want to keep for reference. Also another coccinelle script which I found useful at one point.
-
- 08 Feb, 2021 8 commits
-
-
Nils Goroll authored
We cannot :ref: the rest of the online documentation from here
-
Nils Goroll authored
... regarding multiple VRT_call()s from vmods. Dridi noticed that we need to consider the handling for VRT_call(), thank you!
-
Nils Goroll authored
Based on feedback from Dridi
-
Nils Goroll authored
-
Nils Goroll authored
VMODs must not use SUB references across VCLs. This test checks for a panic if they do.
-
Nils Goroll authored
This test case checks dynamic calls, and, in particular, the runtime context checks and recursion detection. Recursions have to be caught at the first instance. Also test interop with the PRIV_TOP compile time restriction Ref #3498 Ref 6d49b18f The test has been manually annotated with information regarding the results expted from VCCs call tree walk. Explanation: .called == call, but every walk counts (for instance, barbarbar can be reached twice, via bar and barbar) .nref == sum(refs): Each VCL ref gains a ref, and each call from a sub which is itself dynamic (has nref > called).
-
Nils Goroll authored
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.
-
Nils Goroll authored
This implements the first bit of phks suggestion in https://github.com/varnishcache/varnish-cache/pull/3163#issuecomment-769718143 These VPI functions implement the operations on the recursion check bitmask for dynamic SUB calls. Strictly speaking, the VPI subs would not need to be concerned with the methods bitmask check and the assertion that calls originate from the right VCL, but we centralise these aspects for simplicity. We also add an enum to denote check failures.
-