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" <vegorov...@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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to