Bug 2211220 - clang TSA does not analyze across function pointers
Summary: clang TSA does not analyze across function pointers
Keywords:
Status: ASSIGNED
Alias: None
Product: Red Hat Enterprise Linux 9
Classification: Red Hat
Component: clang
Version: 9.2
Hardware: Unspecified
OS: Unspecified
high
unspecified
Target Milestone: rc
: ---
Assignee: Timm Bäder
QA Contact: Jesus Checa
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2023-05-30 19:47 UTC by Stefan Hajnoczi
Modified: 2023-07-25 20:51 UTC (History)
8 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed:
Type: Bug
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Issue Tracker RHELPLAN-158612 0 None None None 2023-05-30 19:48:14 UTC

Description Stefan Hajnoczi 2023-05-30 19:47:01 UTC
Description of problem:
clang's Thread Safety Analysis does not extend across function pointers. Function pointers are very common in C code, making it hard to use TSA in C programs.

There is a workaround in the form of annotating the function pointers
themselves. This way at least the analyzer guarantees the lock is
held when the function pointer value is loaded, but it doesn't actually
analyze the function pointer target function or check that the call
is made while the lock is still held.

Marc-André Lureau had patches for this five years ago but does not have the time to get them upstream:
https://github.com/elmarco/clang/commits/qemu-ta

Version-Release number of selected component (if applicable):
clang-15.0.7-2.el9.x86_64

How reproducible:
100%

Steps to Reproduce:
1. cat >a.c
#include <pthread.h>

typedef pthread_mutex_t __attribute__((capability("mutex"))) mutex;

static mutex lock = PTHREAD_MUTEX_INITIALIZER;
static int counter __attribute__((guarded_by(lock)));

void mutex_lock(mutex *m) __attribute__((acquire_capability(lock))) __attribute__((no_thread_safety_analysis))
{
    pthread_mutex_lock(m);
}

void mutex_unlock(mutex *m) __attribute__((release_capability(lock))) __attribute__((no_thread_safety_analysis))
{
    pthread_mutex_unlock(m);
}

static void counter_inc(void) __attribute__((requires_capability(lock)))
{
    counter++;
}

int main(int argc, char **argv)
{
    /* TSA does not detect that lock must be held */
    void (*counter_inc_fn)(void) = counter_inc;
    counter_inc_fn();
    return 0;
}
^D
2. $ clang -Wthread-safety -o a a.c

Actual results:
No TSA warning is emitted.

Expected results:
TSA warns that counter_inc() was called without holding lock.

Additional info:

Comment 1 Timm Bäder 2023-06-12 14:50:31 UTC
Can you give an example of this pattern in the source code? I assume the called function pointer can have more than one possible value, which is not the case in the posted reproducer?

Comment 2 Kevin Wolf 2023-06-20 16:34:42 UTC
The most common case of this in real world QEMU code (which is where the request comes from) is structs of function pointers that basically serve as a vtable for objects that point at them.

For example given a BlockDriverState *bs, we'd call something like bs->drv->bdrv_co_preadv(), which could then be raw_co_preadv() or vmdk_co_preadv() depending on the image format of the disk image we're looking at.

The other relevant case is where we pass a callback to a function and want to annotate that the lock is held when calling the callback.


Note You need to log in before you can comment on or make changes to this bug.