Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Valgrind errors on amd64 #654

Open
paulfloyd opened this issue Jan 3, 2025 · 6 comments
Open

Valgrind errors on amd64 #654

paulfloyd opened this issue Jan 3, 2025 · 6 comments
Labels
JIT Relating to the JIT feature question Question from a user
Milestone

Comments

@paulfloyd
Copy link

paulfloyd commented Jan 3, 2025

I recently tried running a Qt app (Qt Creator) under Valgrind and I got tons of errors.

Here is a small reproducer

#include <QDir>
#include <QString>
#include <QStringList>
#include <iostream>

using namespace Qt::Literals::StringLiterals;

int main()
{
    QString fontpath = "/usr/local/share/qtcreator/fonts";
    QDir dir(fontpath);

    static const QString nameFilters[] = {
        u"*.ttf"_s,
        u"*.pfa"_s,
        u"*.pfb"_s,
        u"*.otf"_s,
    };

    const auto fis = dir.entryInfoList(QStringList::fromReadOnlyData(nameFilters), QDir::Files);
    for (const QFileInfo &fi : fis) {
        const QByteArray file = QFile::encodeName(fi.absoluteFilePath());
        std::cout << "foo " << file.toStdString() << std::endl;
    }
}

(needs Qt6Core and Qt Creator, you may need to change 'fontpath' to match your install path).

This gives

==8453== Memcheck, a memory error detector
==8453== Copyright (C) 2002-2024, and GNU GPL'd, by Julian Seward et al.
==8453== Using Valgrind-3.25.0.GIT and LibVEX; rerun with -h for copyright info
==8453== Command: ././build/Desktop-Debug/bug496972
==8453== 
==8453== Conditional jump or move depends on uninitialised value(s)
==8453==    at 0x6B40CA1: ???
==8453==    by 0x67379AF: ???
==8453== 
==8453== Conditional jump or move depends on uninitialised value(s)
==8453==    at 0x6B3FF41: ???
==8453==    by 0x67379AF: ???
==8453== 
==8453== Conditional jump or move depends on uninitialised value(s)
==8453==    at 0x6B3F1E1: ???
==8453==    by 0x67379AF: ???
==8453== 
==8453== Conditional jump or move depends on uninitialised value(s)
==8453==    at 0x6B3E481: ???
==8453==    by 0x67379AF: ???
==8453== 
foo /usr/local/share/qtcreator/fonts/SourceCodePro-Bold.ttf
foo /usr/local/share/qtcreator/fonts/SourceCodePro-BoldIt.ttf
foo /usr/local/share/qtcreator/fonts/SourceCodePro-It.ttf
foo /usr/local/share/qtcreator/fonts/SourceCodePro-Medium.ttf
foo /usr/local/share/qtcreator/fonts/SourceCodePro-MediumIt.ttf
foo /usr/local/share/qtcreator/fonts/SourceCodePro-Regular.ttf

I only get that on my amd64 machine (an old Xeon), not on x86, arm7 or aarch64.

Using Valgrind and vgdb and using step-instruction to get back to some source code this being called from

(gdb) list jit_machine_stack_exec
49 #endif /* defined(__has_feature) */
50
51 #ifdef SUPPORT_JIT
52
53 static SLJIT_NOINLINE int jit_machine_stack_exec(jit_arguments *arguments, jit_function executable_func)
54 {
55 sljit_u8 local_space[MACHINE_STACK_SIZE];
56 struct sljit_stack local_stack;
57
58 local_stack.min_start = local_space;
59 local_stack.start = local_space;
60 local_stack.end = local_space + MACHINE_STACK_SIZE;
61 local_stack.top = local_space + MACHINE_STACK_SIZE;
62 arguments->stack = &local_stack;
63 return executable_func(arguments);
64 }


`executable_func` is the point where I returned from the JITted code.

