[llvm-bugs] [Bug 42849] New: Mismatched Clang lock annotations (between prototype and impl) can lead to false negatives

[via llvm-bugs]

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

            Bug ID: 42849
           Summary: Mismatched Clang lock annotations (between prototype
                    and impl) can lead to false negatives
           Product: clang
           Version: trunk
          Hardware: PC
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P
         Component: Frontend
          Assignee: unassignedclangbugs at nondot.org
          Reporter: leonardchan at google.com
                CC: llvm-bugs at lists.llvm.org, neeilans at live.com,
                    richard-llvm at metafoo.co.uk

Created attachment 22326
  --> https://bugs.llvm.org/attachment.cgi?id=22326&action=edit
reproducer

Filing for a coworker

--------------------------

As the title says, a mismatch between lock annotations for a function/method
prototype and for the function's implementation can lead to false negatives for
the static thread analysis feature in clang.

I'll let the example do the talking.  Here is a godbolt link which shows the
behavior. https://godbolt.org/z/2rCs3U

The code is also explicitly attached to this bug for safe-keeping (I'm not sure
how long godbolt links are active for).  The summary of the issues are as
follows.

1. If you prototype a function and provide no annotations, then you use the
function with some requirement, and finally you implement the function with an
annotation which conflicts with the usage requirement; clang will accept this
when it should not.

2. Same as #1, but this time you prototype the function with a compatible
annotation, then you implement the function with a conflicting annotation.

3. Similar to #2, but this time you don't even need to bother to use the
function.  Just prototype it twice with conflicting annotations.

This happens for methods in C++ as well (except for #3, as C++ does not all you
to re-declare methods with identical signatures in the first place, regardless
of what annotations are)

IMO, all of these should be compile time failures.  Specifically, I think the
behavior should be as follows.

1. For a given prototype of a function, all subsequent prototypes should be
required to have identical annotations.  This is not to say that they have to
match character for character, just that if a prototype says that it requires
lock instance X, and excludes lock instance Y, then all other prototypes
encountered during compilation must agree with the first prototype encountered.
 This includes the no-annotation state.  If the first prototype encountered
says no annotations, then all subsequent prototypes must agree.

2. Implementations must follow rule #1 as well, with the following exception. 
If an implementation has no annotations, but a prototype has one or more
annotations, the implementation is assumed to have the same annotations as the
prototype.  This is mostly a convenience feature as the vast majority of code I
have encountered which uses these thread safety annotations uses C++, and is
annotated at the prototype of the methods (and not annotated at all when you
hit implementation).

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20190731/66c63f51/attachment.html>