Re: [Fwd: Gnuplot: MAX_NUM_VAR in src/syscfg.h]

[Ethan A M. <merritt@u.washington.edu>]

On Saturday 25 June 2005 06:50 am, you wrote:
>
> yes, it uses an eval stack. But it's a "pure" estack. When a udf
> function is invoked, the parameters are copied off the estack into
> some other storage. What I'm suggesting a small change to leave them
> on the estack, with a frame being pushed. This gets round the problem
> of having an upper limit number on the number of dummy variables,
> without having to set aside storage to copy parameters.
>
> Looking at the 4.0 sources, f_calln() in internal.c has to
>
>     for (i = 0; i < MAX_NUM_VAR; i++)
>         save_dummy[i] = udf->dummy_values[i];
>
> I'm proposing that we (er... someone !)  changes this so that,
> instead, f_calln() lays down some notion of frame linkage in the
> estack, so that it can find dummy parameters.

OK. I get it now, at least conceptually.
I think you are/were more familiar with the inner workings of the
evaluation code than I am.

It looks to me that there is also a less drastic, though less elegant,
fix. Instead of pre-reserving a fixed size array in the udft_entry
structure, we could allocate the space dynamically immediately prior
to the code you quote above.

--- f_calln.c.orig      2005-06-25 15:14:01.620104672 -0700
+++ f_calln.c   2005-06-25 15:16:07.559958904 -0700
@@ -13,12 +13,13 @@
     if (!udf->at) {            /* undefined */
        int_error(NO_CARET, "undefined function: %s", udf->udf_name);
     }
-    for (i = 0; i < MAX_NUM_VAR; i++)
-       save_dummy[i] = udf->dummy_values[i];
-    /* if there are more parameters than the function is expecting */
-    /* simply ignore the excess */
     (void) pop(&num_params);
+    num_pop = num_params.v.int_val;
+    udf->dummy_values = gp_alloc(sizeof(struct value) * num_pop);
+    for (i = 0; i < num_pop; i++)
+       save_dummy[i] = udf->dummy_values[i];



It also looks to me that there is an overflow flaw in the current
code.  It tries to "ignore the excess parameters" by popping them into
the dummy_values array first, and then overwriting them with the actual
parameters up to MAX_NUM_VAR.  If the number of parameters to be thrown
away itself exceeds MAX_NUM_VAR, it will end up trashing whatever
is next in the malloc heap.

internal.c 198:
    /* if there are more parameters than the function is expecting */
    /* simply ignore the excess */
    (void) pop(&num_params);
    if (num_params.v.int_val > MAX_NUM_VAR) {
        /* pop the dummies that there is no room for */
        num_pop = num_params.v.int_val - MAX_NUM_VAR;
        for (i = 0; i < num_pop; i++)
            (void) pop(&(udf->dummy_values[i]));
    }

Hmmm. Let's see...
gnuplot> f(x) = sin(x)
gnuplot> print f(pi,1,2)
1.0
gnuplot> print f(pi/2,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,
17,18,19,20,21,22,23,24,25,26,27,28,29,30)
gnuplot> Segmentation fault

So indeed there is room to improve this code.


-- 
Ethan A Merritt
Biomolecular Structure Center
University of Washington, Seattle 98195-7742