Summary: | debug memory tracker shows memory overwrite in MAXNESTING | ||
---|---|---|---|
Product: | systemtap | Reporter: | David Smith <dsmith> |
Component: | translator | Assignee: | David Smith <dsmith> |
Status: | RESOLVED FIXED | ||
Severity: | normal | ||
Priority: | P2 | ||
Version: | unspecified | ||
Target Milestone: | --- | ||
Host: | Target: | ||
Build: | Last reconfirmed: |
Description
David Smith
2009-12-16 15:25:48 UTC
There is some subtlety to the way we allocate context locals, with [MAXNESTING+1] slots (to store outgoing arguments to a potentially excessively nested function). Maybe something there is behaving badly. See translate.cxx:1018 ish. Here's code output by the translator to set up the arguments to a function, then call it: ==== c->locals[c->nesting+1].function_recurse.n = l->__tmp0; function_recurse (c); ==== One of the first things a function does is check MAXNESTING: ==== static void function_recurse (struct context* __restrict__ c) { struct function_recurse_locals * __restrict__ l = & c->locals[c->nesting+1].function_recurse; (void) l; #define CONTEXT c #define THIS l if (0) goto out; c->last_stmt = "identifier 'recurse' at ../src/testsuite/systemtap.base/control_limits.stp:3:10"; if (unlikely (c->nesting+1 > MAXNESTING)) { c->last_error = "MAXNESTING exceeded"; return; } else { c->nesting ++; } ==== But, this check is too late - setting up the arguments has already written past the end of the context structure. Hmm, strangely enough, we try to handle this by allocating 1 extra set of locals: o->newline(-1) << "} locals [MAXNESTING+1];"; // NB: The +1 above for extra room for outgoing arguments of next nested function. // If MAXNESTING is set too small, the args will be written, but the MAXNESTING // check done at c_unparser::emit_function will reject. // // This policy wastes memory (one row of locals[] that cannot really // be used), but trades that for smaller code (not having to check // c->nesting against MAXNESTING at every call site). Fixed in commit 101a2bb. The test: if (unlikely (c->nesting+1 > MAXNESTING)) { was wrong. It should have been: if (unlikely (c->nesting+1 >= MAXNESTING)) { The original code actually allowed MAXNESTING+1 recursion levels. Unfortunately, I don't see a good way of adding a testsuite test for this problem, since the -DDEBUG_MEM logic just prints to the console. |