[11u] RFR(S): 8223266: PPC64: Check for branch to illegal address before checking for mem serialization

Doerr, Martin martin.doerr at sap.com
Mon May 6 08:11:44 UTC 2019


Hi Gustavo,

thanks for improving the comment. V2 looks good. Testing is also completed.

I had missed that the jdk11u-fix-request and approval is not yet there.
Can you request it, please?
We can push it once it's approved.

Best regards,
Martin


-----Original Message-----
From: Gustavo Romero <gromero at linux.vnet.ibm.com> 
Sent: Freitag, 3. Mai 2019 18:09
To: Doerr, Martin <martin.doerr at sap.com>; jdk-updates-dev at openjdk.java.net
Subject: Re: [11u] RFR(S): 8223266: PPC64: Check for branch to illegal address before checking for mem serialization

Hi Martin,

On 05/03/2019 10:06 AM, Doerr, Martin wrote:
> thanks for addressing this issue.
> 
> The fix looks good to me.

Thanks a lot for reviewing it.


> I'd appreciate if you could use a term in the comment like quoted "Data Storage Interrupt" such that somebody reading the code has a change to find a description of these bits.

Sure. It makes the term "searchable" in the ISA. Done.

  
> Also, please update copyrights before pushing.
> I don't need for a new webrev for that.
  
> I think it can get pushed once Götz' testing has completed.

Copyrights updated. I think I can't push to jdk11u-dev nor to jdk11u
because I'm not a jdk-updates project Committer, so I'll need a sponsor,
hence I provide a new webrev:

Webrev: http://cr.openjdk.java.net/~gromero/8223266/v2/

BTW, to get consistent with "Data Stotage Interrupt" comment in this
change the following text must be change on jdk/jdk tip and downport. I
can take care of downporting it if you are fine with that change. It looks
like "%note os_trap_1" must be cleaned-up too:

diff -r e9da84f26908 src/hotspot/os_cpu/linux_ppc/os_linux_ppc.cpp
--- a/src/hotspot/os_cpu/linux_ppc/os_linux_ppc.cpp     Thu May 02 18:01:23 2019 -0400
+++ b/src/hotspot/os_cpu/linux_ppc/os_linux_ppc.cpp     Fri May 03 11:58:59 2019 -0400
@@ -302,7 +302,6 @@
    address stub = NULL;
    address pc   = NULL;
  
-  //%note os_trap_1
    if (info != NULL && uc != NULL && thread != NULL) {
      pc = (address) os::Linux::ucontext_get_pc(uc);
  
@@ -311,17 +310,17 @@
        // si_addr may not be valid due to a bug in the linux-ppc64 kernel (see
        // comment below). Use get_stack_bang_address instead of si_addr.
        // If SIGSEGV is caused due to a branch to an invalid address an
-      // "Instruction Storage" interruption is generated and 'pc' (NIP) already
+      // "Instruction Storage Interrupt" is generated and 'pc' (NIP) already
        // contains the invalid address. Otherwise, the SIGSEGV is caused due to
        // load/store instruction trying to load/store from/to an invalid address
-      // and causing a "Data Storage" interruption, so we inspect the intruction
+      // and causing a "Data Storage Interrupt", so we inspect the intruction
        // in order to extract the faulty data addresss.
        address addr;
        if ((ucontext_get_trap(uc) & 0x0F00 /* no IRQ reply bits */) == 0x0400) {
-        // Instruction interruption
+        // Instruction Storage Interrupt (ISI)
          addr = pc;
        } else {
-        // Data interruption (0x0300): extract faulty data address
+        // Data Storage Interrupt (DSI), i.e. 0x0300: extract faulty data address
          addr = ((NativeInstruction*)pc)->get_stack_bang_address(uc);
        }


Thank you!

Best regards,
Gustavo



More information about the jdk-updates-dev mailing list