Page MenuHomePhabricator

[clang analysis] Make mutex guard detection more reliable.
ClosedPublic

Authored by efriedma on Mar 27 2020, 11:53 AM.

Details

Summary

-Wthread-safety was failing to detect certain AST patterns it should detect. Make the pattern detection a bit more comprehensive.

Due to an unrelated bug involving template instantiation, this showed up as a regression in 10.0 vs. 9.0 in the original bug report. The included testcase fails on older versions of clang, though.

Fixes https://bugs.llvm.org/show_bug.cgi?id=45323 .

Diff Detail

Event Timeline

efriedma created this revision.Mar 27 2020, 11:53 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 27 2020, 11:54 AM
rsmith accepted this revision.Mar 27 2020, 1:16 PM
rsmith added inline comments.
clang/lib/Analysis/ThreadSafety.cpp
2144–2145

Should we also skip over CK_UserDefinedConversion here (for conversion functions that return mutex locks, I suppose)?

This revision is now accepted and ready to land.Mar 27 2020, 1:16 PM
aaronpuchert added inline comments.
clang/lib/Analysis/ThreadSafety.cpp
2144–2145

Seems reasonable, there could be a CXXMemberCallExpr to an annotated function below that.

efriedma updated this revision to Diff 253253.Mar 27 2020, 4:59 PM

Add support for conversion operators.

rsmith accepted this revision.Mar 27 2020, 5:12 PM
aaronpuchert added inline comments.Mar 28 2020, 9:07 AM
clang/test/SemaCXX/warn-thread-safety-analysis.cpp
5659

Honestly I'd prefer the annotation to be on this operator instead of the move constructor, but that's an independent issue that existed before this change. (rC322316#294681)

This revision was automatically updated to reflect the committed changes.