From patchwork Sat Dec 1 12:23:18 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 203130 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 330932C008E for ; Sat, 1 Dec 2012 23:23:38 +1100 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1354969419; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Received:Received:Date:From:To:Cc:Subject:Message-ID:Reply-To: References:MIME-Version:Content-Type:Content-Disposition: In-Reply-To:User-Agent:Mailing-List:Precedence:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:Sender: Delivered-To; bh=cCocFHbPDlIELSJbZmMvVxMjxvc=; b=aWjm4mYnjHW7gXP exv/U1u4Hrk6Yidn8AD8oJTU7kTkpJBWBq/fAjGHsOl/oFRPYzp35t0PIxiOBIQD QfbtEiJI8S9RjVX0RvJuPXhlffO9WrPXYcruPF3bFjdJVbAWRU6tf6zZlH8qbJ6C rMP/05g2izEOJ4TGSs+2TNBBjcxQ= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Received:Received:Received:Date:From:To:Cc:Subject:Message-ID:Reply-To:References:MIME-Version:Content-Type:Content-Disposition:In-Reply-To:User-Agent:X-IsSubscribed:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=yFi+X5LEB5QWFp/iOWOnWwRi5fXsaSy6A177v16nzXYo2nXH/e//QwDXpGtJr5 yI5NfoHZbLypvkfZO2j1XgdfF3vwgMVUtlBifxN/N+81i7k+JvcrEupm9y2b+82b Lx2us0hIg2zz/xwgAWg74I/J4hZ2Dgcg83bgZOXyBDzeE=; Received: (qmail 31647 invoked by alias); 1 Dec 2012 12:23:29 -0000 Received: (qmail 31634 invoked by uid 22791); 1 Dec 2012 12:23:27 -0000 X-SWARE-Spam-Status: No, hits=-6.5 required=5.0 tests=AWL, BAYES_00, KHOP_RCVD_UNTRUST, RCVD_IN_DNSWL_HI, RCVD_IN_HOSTKARMA_W, RP_MATCHES_RCVD, SPF_HELO_PASS, TW_VP X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 01 Dec 2012 12:23:22 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id qB1CNLPQ009183 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Sat, 1 Dec 2012 07:23:21 -0500 Received: from zalov.redhat.com (vpn1-4-152.ams2.redhat.com [10.36.4.152]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id qB1CNJ7Q020983 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sat, 1 Dec 2012 07:23:20 -0500 Received: from zalov.cz (localhost [127.0.0.1]) by zalov.redhat.com (8.14.5/8.14.5) with ESMTP id qB1CNJ4d003689; Sat, 1 Dec 2012 13:23:19 +0100 Received: (from jakub@localhost) by zalov.cz (8.14.5/8.14.5/Submit) id qB1CNIWe003688; Sat, 1 Dec 2012 13:23:18 +0100 Date: Sat, 1 Dec 2012 13:23:18 +0100 From: Jakub Jelinek To: Dmitry Vyukov Cc: Dodji Seketeli , GCC Patches Subject: Re: [tsan] Small bugfix Message-ID: <20121201122318.GN2315@tucnak.redhat.com> Reply-To: Jakub Jelinek References: <20121201000440.GL2315@tucnak.redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org On Sat, Dec 01, 2012 at 01:53:52PM +0400, Dmitry Vyukov wrote: > On Sat, Dec 1, 2012 at 4:04 AM, Jakub Jelinek wrote: > > Hi! > > > > When I've tried to compile the attached testcase (I was trying to see > > if tsan could discover the emutls.c data race), I got ICEs because > > expr_ptr in certain cases wasn't is_gimple_val and thus was invalid to > > pass it directly to a call as argument, fixed thusly. > > > > Unfortunately, trying to compile it dynamically against libtsan.so > > doesn't work (apparently it is using -fvisibility=hidden, but not > > saying the public entry points have default visibility), > > Runtime needs to mark all interface functions as visibility("default"), right? Yeah, == SANITIZER_INTERFACE_ATTRIBUTE . > > compiling it > > by hand statically against libtsan.a (we don't have -static-libtsan yet) > > failed at runtime, complaining the binary isn't a PIE - can't it really > > support normal executables? > > It's not trivial to do fast shadow memory mapping in this case. > Initially non pie builds ware not planned at. But I am starting to > think that I know how to do it. > I will try to look into it in next weeks. Perhaps libtsan.a could be for PIEs only, and document that -static-libtsan has that limitation. And libtsan.so.0 could add in some offset to allow even non-PIE binaries. > > and when compiled/linked as PIE, I got > > ================== > > WARNING: ThreadSanitizer: thread leak (pid=31150) > > Thread 3 (tid=31153, finished) created at: > > #0 pthread_create ??:0 (exe+0x00000000e4dc) > > #1 main ??:0 (exe+0x00000000505f) > > > > ================== > > ================== > > WARNING: ThreadSanitizer: thread leak (pid=31150) > > Thread 4 (tid=31155, finished) created at: > > #0 pthread_create ??:0 (exe+0x00000000e4dc) > > #1 main ??:0 (exe+0x00000000505f) > > > > ================== > > ================== > > WARNING: ThreadSanitizer: thread leak (pid=31150) > > Thread 5 (tid=31156, finished) created at: > > #0 pthread_create ??:0 (exe+0x00000000e4dc) > > #1 main ??:0 (exe+0x00000000505f) > > > > ================== > > ThreadSanitizer: reported 3 warnings > > > > which is probably not what I was expecting to see. > > The thread leak reports are correct, right? No idea what do you mean by thread leak. What exactly is leaking? > The race must be detectable. Can you show the code? The first thing to > check is that the memory accesses are instrumented. Also if you build > runtime with -DTSAN_DEBUG_OUTPUT=2 it will print all incoming events; > if you post the log most likely I will be able to say why the race is > not detected. Ah, on closer inspection I found the bug on the GCC side, forgotten gimple_insert_before instead of gimple_insert_seq_before in one case. Now it reports the race (3 times): Jakub ================== WARNING: ThreadSanitizer: data race (pid=3613) Write of size 8 at 0x7fc5c8bfae40 by thread 1: #0 foo emutlstest.c:56 (exe+0x0000000053b7) #1 tf emutlstest.c:102 (exe+0x0000000054a9) Previous read of size 8 at 0x7fc5c8bfae40 by thread 2: #0 foo emutlstest.c:46 (exe+0x0000000052c7) #1 tf emutlstest.c:102 (exe+0x0000000054a9) Thread 1 (tid=3615, running) created at: #0 pthread_create ??:0 (exe+0x00000000e54c) #1 main emutlstest.c:115 (exe+0x00000000505f) Thread 2 (tid=3616, running) created at: #0 pthread_create ??:0 (exe+0x00000000e54c) #1 main emutlstest.c:115 (exe+0x00000000505f) ================== while when using uintptr_t offset = __atomic_load_n (&x->offset, __ATOMIC_ACQUIRE); instead of uintptr_t offset = x->offset; it doesn't report it. So here is the fixed up patch: 2012-12-01 Jakub Jelinek * tsan.c (instrument_expr): If expr_ptr isn't a gimple val, first store it into a SSA_NAME. --- gcc/tsan.c.jj 2012-12-01 12:51:40.437808319 +0100 +++ gcc/tsan.c 2012-12-01 13:11:31.347889889 +0100 @@ -93,10 +93,11 @@ is_vptr_store (gimple stmt, tree expr, b static bool instrument_expr (gimple_stmt_iterator gsi, tree expr, bool is_write) { - tree base, rhs, expr_type, expr_ptr, builtin_decl; + tree base, rhs, expr_ptr, builtin_decl; basic_block bb; HOST_WIDE_INT size; gimple stmt, g; + gimple_seq seq; location_t loc; size = int_size_in_bytes (TREE_TYPE (expr)); @@ -139,21 +140,25 @@ instrument_expr (gimple_stmt_iterator gs rhs = is_vptr_store (stmt, expr, is_write); gcc_checking_assert (rhs != NULL || is_gimple_addressable (expr)); expr_ptr = build_fold_addr_expr (unshare_expr (expr)); - if (rhs == NULL) + seq = NULL; + if (!is_gimple_val (expr_ptr)) { - expr_type = TREE_TYPE (expr); - while (TREE_CODE (expr_type) == ARRAY_TYPE) - expr_type = TREE_TYPE (expr_type); - size = int_size_in_bytes (expr_type); - g = gimple_build_call (get_memory_access_decl (is_write, size), - 1, expr_ptr); + g = gimple_build_assign (make_ssa_name (TREE_TYPE (expr_ptr), NULL), + expr_ptr); + expr_ptr = gimple_assign_lhs (g); + gimple_set_location (g, loc); + gimple_seq_add_stmt_without_update (&seq, g); } + if (rhs == NULL) + g = gimple_build_call (get_memory_access_decl (is_write, size), + 1, expr_ptr); else { builtin_decl = builtin_decl_implicit (BUILT_IN_TSAN_VPTR_UPDATE); g = gimple_build_call (builtin_decl, 1, expr_ptr); } gimple_set_location (g, loc); + gimple_seq_add_stmt_without_update (&seq, g); /* Instrumentation for assignment of a function result must be inserted after the call. Instrumentation for reads of function arguments must be inserted before the call. @@ -170,13 +175,13 @@ instrument_expr (gimple_stmt_iterator gs bb = gsi_bb (gsi); e = find_fallthru_edge (bb->succs); if (e) - gsi_insert_seq_on_edge_immediate (e, g); + gsi_insert_seq_on_edge_immediate (e, seq); } else - gsi_insert_after (&gsi, g, GSI_NEW_STMT); + gsi_insert_seq_after (&gsi, seq, GSI_NEW_STMT); } else - gsi_insert_before (&gsi, g, GSI_SAME_STMT); + gsi_insert_seq_before (&gsi, seq, GSI_SAME_STMT); return true; }