From patchwork Thu Apr 23 15:42:50 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marco Elver X-Patchwork-Id: 1275834 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=gcc.gnu.org Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=hhTTEvaW; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 497M5d66r3z9sSx for ; Fri, 24 Apr 2020 01:43:17 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 7DFD83959E6C; Thu, 23 Apr 2020 15:43:15 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7DFD83959E6C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1587656595; bh=EiX2uL8drSQ7Y08i5a75xrD0rRGZVKBAB52jPTId8Os=; h=Date:Subject:To:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=hhTTEvaWm54ysGEHnyjDBiIaGs9yykJ36TbbQ0rBD3zRAGdl9eAg9JREuPbSa8mOz qF+6Or2cnvE7z+2m89XYp7dPIUIf7tnQJEEJUADDND36wUg7i4XV0vdc/wL5D6XjcS pnZIsPx9vlKptpX4j31IN4wvMVQ7KwA1TP/kD6xo= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-ua1-x949.google.com (mail-ua1-x949.google.com [IPv6:2607:f8b0:4864:20::949]) by sourceware.org (Postfix) with ESMTPS id E1AD03959E56 for ; Thu, 23 Apr 2020 15:43:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org E1AD03959E56 Received: by mail-ua1-x949.google.com with SMTP id l9so3223965uao.12 for ; Thu, 23 Apr 2020 08:43:12 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:message-id:mime-version:subject:from:to:cc; bh=EiX2uL8drSQ7Y08i5a75xrD0rRGZVKBAB52jPTId8Os=; b=Flr840QAKxLz8cYnwY3JyaSKj2vW2pC8sIun8YukvHsgwt9L42qFL6q56MaTAwotUO Hc+Yr2gfycbBDcwBFWMkvoyk2F6XIPPvulVB0dkebiP8aeH/vavVpLgTVBrn4ZcyOYyT olCjVjfTsKkzuMqRJh/7M4Dn9n2lC/iApbWNY3PfH2rWG5zCX0l3e055EqOx43oYZqXP gNSJHmOM/fgoK0wxv7/g85aySsVSVUc5CGehuwrnbx9MVSrFg72iROC+OlLwvg5cXNEk YqQTn+9UREp+FmyE/rRdqAYRL4af+mTfsKjXP9MDU4HYK7kjfGsnUeloH6pEW0WyZIly 5Nxw== X-Gm-Message-State: AGi0PuYe2ZL7mH84zgQqsW6PFbjvVTq5UGugtcVPhOBEqYgRn4OpPxvJ RihPNGQc45AMiWqLs8gMb4/1mdIrNGs3yplQjPNF6XrVm48A+TA2YeLBlXbF8/ITg84kGKIzC2f NbEKVWcBKZtKN0UxYiCNEsVT8TRU6OpNVfvREOC6A5Q1wcXWvFu21rJ7Yogtj X-Google-Smtp-Source: APiQypLFIWkdxilEL81dquiyIARbWUHMrloR03ixjMcRd8ScCsyToY0O14W+BzgzV+qCB3BEApqKQxANMg== X-Received: by 2002:a1f:1e11:: with SMTP id e17mr4013563vke.73.1587656592201; Thu, 23 Apr 2020 08:43:12 -0700 (PDT) Date: Thu, 23 Apr 2020 17:42:50 +0200 Message-Id: <20200423154250.10973-1-elver@google.com> Mime-Version: 1.0 X-Mailer: git-send-email 2.26.1.301.g55bc3eb7cb9-goog Subject: [PATCH] tsan: Add optional support for distinguishing volatiles To: gcc-patches@gcc.gnu.org, jakub@redhat.com X-Spam-Status: No, score=-34.2 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Marco Elver via Gcc-patches From: Marco Elver Reply-To: Marco Elver Cc: elver@google.com, kasan-dev@googlegroups.com Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" Add support to optionally emit different instrumentation for accesses to volatile variables. While the default TSAN runtime likely will never require this feature, other runtimes for different environments that have subtly different memory models or assumptions may require distinguishing volatiles. One such environment are OS kernels, where volatile is still used in various places for various reasons, and often declare volatile to be "safe enough" even in multi-threaded contexts. One such example is the Linux kernel, which implements various synchronization primitives using volatile (READ_ONCE(), WRITE_ONCE()). Here the Kernel Concurrency Sanitizer (KCSAN) [1], is a runtime that uses TSAN instrumentation but otherwise implements a very different approach to race detection from TSAN. While in the Linux kernel it is generally discouraged to use volatiles explicitly, the topic will likely come up again, and we will eventually need to distinguish volatile accesses [2]. The other use-case is ignoring data races on specially marked variables in the kernel, for example bit-flags (here we may hide 'volatile' behind a different name such as 'no_data_race'). [1] https://github.com/google/ktsan/wiki/KCSAN [2] https://lkml.kernel.org/r/CANpmjNOfXNE-Zh3MNP=-gmnhvKbsfUfTtWkyg_=VqTxS4nnptQ@mail.gmail.com 2020-04-23 Marco Elver gcc/ * params.opt: Define --param=tsan-distinguish-volatile=[0,1]. * sanitizer.def (BUILT_IN_TSAN_VOLATILE_READ1): Define new builtin for volatile instrumentation of reads/writes. (BUILT_IN_TSAN_VOLATILE_READ2): Likewise. (BUILT_IN_TSAN_VOLATILE_READ4): Likewise. (BUILT_IN_TSAN_VOLATILE_READ8): Likewise. (BUILT_IN_TSAN_VOLATILE_READ16): Likewise. (BUILT_IN_TSAN_VOLATILE_WRITE1): Likewise. (BUILT_IN_TSAN_VOLATILE_WRITE2): Likewise. (BUILT_IN_TSAN_VOLATILE_WRITE4): Likewise. (BUILT_IN_TSAN_VOLATILE_WRITE8): Likewise. (BUILT_IN_TSAN_VOLATILE_WRITE16): Likewise. * tsan.c (get_memory_access_decl): Argument if access is volatile. If param tsan-distinguish-volatile is non-zero, and access if volatile, return volatile instrumentation decl. (instrument_expr): Check if access is volatile. gcc/testsuite/ * c-c++-common/tsan/volatile.c: New test. Acked-by: Dmitry Vyukov --- gcc/ChangeLog | 19 +++++++ gcc/params.opt | 4 ++ gcc/sanitizer.def | 21 ++++++++ gcc/testsuite/ChangeLog | 4 ++ gcc/testsuite/c-c++-common/tsan/volatile.c | 62 ++++++++++++++++++++++ gcc/tsan.c | 53 ++++++++++++------ 6 files changed, 146 insertions(+), 17 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/tsan/volatile.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 5f299e463db..aa2bb98ae05 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,22 @@ +2020-04-23 Marco Elver + + * params.opt: Define --param=tsan-distinguish-volatile=[0,1]. + * sanitizer.def (BUILT_IN_TSAN_VOLATILE_READ1): Define new + builtin for volatile instrumentation of reads/writes. + (BUILT_IN_TSAN_VOLATILE_READ2): Likewise. + (BUILT_IN_TSAN_VOLATILE_READ4): Likewise. + (BUILT_IN_TSAN_VOLATILE_READ8): Likewise. + (BUILT_IN_TSAN_VOLATILE_READ16): Likewise. + (BUILT_IN_TSAN_VOLATILE_WRITE1): Likewise. + (BUILT_IN_TSAN_VOLATILE_WRITE2): Likewise. + (BUILT_IN_TSAN_VOLATILE_WRITE4): Likewise. + (BUILT_IN_TSAN_VOLATILE_WRITE8): Likewise. + (BUILT_IN_TSAN_VOLATILE_WRITE16): Likewise. + * tsan.c (get_memory_access_decl): Argument if access is + volatile. If param tsan-distinguish-volatile is non-zero, and + access if volatile, return volatile instrumentation decl. + (instrument_expr): Check if access is volatile. + 2020-04-23 Srinath Parvathaneni * config/arm/arm_mve.h (__arm_vbicq_n_u16): Modify function parameter's diff --git a/gcc/params.opt b/gcc/params.opt index 4aec480798b..9b564bb046c 100644 --- a/gcc/params.opt +++ b/gcc/params.opt @@ -908,6 +908,10 @@ Stop reverse growth if the reverse probability of best edge is less than this th Common Joined UInteger Var(param_tree_reassoc_width) Param Optimization Set the maximum number of instructions executed in parallel in reassociated tree. If 0, use the target dependent heuristic. +-param=tsan-distinguish-volatile= +Common Joined UInteger Var(param_tsan_distinguish_volatile) IntegerRange(0, 1) Param +Emit special instrumentation for accesses to volatiles. + -param=uninit-control-dep-attempts= Common Joined UInteger Var(param_uninit_control_dep_attempts) Init(1000) IntegerRange(1, 65536) Param Optimization Maximum number of nested calls to search for control dependencies during uninitialized variable analysis. diff --git a/gcc/sanitizer.def b/gcc/sanitizer.def index 11eb6467eba..a32715ddb92 100644 --- a/gcc/sanitizer.def +++ b/gcc/sanitizer.def @@ -214,6 +214,27 @@ DEF_SANITIZER_BUILTIN(BUILT_IN_TSAN_READ_RANGE, "__tsan_read_range", DEF_SANITIZER_BUILTIN(BUILT_IN_TSAN_WRITE_RANGE, "__tsan_write_range", BT_FN_VOID_PTR_PTRMODE, ATTR_NOTHROW_LEAF_LIST) +DEF_SANITIZER_BUILTIN(BUILT_IN_TSAN_VOLATILE_READ1, "__tsan_volatile_read1", + BT_FN_VOID_PTR, ATTR_NOTHROW_LEAF_LIST) +DEF_SANITIZER_BUILTIN(BUILT_IN_TSAN_VOLATILE_READ2, "__tsan_volatile_read2", + BT_FN_VOID_PTR, ATTR_NOTHROW_LEAF_LIST) +DEF_SANITIZER_BUILTIN(BUILT_IN_TSAN_VOLATILE_READ4, "__tsan_volatile_read4", + BT_FN_VOID_PTR, ATTR_NOTHROW_LEAF_LIST) +DEF_SANITIZER_BUILTIN(BUILT_IN_TSAN_VOLATILE_READ8, "__tsan_volatile_read8", + BT_FN_VOID_PTR, ATTR_NOTHROW_LEAF_LIST) +DEF_SANITIZER_BUILTIN(BUILT_IN_TSAN_VOLATILE_READ16, "__tsan_volatile_read16", + BT_FN_VOID_PTR, ATTR_NOTHROW_LEAF_LIST) +DEF_SANITIZER_BUILTIN(BUILT_IN_TSAN_VOLATILE_WRITE1, "__tsan_volatile_write1", + BT_FN_VOID_PTR, ATTR_NOTHROW_LEAF_LIST) +DEF_SANITIZER_BUILTIN(BUILT_IN_TSAN_VOLATILE_WRITE2, "__tsan_volatile_write2", + BT_FN_VOID_PTR, ATTR_NOTHROW_LEAF_LIST) +DEF_SANITIZER_BUILTIN(BUILT_IN_TSAN_VOLATILE_WRITE4, "__tsan_volatile_write4", + BT_FN_VOID_PTR, ATTR_NOTHROW_LEAF_LIST) +DEF_SANITIZER_BUILTIN(BUILT_IN_TSAN_VOLATILE_WRITE8, "__tsan_volatile_write8", + BT_FN_VOID_PTR, ATTR_NOTHROW_LEAF_LIST) +DEF_SANITIZER_BUILTIN(BUILT_IN_TSAN_VOLATILE_WRITE16, "__tsan_volatile_write16", + BT_FN_VOID_PTR, ATTR_NOTHROW_LEAF_LIST) + DEF_SANITIZER_BUILTIN(BUILT_IN_TSAN_ATOMIC8_LOAD, "__tsan_atomic8_load", BT_FN_I1_CONST_VPTR_INT, ATTR_NOTHROW_LEAF_LIST) diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 245c1512c76..f1d3e236b86 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,7 @@ +2020-04-23 Marco Elver + + * c-c++-common/tsan/volatile.c: New test. + 2020-04-23 Jakub Jelinek PR target/94707 diff --git a/gcc/testsuite/c-c++-common/tsan/volatile.c b/gcc/testsuite/c-c++-common/tsan/volatile.c new file mode 100644 index 00000000000..d51d1e3ce8d --- /dev/null +++ b/gcc/testsuite/c-c++-common/tsan/volatile.c @@ -0,0 +1,62 @@ +/* { dg-additional-options "--param=tsan-distinguish-volatile=1" } */ + +#include +#include +#include + +int32_t Global4; +volatile int32_t VolatileGlobal4; +volatile int64_t VolatileGlobal8; + +static int nvolatile_reads; +static int nvolatile_writes; + +#ifdef __cplusplus +extern "C" { +#endif + +__attribute__((no_sanitize_thread)) +void __tsan_volatile_read4(void *addr) { + assert(addr == &VolatileGlobal4); + nvolatile_reads++; +} +__attribute__((no_sanitize_thread)) +void __tsan_volatile_write4(void *addr) { + assert(addr == &VolatileGlobal4); + nvolatile_writes++; +} +__attribute__((no_sanitize_thread)) +void __tsan_volatile_read8(void *addr) { + assert(addr == &VolatileGlobal8); + nvolatile_reads++; +} +__attribute__((no_sanitize_thread)) +void __tsan_volatile_write8(void *addr) { + assert(addr == &VolatileGlobal8); + nvolatile_writes++; +} + +#ifdef __cplusplus +} +#endif + +__attribute__((no_sanitize_thread)) +static void check() { + assert(nvolatile_reads == 4); + assert(nvolatile_writes == 4); +} + +int main() { + Global4 = 1; + + VolatileGlobal4 = 1; + Global4 = VolatileGlobal4; + VolatileGlobal4 = 1 + VolatileGlobal4; + + VolatileGlobal8 = 1; + Global4 = (int32_t)VolatileGlobal8; + VolatileGlobal8 = 1 + VolatileGlobal8; + + check(); + return 0; +} diff --git a/gcc/tsan.c b/gcc/tsan.c index 8d22a776377..04e92559584 100644 --- a/gcc/tsan.c +++ b/gcc/tsan.c @@ -52,25 +52,41 @@ along with GCC; see the file COPYING3. If not see void __tsan_read/writeX (void *addr); */ static tree -get_memory_access_decl (bool is_write, unsigned size) +get_memory_access_decl (bool is_write, unsigned size, bool volatilep) { enum built_in_function fcode; - if (size <= 1) - fcode = is_write ? BUILT_IN_TSAN_WRITE1 - : BUILT_IN_TSAN_READ1; - else if (size <= 3) - fcode = is_write ? BUILT_IN_TSAN_WRITE2 - : BUILT_IN_TSAN_READ2; - else if (size <= 7) - fcode = is_write ? BUILT_IN_TSAN_WRITE4 - : BUILT_IN_TSAN_READ4; - else if (size <= 15) - fcode = is_write ? BUILT_IN_TSAN_WRITE8 - : BUILT_IN_TSAN_READ8; + if (param_tsan_distinguish_volatile && volatilep) + { + if (size <= 1) + fcode = is_write ? BUILT_IN_TSAN_VOLATILE_WRITE1 + : BUILT_IN_TSAN_VOLATILE_READ1; + else if (size <= 3) + fcode = is_write ? BUILT_IN_TSAN_VOLATILE_WRITE2 + : BUILT_IN_TSAN_VOLATILE_READ2; + else if (size <= 7) + fcode = is_write ? BUILT_IN_TSAN_VOLATILE_WRITE4 + : BUILT_IN_TSAN_VOLATILE_READ4; + else if (size <= 15) + fcode = is_write ? BUILT_IN_TSAN_VOLATILE_WRITE8 + : BUILT_IN_TSAN_VOLATILE_READ8; + else + fcode = is_write ? BUILT_IN_TSAN_VOLATILE_WRITE16 + : BUILT_IN_TSAN_VOLATILE_READ16; + } else - fcode = is_write ? BUILT_IN_TSAN_WRITE16 - : BUILT_IN_TSAN_READ16; + { + if (size <= 1) + fcode = is_write ? BUILT_IN_TSAN_WRITE1 : BUILT_IN_TSAN_READ1; + else if (size <= 3) + fcode = is_write ? BUILT_IN_TSAN_WRITE2 : BUILT_IN_TSAN_READ2; + else if (size <= 7) + fcode = is_write ? BUILT_IN_TSAN_WRITE4 : BUILT_IN_TSAN_READ4; + else if (size <= 15) + fcode = is_write ? BUILT_IN_TSAN_WRITE8 : BUILT_IN_TSAN_READ8; + else + fcode = is_write ? BUILT_IN_TSAN_WRITE16 : BUILT_IN_TSAN_READ16; + } return builtin_decl_implicit (fcode); } @@ -204,8 +220,11 @@ instrument_expr (gimple_stmt_iterator gsi, tree expr, bool is_write) g = gimple_build_call (builtin_decl, 2, expr_ptr, size_int (size)); } else if (rhs == NULL) - g = gimple_build_call (get_memory_access_decl (is_write, size), - 1, expr_ptr); + { + builtin_decl = get_memory_access_decl (is_write, size, + TREE_THIS_VOLATILE(expr)); + g = gimple_build_call (builtin_decl, 1, expr_ptr); + } else { builtin_decl = builtin_decl_implicit (BUILT_IN_TSAN_VPTR_UPDATE);