(gdb) p *arguments
$14 = {stack = 0x1fffff69e0, str = 0x67379b0, begin = 0x67379b0, end = 0x67379d2, match_data = 0x6737d60, startchar_ptr = 0x67379b0, mark_ptr = 0x0, callout = 0x0, callout_data = 0x0, offset_limit = 18446744073709551615, limit_match = 10000000, oveccount = 2, options = 0}

Everything in there is initialized apart from offset_limit and 4 bytes of padding at the end.

Should offset_limit be initialized?

Also the pointer members of arguments are all initialized.

I'm not certain that this isn't a Valgrind problem. --smc-check=all makes no difference.

I'm not familiar with pcre2 or sljit. Are there debug options to see the text for the JITted code?

Here is the call from Qt.

gdb) list safe_pcre2_match_16
1055 PCRE2_SPTR16 subject, qsizetype length,
1056 qsizetype startOffset, int options,
1057 pcre2_match_data_16 *matchData,
1058 pcre2_match_context_16 *matchContext)
1059 {
1060 int result = pcre2_match_16(code, subject, length,
1061 startOffset, options, matchData, matchContext);

"subject" looks OK.

There's a lot of code between the RE construction, call to match and the pcre2 backend.

@zherczeg
Copy link
Collaborator

zherczeg commented Jan 3, 2025

With gdb disassembly, you can display the text of the jitted function.

Usually these are caused by simd code, which reads data as aligned blocks, and may read after the end of the buffer. These are harmless due to virtual memory layout (paging). You can suppress these warnings in valgrind, or compile the pcre2 code using valgrind friendly mode. There are many similar bug reports, google can find them.

@NWilson
Copy link
Member

NWilson commented Jan 3, 2025

The performance cost is presumably negligible, if we were to simply zero out the extra bytes at the end of the buffer, if we know that SIMD is going to read them? It would be preferable for the JIT code to be valgrind-clean, if that's achievable.

I haven't looked into at all though. That would just be my preference for the code, eventually, if it's possible.

@paulfloyd
Copy link
Author

Suppressions aren't possible - Valgrind needs at least one function name. Does the JITter have any way of providing a function name?

Qt has
export QT_ENABLE_REGEXP_JIT=0
which seems to fix the problem with Qt apps.

I just added an entry for that to the Valgrind FAQ.

It would be nice to have a more general solution. I don't think that it's possible to fix this in memcheck. There is a heuristic for partial loads up to the native word size but if this is bigger than that then it won't apply.

@zherczeg
Copy link
Collaborator

zherczeg commented Jan 4, 2025

Try to compile pcre2 with --enable-valgrind which enables valgrind support. There are many similar discussions, please find them with google (e.g: "pcre2 valgrind uninitialized"). You need to know how virtual memory works, so it needs some extra knowledge about the cpu.

@paulfloyd
Copy link
Author

Try to compile pcre2 with --enable-valgrind which enables valgrind support. There are many similar discussions, please find them with google (e.g: "pcre2 valgrind uninitialized"). You need to know how virtual memory works, so it needs some extra knowledge about the cpu.

That's another possibility.

I know how memory works but I'm not familiar with pcre2 and sljit.

It would be nice to find a more generic solution.

@zherczeg
Copy link
Collaborator

zherczeg commented Jan 4, 2025

Generic? Valgrind is debugging tool, it is only used for finding issues / program analysis (We have a Valgrind Freya tool for configurable memory analysis). Passing an option when you do debugging should not be hard. In production environments, there are no issues.

You don't need to know about the jit compiler anything. The only thing that matters is that reading from an aligned address is always valid if the starting address is valid regardless how a buffer intersects with the area of the read. Memcheck in valgrind will complain, and that is its job, but the operation is still correct.

@NWilson NWilson added this to the 10.46 milestone Jan 8, 2025
@NWilson NWilson added JIT Relating to the JIT feature question Question from a user labels Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JIT Relating to the JIT feature question Question from a user
Projects
None yet
Development

No branches or pull requests

3 participants