[mapserver-dev] App crash in mapparser.y

Tue Feb 22 15:22:01 PST 2022

If there is no layer named "Height of Buildings (2013)" then lexer would
see that as an attribute binding and with no quotes around it (the outer
"'s are stripped off beforehand) it would be interpreted as a numeric
binding. I guess the initial questions I have are:

   1. is there a layer with that name in the mapfile.
   2. if yes, does that layer also have the same LABELREQUIRES attribute? I
   believe the current layer name is skipped with processing its own
   LABELREQUIRES expression.

Can you share the mapfile privately?

On Tue, Feb 22, 2022 at 2:22 PM Tamas Szekeres <szekerest at gmail.com> wrote:

> Hi Steve,
>
> I've got the corresponding mapfile causing the crash, it looks like each
> LABELREQUIRES expression has a corresponding layer with the same name. The
> only thing I see is that some of the expressions contain brackets (either
> in the expression and in the layer name) like so:
>
> *LABELREQUIRES "[Height of Buildings (2013)]"*
>
> Is that something that is considered as an attribute binding by the lexer?
>
> The crash location is looking like:
>
>
>
>
>
> *case MS_TOKEN_BINDING_DOUBLE:  case MS_TOKEN_BINDING_INTEGER:    token =
> NUMBER;    (*lvalp).dblval =
> atof(p->shape->values[p->expr->curtoken->tokenval.bindval.index]);
> break;*
>
> However the p->shape has been initialized to NULL (in msEvalContext),
> definitely.
>
> Thanks,
>
> Tamas
>
>
> Steve Lime <sdlime at gmail.com> ezt írta (időpont: 2022. febr. 22., K,
> 15:21):
>
>> Hi Tamas: Ok, reacquainting myself with that code. There are actually two
>> lexers in play - msyylex() is the main one that is used to parse a mapfile
>> and also expression strings - it is not thread-safe. With regards to
>> expressions, msyylex() is used to generate a series of tokens - see
>> msTokenizeExpression() in maplayer.c. There is a lock present when
>> msyylex() is used as part of that function. The Bison parser has its own
>> lexer (yylex(), defined in mapparser.y) that is used to traverse the series
>> of tokens. That function (and yyparse()) operate on a parseObj that fully
>> encapsulates expression evaluation. So the parser should be reentrant...
>>
>> Assuming that LABELREQUIRES is in play here, the simplest explanation is
>> that its value contains a bad layer name in the expression. That would
>> leave something like "[badlayer]" remaining in the string passed to
>> msTokenizeString() and would result in the error you noted originally. We
>> could/should add a check in msEvalContext() to examine the expression
>> string to catch this condition. Using strchr to check for remaining '[' or
>> ']' characters would probably do it.
>>
>> --Steve
>>
>> On Sat, Feb 19, 2022 at 12:19 PM Tamas Szekeres <szekerest at gmail.com>
>> wrote:
>>
>>> Hi Steve,
>>>
>>> According to the call stack below, yyparse also calling yylex
>>> internally, which is not considered to be reentrant I think.
>>>
>>> *mapserver!yylex+0x1b5 [e:\builds-t1\src\mapserver-intramaps\vc15x64\mapparser.y @ 858]
>>> mapserver!yyparse+0x247 [e:\home\even\mapserver\mapserver\mapparser.c @ 1461]
>>> mapserver!msEvalContext+0x178 [e:\builds-t1\src\mapserver-intramaps\maputil.c @ 442]*
>>>
>>>
>>> Best regards,
>>>
>>> Tamas
>>>
>>>
>>> Steve Lime <sdlime at gmail.com> ezt írta (időpont: 2022. febr. 19., Szo,
>>> 17:53):
>>>
>>>> I thought generating tokens for an expression needed a lock since it
>>>> uses the lexer but that the parser did not. I could be wrong though.
>>>>
>>>> On Sat, Feb 19, 2022 at 10:21 AM Steve Lime <sdlime at gmail.com> wrote:
>>>>
>>>>> I don’t remember offhand and will have to check. That said if there is
>>>>> a attribute level token in a layer level expression than that would explain
>>>>> your research. Is there really no way to get info on the expression(s) in
>>>>> question?
>>>>>
>>>>> On Sat, Feb 19, 2022 at 9:22 AM Tamas Szekeres <szekerest at gmail.com>
>>>>> wrote:
>>>>>
>>>>>> I'm not too familiar in the current parser/lexer implementation, but
>>>>>> is msTokenizeExpression and yyparse supposed to be protected by the same
>>>>>> parser lock in msEvalContext? Or if  msTokenizeExpression doesn't
>>>>>> leave any global variables behind, should yyparse be protected by it's own?
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Tamas
>>>>>>
>>>>>>
>>>>>> Steve Lime <sdlime at gmail.com> ezt írta (időpont: 2022. febr. 19.,
>>>>>> Szo, 16:02):
>>>>>>
>>>>>>> Requires and labelrequires aren’t operating at the feature level,
>>>>>>> rather the layer level so they definitely don’t support attribute binding.
>>>>>>> We’d need to whip up a test case to figure out a method to trap this. The
>>>>>>> workaround is to fix the expression. I mean, obviously we don’t want to
>>>>>>> crash but it’s never going to work if written as you suspect.
>>>>>>>
>>>>>>> On Sat, Feb 19, 2022 at 7:55 AM Tamas Szekeres <szekerest at gmail.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Hi Devs,
>>>>>>>>
>>>>>>>> I got a crash log which shows that the access violation is
>>>>>>>> happening in the following location:
>>>>>>>>
>>>>>>>> *mapserver!yylex+0x1b5 [e:\builds-t1\src\mapserver-intramaps\vc15x64\mapparser.y @ 858]
>>>>>>>> 		mapserver!yyparse+0x247 [e:\home\even\mapserver\mapserver\mapparser.c @ 1461]
>>>>>>>> 		mapserver!msEvalContext+0x178 [e:\builds-t1\src\mapserver-intramaps\maputil.c @ 442]
>>>>>>>> 		mapserver!msDrawVectorLayer+0xb0 [e:\builds-t1\src\mapserver-intramaps\mapdraw.c @ 918]
>>>>>>>> 		mapserver!msDrawLayer+0x3a3 [e:\builds-t1\src\mapserver-intramaps\mapdraw.c @ 813]
>>>>>>>> 		mapserver!msDrawMap+0x415 [e:\builds-t1\src\mapserver-intramaps\mapdraw.c @ 403]
>>>>>>>> 		mapscript!CSharp_mapObj_draw+0xd*
>>>>>>>>
>>>>>>>> By looking into the corresponding code, the problem has happened in
>>>>>>>> the following location in yylex():
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> * case MS_TOKEN_BINDING_INTEGER:    token = NUMBER; ---->   (*lvalp).dblval = atof(p->shape->values[p->expr->curtoken->tokenval.bindval.index]);    break;*
>>>>>>>>
>>>>>>>> In maputil.c the call stack location is here (msEvalContext):
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> *msTokenizeExpression(&e, NULL, NULL);  p.shape = NULL;  p.expr = &e;  p.expr->curtoken = p.expr->tokens; /* reset */  p.type = MS_PARSE_TYPE_BOOLEAN;----->  status = yyparse(&p);  msFreeExpression(&e);*
>>>>>>>>
>>>>>>>> And in mapdraw.c
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> * /* TODO TBT: draw as raster layer in vector renderers */----->  annotate = msEvalContext(map, layer, layer->labelrequires);  if(map->scaledenom > 0) {    if((layer->labelmaxscaledenom != -1) && (map->scaledenom >= layer->labelmaxscaledenom)) annotate = MS_FALSE;    if((layer->labelminscaledenom != -1) && (map->scaledenom < layer->labelminscaledenom)) annotate = MS_FALSE;  }*
>>>>>>>>
>>>>>>>> I'm not sure if the LABELREQUIRES option supports attribute binding
>>>>>>>> (probably not) and in that case the
>>>>>>>> p->expr->curtoken->tokenval.bindval.index would probably have undefined
>>>>>>>> value. I don't have any information about the corresponding LABELREQUIRES
>>>>>>>> expression , but if I assume it doesn't contain attribute binding, then
>>>>>>>> this crash might probably happen due to a missing parser lock around *yyparse(&p).
>>>>>>>> *What do you think about the problem and how could we work around
>>>>>>>> this?
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> Tamas
>>>>>>>>
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> MapServer-dev mailing list
>>>>>>>> MapServer-dev at lists.osgeo.org
>>>>>>>> https://lists.osgeo.org/mailman/listinfo/mapserver-dev
>>>>>>>>
>>>>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/mapserver-dev/attachments/20220222/de8fd802/attachment-0001.html>