Skip to content

False positive in -Wthread-safety-analysis when using mutexes in collections #58535

@rupprecht

Description

@rupprecht

I found this while making fixes for D129755, but it looks like this is an existing limitation of -Wthread-safety-analysis.

When using mutexes as part of a collection, thread safety analysis seems to get confused and think that all mutexes in the collection are the same.

void bad_loop() {
    std::vector<std::mutex *> mutexes{new std::mutex(), new std::mutex()};

    for (std::mutex *m : mutexes) { // error: expecting mutex 'm' to be held at start of each loop [-Werror,-Wthread-safety-analysis]
        m->lock(); // note: mutex acquired here
    }

    // This is where we do stuff that's only safe when _all_ locks are locked

    for (std::mutex *m : mutexes) {
        m->unlock(); // error: releasing mutex 'm' that was not held [-Werror,-Wthread-safety-analysis]
    }
}

The analysis would make sense if we were locking the same mutex over and over again, but m is a different mutex on each loop iteration. Maybe the analysis is confused because, due to being in a loop, m has the same name on each iteration. But it also fails if we unroll:

void bad_unroll() {
    std::vector<std::mutex *> mutexes{new std::mutex(), new std::mutex()};

    mutexes[0]->lock(); // note: mutex acquired here
    mutexes[1]->lock(); // error: acquiring mutex 'operator[](mutexes, 1)' that is already held [-Werror,-Wthread-safety-analysis]

    // This is where we do stuff that's only safe when _all_ locks are locked

    mutexes[0]->unlock(); // note: mutex released here
    mutexes[1]->unlock(); // error: releasing mutex 'operator[](mutexes, 1)' that was not held [-Werror,-Wthread-safety-analysis]
}

This could be a valid warning if thread safety analysis is conservatively assuming that mutexes[0] and mutexes[1] might point to the same thing, but since alias analysis is unsupported, I don't think that's it.

This might be something that's intentionally unsupported by thread safety analysis, but I didn't see any documentation for it being a known failure.

Live demo: https://godbolt.org/z/1sn5TMj3h

Metadata

Metadata

Assignees

Labels

clang:diagnosticsNew/improved warning or error message in Clang, but not in clang-tidy or static analyzerfalse-positiveWarning fires when it should not

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions