[Mesa-dev] [PATCH 08/12] glcpp: Use strpbrk in the line continuations pass

[Vladislav Egorov]

Well, running radamsa fuzzer right now and it's able to find quite a bit 
of various problems (difference in output/error reporting against 
master) in the hand-written parser without any effort. Some cases should 
be added to the test-suite maybe.

Clearly the patch series requires more work, sadly. Will try other 
fuzzers as well.


08.01.2017 00:56, Vladislav Egorov пишет:
> Oh. Thanks! This is embarrassing. I have no idea how did I manage to 
> miss it. Line continuations are so rare that only two games in my 
> shader-db use them -- Talos Principle and Serious Sam (both use 
> Serious Engine). And neither of them hit this condition. I probably 
> need to fuzz this thing or something.
>
> I'll send fixup.
>
> 07.01.2017 22:53, Gustaw Smolarczyk пишет:
>> 7 sty 2017 20:04 "Vladislav Egorov" <vegorov180 at gmail.com> napisał(a):
>>
>>     To find newlines the line continuations removal pass uses strchr()
>>     twice -- first time to find '\n', second time to find '\r'. Now,
>>     if the shader uses Unix line separators '\n', the file could contain
>>     no '\r' at all, so each time it will scan to the end of the shader.
>>     Use strpbrk() standard function instead that was specifically created
>>     to search many characters at once.
>>     ---
>>      src/compiler/glsl/glcpp/pp.c | 28 ++++++++--------------------
>>      1 file changed, 8 insertions(+), 20 deletions(-)
>>
>>     diff --git a/src/compiler/glsl/glcpp/pp.c
>>     b/src/compiler/glsl/glcpp/pp.c
>>     index c196868..c4c0196 100644
>>     --- a/src/compiler/glsl/glcpp/pp.c
>>     +++ b/src/compiler/glsl/glcpp/pp.c
>>     @@ -231,7 +231,6 @@ remove_line_continuations(glcpp_parser_t
>>     *ctx, const char *shader)
>>      {
>>             struct string_buffer sb;
>>             const char *backslash, *newline, *search_start;
>>     -        const char *cr, *lf;
>>              char newline_separator[3];
>>             int collapsed_newlines = 0;
>>             int separator_len;
>>     @@ -266,23 +265,19 @@ remove_line_continuations(glcpp_parser_t
>>     *ctx, const char *shader)
>>              * examining the first encountered newline terminator,
>>     and using the
>>              * same terminator for any newlines we insert.
>>              */
>>     -       cr = strchr(search_start, '\r');
>>     -       lf = strchr(search_start, '\n');
>>     +       newline = strpbrk(search_start, "\r\n");
>>
>>             newline_separator[0] = '\n';
>>             newline_separator[1] = '\0';
>>             newline_separator[2] = '\0';
>>
>>     -       if (cr == NULL) {
>>     +       if (newline == NULL) {
>>                     /* Nothing to do. */
>>     -       } else if (lf == NULL) {
>>     -               newline_separator[0] = '\r';
>>     -       } else if (lf == cr + 1) {
>>     -               newline_separator[0] = '\r';
>>     -               newline_separator[1] = '\n';
>>     -       } else if (cr == lf + 1) {
>>     -               newline_separator[0] = '\n';
>>     -               newline_separator[1] = '\r';
>>     +       } else if (newline[1] == '\n' || newline[1] == '\r') {
>>
>> What if we have \n\n ? Won't it match it as a single newline?
>>
>> Regards,
>> Gustaw
>