Commit Graph

4 Commits

Author SHA1 Message Date
Azat Khuzhin
301ac5dab7 Fix possible data-race StorageKafka with statistics_interval_ms>0
The problem here is that ignorelist did not work by some reason, if I
will look at the ignored functions it should not contain any TSan
interseption code, while it does:

    $ lldb-13 clickhouse
    (lldb) target create "clickhouse"
    disas -n rd_avg_rollover
    Current executable set to '/home/azat/ch/tmp/tsan-test/clickhouse' (x86_64).
    (lldb) disas -n rd_avg_rollover
    clickhouse`rd_kafka_stats_emit_avg:
    clickhouse[0x1cbf84a7] <+39>:  leaq   0x30(%r15), %r12
    clickhouse[0x1cbf84ab] <+43>:  movq   %r12, %rdi
    clickhouse[0x1cbf84ae] <+46>:  callq  0x1ccdad40                ; rdk_thread_mutex_lock at tinycthread.c:111
    clickhouse[0x1cbf84b3] <+51>:  leaq   0x58(%r15), %rdi
    clickhouse[0x1cbf84b7] <+55>:  callq  0x71b5390                 ; __tsan_read4
    clickhouse[0x1cbf84bc] <+60>:  cmpl   $0x0, 0x58(%r15)
    clickhouse[0x1cbf84c1] <+65>:  je     0x1cbf8595                ; <+277> [inlined] rd_avg_rollover + 238 at rdavg.h
    clickhouse[0x1cbf84c7] <+71>:  leaq   -0xc8(%rbp), %rdi
    clickhouse[0x1cbf84ce] <+78>:  xorl   %esi, %esi
    clickhouse[0x1cbf84d0] <+80>:  callq  0x1ccdac80                ; rdk_thread_mutex_init at tinycthread.c:62
    clickhouse[0x1cbf84d5] <+85>:  leaq   0x5c(%r15), %rdi
    clickhouse[0x1cbf84d9] <+89>:  callq  0x71b5390                 ; __tsan_read4

    (lldb) disas -n rd_avg_calc
    clickhouse`rd_kafka_broker_ops_io_serve:
    clickhouse[0x1cbdf086] <+1990>: leaq   0x5a4(%rbx), %rdi
    clickhouse[0x1cbdf08d] <+1997>: callq  0x71b5390                 ; __tsan_read4
    clickhouse[0x1cbdf092] <+2002>: cmpl   $0x0, 0x5a4(%rbx)
    clickhouse[0x1cbdf099] <+2009>: je     0x1cbdf12b                ; <+2155> [inlined] rd_kafka_broker_timeout_scan + 719 at rdkafka_broker.c

I guess the reason is that they had been inlined

So now rd_avg_calc() guarded with a mutex.

Refs: https://github.com/ClickHouse/librdkafka/pull/11
Fixes: https://github.com/ClickHouse/ClickHouse/issues/60939
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
2024-08-08 07:43:29 +02:00
Azat Khuzhin
eb4c095077 Fix suppressions for rd_avg_calc()/rd_avg_rollover() (due to static qualifier)
The problem is that TSan still fails [1] is that ignorelist does not
work for static functions without asterisk:

    // test.cpp
    #include <thread>

    bool flag = false;

    // avoid mangling
    extern "C" {

    static void set_flag_impl()
    {
        flag = true;
    }

    void set_flag()
    {
        set_flag_impl();
    }
    void set_flag_if()
    {
        if (flag)
            flag = false;
    }

    }

    int main()
    {
        std::thread t1([]{ set_flag(); });
        std::thread t2([]{ set_flag_if(); });

        t1.join();
        t2.join();

        return 0;
    }

    // ignorelist
    [thread]
    fun:set_flag_impl

    $ clang++ -g -fno-omit-frame-pointer -fsanitize=thread -fsanitize-ignorelist=ignorelist -o test test.cpp && ./test
    SUMMARY: ThreadSanitizer: data race /tmp/test-tsan-ignorelist/test.cpp:19:9 in set_flag_if
    $ sed -i 's/set_flag_impl/*set_flag_impl*/' ignorelist
    $ clang++ -g -fno-omit-frame-pointer -fsanitize=thread -fsanitize-ignorelist=ignorelist -o test test.cpp && ./test
    OK

But, note that ignorelist is tricky, and will not work for
functions with __always_inline__ attribute for example.

P.S. set_flag_impl also has brackets in the output (i.e.
set_flag_impl()), while ther eis brackets for rd_avg_calc on CI [1].

  [1]: https://s3.amazonaws.com/clickhouse-test-reports/63039/84bebc534ba7cf6e9dbfc1d91e8350939a84f87c/integration_tests__tsan__[6_6]//home/ubuntu/actions-runner/_work/_temp/test/output_dir/integration_run_parallel4_0.log

Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
2024-04-27 09:51:59 +02:00
Azat Khuzhin
6e534650e4 Use sanitizer specific ignorelists
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
2024-04-26 15:46:04 +03:00
Azat Khuzhin
90e1f7d8ec Fix sanitizers suppressions
The -fsanitize-ignorelist (-fsanitize-blacklist is the alias for it)
accepts not the suppressions but special case list, that accept only
`fun` and `src`, so convert tsan_suppressions.txt into a proper
tsan_ignorelist.txt with a proper syntax, otherwise suppressions simply
does not work [1].

  [1]: https://s3.amazonaws.com/clickhouse-test-reports/61526/958659584957ff419a9305d9c7edee5703fedbdc/integration_tests__tsan__[6_6].html

Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
2024-03-24 16:22:23 +01:00