Interesting behavior with PCRE2 JIT

[Ervin Hegedüs]

Hi,

here is a sample code:

==%==

#include <stdio.h>
#include <string.h>

#define PCRE2_CODE_UNIT_WIDTH 8
#include <pcre2.h>

int search(pcre2_code *re, unsigned char * subject) {

    pcre2_match_data *match_data_real = pcre2_match_data_create_from_pattern(re, NULL);

    size_t len_subject = strlen((const char *)subject);

    int rc = pcre2_match(
        re,
        (PCRE2_SPTR)subject,
        len_subject,
        0,
        0,
        match_data_real,
        NULL
    );

    pcre2_match_data_free(match_data_real);
    return rc;
}

int main(int argc, char ** argv) {

    unsigned char subject[][100]     = {
        "this is a foobar",
        "this is a barfoo",
        "this is a barbar",
        "this is a foofoo"
    };

    pcre2_code *re;
    PCRE2_SPTR  pattern = (unsigned char *)"foo";
    int         errornumber;
    PCRE2_SIZE  erroroffset;

    re = pcre2_compile(
        pattern,
        PCRE2_ZERO_TERMINATED,
        0,
        &errornumber,
        &erroroffset,
        NULL
    );
    pcre2_jit_compile(re, PCRE2_JIT_COMPLETE);

    FILE *fp;

    int s = 0;
    while(s < 2) {
        search(re, subject[s++]);
    }

    if (argc >= 2) {
        fp = fopen(argv[1], "r");
        if (fp != NULL) {
            char tline[2048];
            while(fgets(tline, 2048, fp) != NULL) {
                search(re, (unsigned char *)tline);
            }
            fclose(fp);
        }
    }

    pcre2_code_free(re);

    return 0;
}

==%==

I would like to read a file and find patterns in lines. This code contains few hard-coded lines, but if I pass an argument to the compiled program, it tries to open it as a file, and reads that too (see line 58). Also I'd like to use JIT (line 49).

Compile the code:

gcc -Wall -O2 -g minimal.c -o minimal -lpcre2-8

The behaviors when I run the code through valgrind:

* if I run the program without argument, Valgrind does not report any issue

* if I run the program with an argument, Valgrind gives the report below

* if I remove/comment the pcre2_jit_compile() in line 49, and run WITH argument, Valgrind does not report any issue again

The commands what I use:

valgrind --tool=memcheck --leak-check=full --show-leak-kinds=all --track-origins=yes -s ./minimal

valgrind --tool=memcheck --leak-check=full --show-leak-kinds=all --track-origins=yes -s ./minimal mytext.txt

The report when I run with argument (code opens the file) and use JIT:

==%==

==31385== Conditional jump or move depends on uninitialised value(s)
==31385==    at 0x4EECD1A: ???
==31385==    by 0x1FFEFFFC1F: ???
==31385==  Uninitialised value was created by a stack allocation
==31385==    at 0x1090FA: main (minimal.c:27)
==31385==
==31385==
==31385== HEAP SUMMARY:
==31385==     in use at exit: 0 bytes in 0 blocks
==31385==   total heap usage: 12 allocs, 12 frees, 13,486 bytes allocated
==31385==
==31385== All heap blocks were freed -- no leaks are possible
==31385==
==31385== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
==31385==
==31385== 1 errors in context 1 of 1:
==31385== Conditional jump or move depends on uninitialised value(s)
==31385==    at 0x4EECD1A: ???
==31385==    by 0x1FFEFFFC1F: ???
==31385==  Uninitialised value was created by a stack allocation
==31385==    at 0x1090FA: main (minimal.c:27)
==31385==
==31385== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

==%==

There is the `int main()` in line 27...

What do I forget to receive this message?

Thanks,

a.

[Philip Hazel]

Using valgrind with JIT usually shows up some false positives. The main test suite for PCRE2 uses a valgrind suppressions file that contains this:

{
   name
   Memcheck:Addr16
   obj:???
   obj:???
   obj:???
}

{
   name
   Memcheck:Cond
   obj:???
   obj:???
   obj:???

}

The comment in the RunTest file says this:

