From patchwork Thu Feb 20 09:57:19 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Edward Nevill X-Patchwork-Id: 25000 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-oa0-f70.google.com (mail-oa0-f70.google.com [209.85.219.70]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id AD83E203C6 for ; Thu, 20 Feb 2014 09:57:23 +0000 (UTC) Received: by mail-oa0-f70.google.com with SMTP id m1sf6434756oag.9 for ; Thu, 20 Feb 2014 01:57:23 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:message-id:subject:from:reply-to:to :cc:date:organization:mime-version:x-original-sender :x-original-authentication-results:precedence:mailing-list:list-id :list-post:list-help:list-archive:list-unsubscribe:content-type :content-transfer-encoding; bh=QCitmQ/yOn31LsrNxBz4UNPd3UnRp8MZDMEeY6iiNYc=; b=aR27zzJELs7X5gGYU1D0uxEg8cdmhtuTQd4qCiLcBejInX8CJUIlW2erLZEsXIBPWx qrcxHb2g/wQ6yHAkki1n6N99xC+WtoMCV7VUqhRRoFU0eKF+RN2Zapo9mkiW3Wk8CFBm KzlqsXQ1I5+9bmZyOZXcQt1d60CmTiaespUmfg72p97asMIa7ABlrv8LfL4RtlKDQWZH YKooK2cAYZNsn9LxL+xLcD8F9c4eHeGocuf+zZ4z0yRfHK16Mkorg7EPubz/6bVFqY+I 9Qhx1qwBBptkLnqiTjlRkoej+AZuDDWkTiGk28eKpOCxTfnf/mhbi2/ksJ1Zz28RNwee sPag== X-Gm-Message-State: ALoCoQkaPfLEvSGkBQb9WhAv4KpiGnDMKviLMps43XamlWHbwvPjJdu/Lz2EtEr+ps0/V1brHn3o X-Received: by 10.182.87.2 with SMTP id t2mr391915obz.2.1392890243208; Thu, 20 Feb 2014 01:57:23 -0800 (PST) X-BeenThere: patchwork-forward@linaro.org Received: by 10.140.92.240 with SMTP id b103ls516250qge.57.gmail; Thu, 20 Feb 2014 01:57:23 -0800 (PST) X-Received: by 10.236.118.212 with SMTP id l60mr1589652yhh.78.1392890243094; Thu, 20 Feb 2014 01:57:23 -0800 (PST) Received: from mail-ve0-f175.google.com (mail-ve0-f175.google.com [209.85.128.175]) by mx.google.com with ESMTPS id n48si3374029yhj.68.2014.02.20.01.57.23 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 20 Feb 2014 01:57:23 -0800 (PST) Received-SPF: neutral (google.com: 209.85.128.175 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) client-ip=209.85.128.175; Received: by mail-ve0-f175.google.com with SMTP id c14so1605971vea.20 for ; Thu, 20 Feb 2014 01:57:22 -0800 (PST) X-Received: by 10.220.192.71 with SMTP id dp7mr446540vcb.45.1392890242797; Thu, 20 Feb 2014 01:57:22 -0800 (PST) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patches@linaro.org Received: by 10.220.174.196 with SMTP id u4csp44400vcz; Thu, 20 Feb 2014 01:57:22 -0800 (PST) X-Received: by 10.194.109.134 with SMTP id hs6mr919860wjb.65.1392890241702; Thu, 20 Feb 2014 01:57:21 -0800 (PST) Received: from mail-we0-f178.google.com (mail-we0-f178.google.com [74.125.82.178]) by mx.google.com with ESMTPS id w3si4522867wij.4.2014.02.20.01.57.21 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 20 Feb 2014 01:57:21 -0800 (PST) Received-SPF: neutral (google.com: 74.125.82.178 is neither permitted nor denied by best guess record for domain of edward.nevill@linaro.org) client-ip=74.125.82.178; Received: by mail-we0-f178.google.com with SMTP id q59so1260795wes.37 for ; Thu, 20 Feb 2014 01:57:21 -0800 (PST) X-Received: by 10.194.82.105 with SMTP id h9mr1028493wjy.52.1392890241256; Thu, 20 Feb 2014 01:57:21 -0800 (PST) Received: from [10.0.7.5] ([88.98.47.97]) by mx.google.com with ESMTPSA id q15sm7588175wjw.18.2014.02.20.01.57.20 for (version=SSLv3 cipher=RC4-SHA bits=128/128); Thu, 20 Feb 2014 01:57:20 -0800 (PST) Message-ID: <1392890239.3317.30.camel@localhost.localdomain> Subject: RFR: Fix SEGV with volatile field accesses From: Edward Nevill Reply-To: edward.nevill@linaro.org To: "aarch64-port-dev@openjdk.java.net" Cc: Edward Nevill , patches@linaro.org Date: Thu, 20 Feb 2014 09:57:19 +0000 Organization: Linaro X-Mailer: Evolution 3.8.5 (3.8.5-2.fc19) Mime-Version: 1.0 X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: edward.nevill@linaro.org X-Original-Authentication-Results: mx.google.com; spf=neutral (google.com: 209.85.128.175 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Precedence: list Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org List-ID: X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , Hi, The following patch fixes an ongoing problem I have seen with volatile field accesses. The symptom I have seen is a SEGV with the PC pointing to an 'ldar' instruction. This has been quite unpredictable and difficult to reproduce, however I have managed to reliably reproduce it in specjbb2013, but I have also seen this fault occur in the JCK test harness. The method generating the SEGV is java/util/concurrent/ForkJoinTask.cancelIgnoringExceptions. Here is the source code for this. static final void cancelIgnoringExceptions(ForkJoinTask t) { if (t != null && t.status >= 0) { try { t.cancel(false); } catch (Throwable ignore) { } } } The code which generates the SEGV is the reference to t.status where status is a volatile field. And here is the generated code. Search for the marker <<<< LOOK HERE [Verified Entry Point] # {method} {0x0000007f8b718300} 'cancelIgnoringExceptions' '(Ljava/util/concurrent/ForkJoinTask;)V' in 'java/util/concurrent/ForkJoinTask' # parm0: c_rarg1:c_rarg1 = 'java/util/concurrent/ForkJoinTask' # [sp+0x30] (sp of caller) 0x0000007f9118f800: nop ; {no_reloc} 0x0000007f9118f804: orr x9, xzr, #0xffffffffffffc000 0x0000007f9118f808: ldr xzr, [sp,x9] 0x0000007f9118f80c: stp x29, x30, [sp,#-16]! 0x0000007f9118f810: sub sp, sp, #0x20 ;*synchronization entry ; - java.util.concurrent.ForkJoinTask::cancelIgnoringExceptions@-1 (line 508) <<< LOOK HERE 0x0000007f9118f814: add x8, x1, #0xc ; implicit exception: dispatches to 0x0000007f9118f840 0x0000007f9118f818: ldar w11, [x8] ;*getfield status ; - java.util.concurrent.ForkJoinTask::cancelIgnoringExceptions@5 (line 508) <<< LOOK HERE 0x0000007f9118f81c: cmp w11, #0x0 0x0000007f9118f820: b.lt 0x0000007f9118f82c ;*iflt ; - java.util.concurrent.ForkJoinTask::cancelIgnoringExceptions@8 (line 508) 0x0000007f9118f824: mov w2, wzr 0x0000007f9118f828: bl 0x0000007f910b1fe0 ; OopMap{off=44} ;*invokevirtual cancel ; - java.util.concurrent.ForkJoinTask::cancelIgnoringExceptions@13 (line 510) ; {optimized virtual_call} 0x0000007f9118f82c: add sp, sp, #0x20 0x0000007f9118f830: ldp x29, x30, [sp],#16 0x0000007f9118f834: adrp x8, 0x0000007fa5d75000 ; {poll_return} 0x0000007f9118f838: ldr wzr, [x8,#256] ; {poll_return} 0x0000007f9118f83c: ret 0x0000007f9118f840: str x1, [sp,#8] 0x0000007f9118f844: movn w1, #0x52 0x0000007f9118f848: bl 0x0000007f910b36e0 ; OopMap{[8]=Oop off=76} ;*ifnull ; - java.util.concurrent.ForkJoinTask::cancelIgnoringExceptions@1 (line 508) ; {runtime_call} 0x0000007f9118f84c: brk #0x3e7 ;*ifnull ; - java.util.concurrent.ForkJoinTask::cancelIgnoringExceptions@1 (line 508) 0x0000007f9118f850: .inst 0x00000000 ; undefined 0x0000007f9118f854: .inst 0x00000000 ; undefined 0x0000007f9118f858: .inst 0x00000000 ; undefined 0x0000007f9118f85c: .inst 0x00000000 ; undefined [Stub Code] 0x0000007f9118f860: ldr x12, 0x0000007f9118f7e0 ; {no_reloc} 0x0000007f9118f864: b 0x0000007f9118f864 [Exception Handler] 0x0000007f9118f868: b 0x0000007f910c3b20 ; {runtime_call} [Deopt Handler Code] 0x0000007f9118f86c: adr x30, 0x0000007f9118f86c 0x0000007f9118f870: b 0x0000007f910b3340 ; {runtime_call} 0x0000007f9118f874: .inst 0x00000000 ; undefined And here is the error report showing a SEGV on the ldar instruction above at address 0x0000007f9118f818. # # A fatal error has been detected by the Java Runtime Environment: # # SIGSEGV (0xb) at pc=0x0000007f9118f818, pid=3191, tid=547633492496 # # JRE version: OpenJDK Runtime Environment (8.0) (build 1.8.0-internal-ed_2014_02_18_14_48-b00) # Java VM: OpenJDK 64-Bit Server VM (25.0-b69 mixed mode linux-aarch64 compressed oops) # Problematic frame: # J 2 C2 java.util.concurrent.ForkJoinTask.cancelIgnoringExceptions(Ljava/util/concurrent/ForkJoinTask;)V (22 bytes) @ 0x0000007f9118f818 [0x0000007f9118f800+0x18] # # Failed to write core dump. Core dumps have been disabled. To enable core dumping, try "ulimit -c unlimited" before starting Java again # # An error report file with more information is saved as: # /home/ed/SPECjbb2013/hs_err_pid3191.log # # If you would like to submit a bug report, please visit: # http://bugreport.sun.com/bugreport/crash.jsp # And the register dump from hs_err_pid3191 shows siginfo:si_signo=SIGSEGV: si_errno=0, si_code=1 (SEGV_MAPERR), si_addr=0x000000000000000c Registers: R0=0x0000000000000000 R1=0x0000000000000000 <<< LOOK HERE R2=0x0000007f81b974c0 R3=0x0000000080400001 R4=0x000000000000001a R5=0x00000000c0000000 R6=0x000000000000deab R7=0x000000000000000c R8=0x000000000000000c <<< LOOK HERE R9=0xffffffffffffc000 R10=0x0000007fa59e6d84 R11=0x0000000000000000 R12=0x0000007f8b718300 R13=0x0000007f817fd500 R14=0x0000000000000000 R15=0x001c8186b8000000 R16=0x0000007fa5b066f8 R17=0x0000000006000111 R18=0x0000007fa026ecbf R19=0x00000000000000b8 R20=0x0000007f817fd528 R21=0x0000007fa5b773c0 R22=0x0000007f81b967d4 R23=0x0000007f817fe890 R24=0x0000007f817fd588 R25=0x0000000000000000 R26=0x0000007f81b971a0 R27=0x0000000000000000 R28=0x0000007fa07cb120 R29=0x0000007f817fd570 R30=0x0000007f91081d40 So in this instance 't' is null which results in x8 having the value '0xc'. This should cause a null pointer exception but instead causes a SEGV. The reason is that in the implicit null exception table the exception entry points to the first instruction of the two instruction sequence rather than pointing to the ldar. IE in the following. 0x0000007f9118f814: add x8, x1, #0xc ; implicit exception: dispatches to 0x0000007f9118f840 <<<< LOOK HERE implicit null exception table entry points to the add instruction above, not the ldar instruction. 0x0000007f9118f818: ldar w11, [x8] ;*getfield status ; - java.util.concurrent.ForkJoinTask::cancelIgnoringExceptions@5 (line 508) This is caused by the following in aarch64.ad enc_class aarch64_enc_ldar(iRegL dst, memory mem) %{ MOV_VOLATILE(as_Register($dst$$reg), $mem$$base, $mem$$index, $mem$$scale, $mem$$disp, rscratch1, ldar); %} Where MOV_VOLATILE is #define MOV_VOLATILE(REG, BASE, INDEX, SCALE, DISP, SCRATCH, INSN) \ MacroAssembler _masm(&cbuf); \ { \ Register base = as_Register(BASE); \ if (INDEX == -1) { \ __ lea(SCRATCH, Address(base, DISP)); \ } else { \ Register index_reg = as_Register(INDEX); \ if (DISP == 0) { \ __ lea(SCRATCH, Address(base, index_reg, Address::lsl(SCALE))); \ } else { \ __ lea(SCRATCH, Address(base, DISP)); \ __ lea(SCRATCH, Address(SCRATCH, index_reg, Address::lsl(SCALE))); \ } \ } \ __ INSN(REG, SCRATCH); \ } The above code may generate 1 or more instructions before the actual load. However there is an implicit assumption in src/share/vm/opto/output.cpp that the instruction encoding will generate precisely 1 instruction only. This assumption is contained in the code. // If this is a null check, then add the start of the previous instruction to the list else if( mach->is_MachNullCheck() ) { inct_starts[inct_cnt++] = previous_offset; Where 'previous_offset' is the offset of the last instruction encoding which in this case points to the add rather than the ldar. The solution I have adopted is (in the case of aarch64) to use 'current_offset - NativeInstruction::instruction_size' rather than 'previous_offset'. This is safe on aarch64 because a) All instructions are fixed size b) Sequences like the above always to an address calculation and then do the load/store as the last instruction c) On x86 we cannot do this because instructions are variable sized so the code must be conditionalised. I have tested this on specjbb2013 and I get a clean run which takes slightly over 2 hours. OK to push? Ed. --- CUT HERE --- exporting patch: # HG changeset patch # User Edward Nevill edward.nevill@linaro.org # Date 1392888023 0 # Thu Feb 20 09:20:23 2014 +0000 # Node ID e9706e7174f522754353bbe737c012221e909015 # Parent 9fb1040177d04e702d8e0d683d344989aaf61d46 Fix SEGV seen with volatile field accesses diff -r 9fb1040177d0 -r e9706e7174f5 src/share/vm/opto/output.cpp --- a/src/share/vm/opto/output.cpp Tue Feb 18 16:41:40 2014 +0000 +++ b/src/share/vm/opto/output.cpp Thu Feb 20 09:20:23 2014 +0000 @@ -1355,7 +1355,18 @@ // If this is a null check, then add the start of the previous instruction to the list else if( mach->is_MachNullCheck() ) { +#ifdef AARCH64 + /* aarch64 may issue multiple instruction sequences. + * The implicit exception must point to the load/store instruction. + * However, the load/store will always be the last instruction. + * Since instructions sequences are fixed size we can simply + * use current_offset - NativeInstruction::instruction_size + * x86 cannot do this because of variable size instructions. + */ + inct_starts[inct_cnt++] = current_offset - NativeInstruction::instruction_size; +#else inct_starts[inct_cnt++] = previous_offset; +#endif } // If this is a branch, then fill in the label with the target BB's label --- CUT HERE ---