From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 63F2B38582A4 for ; Wed, 14 Sep 2022 22:25:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 63F2B38582A4 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1663194358; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=Ne11xFfZDxJCzQA2pHWxdeZVAnUmdFy2+vK04RoMe1U=; b=CEkEM1ho3Qw4nKezXjmoqgk8r6gcXlwgs4oPtalSxhAhniq0vtC6ytYuWfKYuEsh51zcsg qYYqub8w2/8ZpCcve+3ofosPEAznCZOa3ZBwO5WdHcBWNbOzKsj+kjZ8kEpCsVZ8LDxeX2 08baaF/qziZ/tS5or6Ht2aWEekE57cY= Received: from mail-qk1-f198.google.com (mail-qk1-f198.google.com [209.85.222.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-618-La81DQAHORuggnJIqqNuWg-1; Wed, 14 Sep 2022 18:25:57 -0400 X-MC-Unique: La81DQAHORuggnJIqqNuWg-1 Received: by mail-qk1-f198.google.com with SMTP id u15-20020a05620a0c4f00b006b8b3f41303so14044940qki.8 for ; Wed, 14 Sep 2022 15:25:57 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date; bh=Ne11xFfZDxJCzQA2pHWxdeZVAnUmdFy2+vK04RoMe1U=; b=Y3nkhqR5VYWhbFr5J9xnIn49YuSCch51oI2AP15Ff6PoR90oKjx7O+e2Y4LAG0i9LX k9K29p5uUJQoONVkhGD5kG8f2jaHG4eVMP5IeroB1HX5hfHQoE5wwZlz9aNALvyHWRwS wAab9tb8Fv8eHLzAgbElg/6IMWnnr6qfR61XO65UBsybUd/mp661E4E1GKOEfgKgqxyN FQEg/ElTd9/xCwr+l+YOJY3UpMdoU2NPlW/+wiQRXz5BWDxcjeGdWD8J74RG2w7ZAUsc YicKfselSr8zMTvgR249ZdMScGuPdWGAKLwwFBajQsnjBnoiv620PfW3IxQ21gJn1kfC MUeg== X-Gm-Message-State: ACgBeo2mjopCyQHNQYlVvZZabCjd68k1ZiHe1T5Mi2HaQldR+9bDg2Wd jQq5KyjFG/dGqvHhnr0imTtKqnee2HPEpoBX5nxN7420LI3Jfl+Ft5PmKmAx510hYyPWEPMhwdu TUGjoTC1yB8ehEO5drX1tIytiyT3y5Fc= X-Received: by 2002:ac8:59c3:0:b0:343:6528:db29 with SMTP id f3-20020ac859c3000000b003436528db29mr34413011qtf.575.1663194357474; Wed, 14 Sep 2022 15:25:57 -0700 (PDT) X-Google-Smtp-Source: AA6agR5aGwIG8NfYNcmhj5cTCaxleR9wRayp/IK2IBwoSwfBrT2nPvL4xHrXq9umS9vEoj2OrXu1snsVkXiOiUYS4uw= X-Received: by 2002:ac8:59c3:0:b0:343:6528:db29 with SMTP id f3-20020ac859c3000000b003436528db29mr34413004qtf.575.1663194357274; Wed, 14 Sep 2022 15:25:57 -0700 (PDT) MIME-Version: 1.0 References: <20220914220449.276340-1-jwakely@redhat.com> In-Reply-To: <20220914220449.276340-1-jwakely@redhat.com> From: Jonathan Wakely Date: Wed, 14 Sep 2022 23:25:46 +0100 Message-ID: Subject: Re: [committed] libstdc++: Add TSan annotations to std::atomic> To: Jonathan Wakely Cc: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-13.0 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_LOW,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Wed, 14 Sept 2022 at 23:05, Jonathan Wakely via Libstdc++ wrote: > > Tested powerpc64le-linux, pushed to trunk. > > -- >8 -- > > This adds annotations to std::atomic> to enable TSan to > understand the custom locking. Without this, TSan reports data races for > accesses to the _M_ptr member, even though those are correctly > synchronized using atomic operations on the tagged pointer. > > libstdc++-v3/ChangeLog: > > * include/bits/shared_ptr_atomic.h (_GLIBCXX_TSAN_MUTEX_DESTROY) > (_GLIBCXX_TSAN_MUTEX_PRE_LOCK, _GLIBCXX_TSAN_MUTEX_POST_LOCK) > (_GLIBCXX_TSAN_MUTEX_PRE_UNLOCK, _GLIBCXX_TSAN_MUTEX_POST_UNLOCK) > (_GLIBCXX_TSAN_MUTEX_PRE_SIGNAL, _GLIBCXX_TSAN_MUTEX_POST_SIGNAL): > Define macros for TSan annotation functions. > (_Sp_atomic::_Atomic_count): Add annotations. > --- > libstdc++-v3/include/bits/shared_ptr_atomic.h | 38 +++++++++++++++++++ > 1 file changed, 38 insertions(+) > > diff --git a/libstdc++-v3/include/bits/shared_ptr_atomic.h b/libstdc++-v3/include/bits/shared_ptr_atomic.h > index d4bd712fc7d..4580807f42c 100644 > --- a/libstdc++-v3/include/bits/shared_ptr_atomic.h > +++ b/libstdc++-v3/include/bits/shared_ptr_atomic.h > @@ -32,6 +32,30 @@ > > #include > > +#if defined _GLIBCXX_TSAN && __has_include() > +#include > +#define _GLIBCXX_TSAN_MUTEX_DESTROY(X) \ > + __tsan_mutex_destroy(X, __tsan_mutex_not_static) > +#define _GLIBCXX_TSAN_MUTEX_PRE_LOCK(X) \ > + __tsan_mutex_pre_lock(X, __tsan_mutex_not_static) > +#define _GLIBCXX_TSAN_MUTEX_POST_LOCK(X) \ > + __tsan_mutex_post_lock(X, __tsan_mutex_not_static, 0) > +#define _GLIBCXX_TSAN_MUTEX_PRE_UNLOCK(X) \ > + __tsan_mutex_pre_unlock(X, __tsan_mutex_not_static) > +#define _GLIBCXX_TSAN_MUTEX_POST_UNLOCK(X) \ > + __tsan_mutex_post_unlock(X, __tsan_mutex_not_static) > +#define _GLIBCXX_TSAN_MUTEX_PRE_SIGNAL(X) __tsan_mutex_pre_signal(X, 0) > +#define _GLIBCXX_TSAN_MUTEX_POST_SIGNAL(X) __tsan_mutex_post_signal(X, 0) > +#else > +#define _GLIBCXX_TSAN_MUTEX_DESTROY(X) > +#define _GLIBCXX_TSAN_MUTEX_PRE_LOCK(X) > +#define _GLIBCXX_TSAN_MUTEX_POST_LOCK(X) > +#define _GLIBCXX_TSAN_MUTEX_PRE_UNLOCK(X) > +#define _GLIBCXX_TSAN_MUTEX_POST_UNLOCK(X) > +#define _GLIBCXX_TSAN_MUTEX_PRE_SIGNAL(X) > +#define _GLIBCXX_TSAN_MUTEX_POST_SIGNAL(X) > +#endif > + > namespace std _GLIBCXX_VISIBILITY(default) > { > _GLIBCXX_BEGIN_NAMESPACE_VERSION > @@ -377,6 +401,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > ~_Atomic_count() > { > auto __val = _M_val.load(memory_order_relaxed); > + _GLIBCXX_TSAN_MUTEX_DESTROY(&_M_val); After further thought, I'm not sure this is right. This tells tsan that the "mutex" at &_M_val cannot be locked or unlocked again after this. But what happens if the address is reused by a different atomic> which happens to be at the same memory address? Will tsan think that's an invalid use of the original "mutex" after its destruction? I will investigate. We might need to stop using the __tsan_mutex_destroy call, and if so, we can stop using the __tsan_mutex_not_static flag too. The pre/post lock/unlock/signal pairs are still valuable without the lifetime checking.