On 17.01.2011, at 15:18, Davi Arnaut wrote:
> Hi Dmitry,
> On 11/24/10 7:07 AM, Dmitry Shulga wrote:
>> #At file:///Users/shulga/projects/mysql/5.1-bugteam-bug58026/ based on
>> 3514 Dmitry Shulga 2010-11-24
>> Fixed bug#58026 - massive recursion and crash in regular expression
> Looks good so far, a few comments below.
>> The problem was that parsing of nested regular expression involved
>> recursive calls. Such recursion was not take care of available memory,
> "Such recursion was not take care of available memory," sounds strange.
> I suggest rewriting to:
> "Such recursion didn't take into account the amount of available stack space, which
> ended up leading to stack overflow crashes."
>> which may lead to stack overflow crashes.
>> @ client/mysqltest.cc
>> Passed NULL pointer to function as last actual argument in call
>> to my_regcomp. In this case check for enough memory in stack
>> doesn't occur. Such checking doesn't need in mysql clients.
> "Such checking is not necessary in mysql clients"
> Please introduce a typedef for the stack check function. Suggestion:
> typedef int (*my_regex_stack_check_t)();
> To be added to my_regex.h
>> +extern "C"
> Hum, since the hook is global, there isn't much reason to pass the stack check
> function each time we need to compile a expression.
> Just add the hook to regcomp.c with internal linkage:
> static my_regex_stack_check_t stack_check;
> and we let a hook be registered at my_regex_init time:
> my_regex_init(CHARSET_INFO *cs, my_regex_stack_check_t stack_check);
> In this case, my_regex_init sets the hook to the given one.
I don't agree with your suggestion.
First, because solution with explicit argument is more meaningful. With your approach
usage of check_stack_overrun function is implicit and
hidden inside body of my_regex_init/my_regcomp. In my approach check_stack_overrun passed
explicitly and developer reading this source code
clearly understand what I do there.
Second, because current_thd is not defined for some client programs, mysqltest for
>> +#ifndef EMBEDDED_LIBRARY // Avoid compiler warning
>> + uchar stack_top;
>> + return check_stack_overrun(current_thd, STACK_MIN_SIZE,
> This won't compile with embedded? stack_top won't have been declared.
As for the embedded version of mysql the function check_stack_overrun() set to nothing
without this #ifdef we got next error during compilation:
cc1plus: warnings being treated as errors
item_cmpfunc.cc: In function ‘int check_enough_stack_size()’:
item_cmpfunc.cc:4799: warning: unused variable ‘stack_top’