From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 534 invoked by alias); 5 Sep 2008 09:39:22 -0000 Received: (qmail 526 invoked by uid 22791); 5 Sep 2008 09:39:21 -0000 X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org (qpsmtpd/0.31) with ESMTP; Fri, 05 Sep 2008 09:38:47 +0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id m859cjba008073; Fri, 5 Sep 2008 05:38:45 -0400 Received: from zebedee.pink (vpn-12-243.rdu.redhat.com [10.11.12.243]) by int-mx1.corp.redhat.com (8.13.1/8.13.1) with ESMTP id m859cfZh015104; Fri, 5 Sep 2008 05:38:41 -0400 Message-ID: <48C0FE20.2020103@redhat.com> Date: Fri, 05 Sep 2008 11:28:00 -0000 From: Andrew Haley User-Agent: Thunderbird 2.0.0.16 (X11/20080707) MIME-Version: 1.0 To: "Boehm, Hans" CC: Java Patch List Subject: Re: Remove data race in libgcj interpreter References: <48AD67B2.4040308@redhat.com> <48C0042C.5080804@redhat.com> <238A96A773B3934685A7269CC8A8D0423152AF178E@GVW0436EXB.americas.hpqcorp.net> In-Reply-To: <238A96A773B3934685A7269CC8A8D0423152AF178E@GVW0436EXB.americas.hpqcorp.net> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes Mailing-List: contact java-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: java-patches-owner@gcc.gnu.org X-SW-Source: 2008-q3/txt/msg00075.txt.bz2 Boehm, Hans wrote: >> + >> /************************************************************* >> ********** >> + Locking in the direct-threaded case is quite subtle. See PR >> + libgcj/8995 for more details. >> + >> + We must ensure that no-one ever reads an inconsistent {insn,value} >> + pair. Rather than trying to lock everything we detect when some >> + other thread has modified the insn we are trying to >> modify and back >> + out. >> + >> + When we read a pair before rewriting it we do this: >> + >> + i1 = insn >> + v = value >> + lock >> + i2 = insn >> + unlock >> + if (i2 != i1) try again Ah, thanks for replying. I was hoping you'd see this. > This is probably statistically likely to generate correct code on > some machines, but it doesn't look correct to me. I think you're > counting on the three loads to be performed in the order written. > But the compiler gets to reshuffle the first two. And there is > generally no prohibition against moving loads INTO a critical > section, so they can in fact also be arbitrarily reordered with the > i2 assignment as well. (On IA64 and SPARC, the hardware might > conceivably do that, depending on the lock implementation. There is > a long story about when this is Posix compatible, but it doesn't > really matter here.) You're right, I was assuming exactly that. OK, but that is presumably simply a lack of volatility in the declaration of PC. > There is also another important issue: Posix and the upcoming C++ > standard (among others) give undefined behavior to any program > involving a data race. Clearly this program involves data races on > the initial insn and value reads. The problem is that there are > several good reasons for the Posix and C++ positions: > 1) Compilers, including I believe gcc, implicitly assume that there > are no asynchronous changes on non-volatile variables. If you > invalidate this assumption, weird things may happen. These weird > things don't always correspond to reading a different value. For > example, the compiler may reread a register copy of the value from > the shared variable when it has to spill the register, making it > appear that a local variable (which is a copy of the shared > variable) suddenly changes without an assignment to it. It's not > hard to come up with cases in which this would provoke a crash. > Empirically, this sort of thing doesn't happen much, but we were > told at some point that gcc may behave this way. > 2) I would guess that gcc supports some platforms on which things > like pointer loads are not always atomic. Sure, but on such platforms gcj won't run so I don't care about them. > 3) In the not-too-distant future, we'd really like to have reliable > data-race detectors. This is much easier if code doesn't contain > intentional data races. Point taken. > 4) None of us has managed to come up with an entirely satisfactory > definition of what programs with data races mean. The JSR133 > definition is reasonably close, but still has some issues that have > come up recently. And it would probably have more serious > performance implications on something like C++. > I haven't thought though this through enough. Does it work to > instead keep a shared (possibly per method?) count of the number of > running interpreters, together with a shared bit (tested on backward > branches) asking them to stop? You only modify the code if no > interpreters are running (in that method). Getting this right is > still nontrivial, but the races are restricted to well-defined > atomic variables, which is fine. Thank you for that suggestion. I'll think some more. Andrew.