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

[Ian Romanick]

On 01/07/2017 06:30 PM, Vladislav Egorov wrote:
> 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.

There are a few of the patches that seem very, very unlikely to cause
changes.  I'd strongly recommend trying a branch with, for example, just
patches 2, 3 (maybe), 6, and 7.  Landing some of the code before the
rest would allow progress while the rest of the kinds are worked out.

> 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
>>
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>