# When JIT is used with valgrind, we need to set up valgrind suppressions as
# otherwise there are a lot of false positive valgrind reports when the
# the hardware supports SSE2. 

Regards,

Philip

--
You received this message because you are subscribed to the Google Groups "PCRE2 discussion list" group.
To unsubscribe from this group and stop receiving emails from it, send an email to pcre2-dev+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/pcre2-dev/CAJ2uXbcC2E523ZLhZEzNfrq9HiBBEC9dCtSd87M6ynwjkbnJhQ%40mail.gmail.com.

[Ervin Hegedüs]

Hi Philip,


thank you so much for your answer,

On Tue, Dec 13, 2022 at 04:21:13PM +0000, Philip Hazel wrote:
> Using valgrind with JIT usually shows up some false positives. The main
> test suite for PCRE2 uses a valgrind suppressions file that contains this:
>
> {
> name
> Memcheck:Addr16
> obj:???
> obj:???
> obj:???
> }
>
> {
> name
> Memcheck:Cond
> obj:???
> obj:???
> obj:???
> }

thanks, I tried to use this supression, and it worked:

{
name
Memcheck:Cond
obj:*
obj:*

}

> The comment in the RunTest file says this:
>
> # When JIT is used with valgrind, we need to set up valgrind suppressions as
> # otherwise there are a lot of false positive valgrind reports when the
> # the hardware supports SSE2.

yes, my HW supports SSE2.

But is there any explanation why this error occurs only when I
read from a file?


Thanks,



a.

[Philip Hazel]

No, that is a mystery. Probably something to do with memory allocation - reading from a file presumably changes something. I reproduced your issue before I remembered about the valgrind suppressions, and while doing that I discovered that the error occurs only when using a shared PCRE2 library - not with a static library, which is another mystery. I don't know internal JIT details, but it is creating executable code in data memory, so it's all related to that.

Regards,

Philip

[Ervin Hegedüs]

Hi Philip,

thanks again. Meanwhile I asked this question on SO too:

https://stackoverflow.com/questions/74777619/valgrind-conditional-jump-error-with-pcre2-jit-when-reading-from-file

On Tue, Dec 13, 2022 at 05:00:40PM +0000, Philip Hazel wrote:
> No, that is a mystery. Probably something to do with memory allocation -
> reading from a file presumably changes something. I reproduced your issue
> before I remembered about the valgrind suppressions, and while doing that I
> discovered that the error occurs only when using a shared PCRE2 library -
> not with a static library, which is another mystery. I don't know internal
> JIT details, but it is creating executable code in data memory, so it's all
> related to that.

as John Bollinger suggested, I initialized the

char tline[2048] = {0};

and this solved the problem. All Valgrind issue has gone without
any supression.

It's very odd.


Anyway, many thanks for your time.



Regards,


a.

[enh]

it's not _very_ odd...

1. most of these kind of simd/vector optimizations work by reading more data in each chunk.

2. this makes them prone to over-reading (past the end of the actually valid data).

3. that upsets tools like valgrind if they see you read bytes that haven't been written.

imagine you're implementing strlen(), say. you can either safely (and slowly) read a byte at a time, checking for '\0', and be certain you never read past the end. or you can try reading chunks of 4 or 8 or 16 bytes at a time, and look for a '\0' in there, and "ignore" the rest. but you've already read it in, and a memory error tool with byte (or at least "smaller than your chunk size") granularity will notice that.

the workaround you have here silences valgrind by ensuring that there are no bytes read by the optimized code that haven't already been written (by the `= {}`). (remember that without the `= {}` that allocation is just a subtraction of the stack pointer, so you're getting whatever uninitialized memory happens to be there.)

 

Anyway, many thanks for your time.



Regards,


a.

--
You received this message because you are subscribed to the Google Groups "PCRE2 discussion list" group.
To unsubscribe from this group and stop receiving emails from it, send an email to pcre2-dev+...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/pcre2-dev/20221213171914.w7ajso3mthk4w7gk%40arxnet.hu.

[Ervin Hegedüs]

hi,

On Tue, Dec 13, 2022 at 09:28:08AM -0800, enh wrote:
> On Tue, Dec 13, 2022 at 9:19 AM Ervin Hegedüs <air...@gmail.com> wrote:

