This is an archive of the discontinued LLVM Phabricator instance.

Thread safety analysis: Fix false negative on break
ClosedPublic

Authored by aaronpuchert on Apr 23 2021, 2:17 PM.

Details

Summary

We weren't modifying the lock set when intersecting with one coming
from a break-terminated block. This is inconsistent, since break isn't a
back edge, and it leads to false negatives with scoped locks. We usually
don't warn for those when joining locksets aren't the same, we just
silently remove locks that are not in the intersection. But not warning
and not removing them isn't right.

Diff Detail

Unit TestsFailed

Event Timeline

aaronpuchert requested review of this revision.Apr 23 2021, 2:17 PM
aaronpuchert created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptApr 23 2021, 2:17 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
aaronpuchert added inline comments.Apr 23 2021, 2:49 PM
clang/test/SemaCXX/warn-thread-safety-analysis.cpp
341

This is just a side effect, not the reason I did the change.

It's consistent though with our usual behavior on joining forward edges, see e.g. sls_fun_bad_11.

2634

This warning was missing, and is the reason for this change.

aaron.ballman accepted this revision.Apr 30 2021, 6:04 AM

LGTM, thank you for explaining the test cases, that helped (and I agree that the drive-by test changes look sensible to me).

This revision is now accepted and ready to land.Apr 30 2021, 6:04 AM
This revision was landed with ongoing or failed builds.May 3 2021, 5:04 AM
This revision was automatically updated to reflect the committed changes.