> > as John Bollinger suggested, I initialized the
> >
> > char tline[2048] = {0};
> >
> > and this solved the problem. All Valgrind issue has gone without
> > any supression.
> >
> > It's very odd.
> >
>
> it's not _very_ odd...
>
> 1. most of these kind of simd/vector optimizations work by reading more
> data in each chunk.
> 2. this makes them prone to over-reading (past the end of the actually
> valid data).
> 3. that upsets tools like valgrind if they see you read bytes that haven't
> been written.

thanks for explanation,

> imagine you're implementing strlen(), say. you can either safely (and
> slowly) read a byte at a time, checking for '\0', and be certain you never
> read past the end. or you can try reading chunks of 4 or 8 or 16 bytes at a
> time, and look for a '\0' in there, and "ignore" the rest. but you've
> already read it in, and a memory error tool with byte (or at least "smaller
> than your chunk size") granularity will notice that.
>
> the workaround you have here silences valgrind by ensuring that there are
> no bytes read by the optimized code that haven't already been written (by
> the `= {}`). (remember that without the `= {}` that allocation is just a
> subtraction of the stack pointer, so you're getting whatever uninitialized
> memory happens to be there.)

yes, this makes sense, but it's odd to me that this behavior
comes only when I set up JIT.

Nevermind, it's better for me than suppress any warning/error, so
I can live with it :).


Thanks again.


a.

[Jeffrey Walton]

You should use -O0 or -O1; not -O2 or above. See below.

From the Valgrind Guide at https://valgrind.org/docs/manual/quick-start.html :

Compile your program with -g to include debugging
information so that Memchecks error messages include exact
line numbers. Using -O0 is also a good idea, if you can
tolerate the slowdown. With -O1 line numbers in error
messages can be inaccurate, although generally speaking
running Memcheck on code compiled at -O1 works fairly well,
and the speed improvement compared to running -O0 is quite
significant. Use of -O2 and above is not recommended as
Memcheck occasionally reports uninitialised-value errors
which dont really exist.

Jeff

[enh]

not particularly. the JIT explicitly does this kind of optimization. i'm guessing the regular C code doesn't.

in fact, if you do a web search for "pcre jit sse2", the first match is https://lists.exim.org/lurker/message/20150824.065129.dfdeb079.pt-BR.html which talks about exactly this.

[Ervin Hegedüs]

hi,

On Tue, Dec 13, 2022 at 10:31:09AM -0800, enh wrote:
> On Tue, Dec 13, 2022 at 9:40 AM Ervin Hegedüs <air...@gmail.com> wrote:
> >
> > yes, this makes sense, but it's odd to me that this behavior
> > comes only when I set up JIT.
> >
>
> not particularly. the JIT explicitly does this kind of optimization. i'm
> guessing the regular C code doesn't.

yepp,

> in fact, if you do a web search for "pcre jit sse2", the first match is
> https://lists.exim.org/lurker/message/20150824.065129.dfdeb079.pt-BR.html
> which talks about exactly this.

ah, thanks.



a.

[Ervin Hegedüs]

Hi Jeff,

On Tue, Dec 13, 2022 at 01:28:09PM -0500, Jeffrey Walton wrote:
> >> Compile the code:
> >> gcc -Wall -O2 -g minimal.c -o minimal -lpcre2-8
>
> You should use -O0 or -O1; not -O2 or above. See below.

thanks,

> From the Valgrind Guide at https://valgrind.org/docs/manual/quick-start.html :
>
> Compile your program with -g to include debugging
> information so that Memchecks error messages include exact
> line numbers. Using -O0 is also a good idea, if you can
> tolerate the slowdown. With -O1 line numbers in error
> messages can be inaccurate, although generally speaking
> running Memcheck on code compiled at -O1 works fairly well,
> and the speed improvement compared to running -O0 is quite
> significant. Use of -O2 and above is not recommended as
> Memcheck occasionally reports uninitialised-value errors
> which dont really exist.

while I develop the code, I think it's better to see each
notification (even more warnings and errors).

But thank you for pointig that.


a.