Re: Crash due to bad oop map

[Denghui Dong]

Hi @dean-long,

I tried, and it works.

Another way is to skip the processing of MachTemp when building the oop map. Do you think this is the correct way?

Denghui Dong


------------------------------------------------------------------
From:dean.long <dean.long at oracle.com>
Send Time:2022年7月22日(星期五) 11:28
To:undefined <undefined>; undefined <undefined>
Subject:Re: Crash due to bad oop map

Hi Denghui Dong.  This looks like JDK-8051805 
(https://bugs.openjdk.org/browse/JDK-8051805).  As you discovered, it is 
triggered when RegN is used as a TEMP.  As a work-around, have you tried 
using rRegI instead of rRegN for your TEMPs?

dl

On 7/21/22 12:11 AM, Denghui Dong wrote:
> Hi team,
> 
> We encountered a crash due to a bad oop map.
> The following steps can quickly reproduce the problem on JDK master on 
> Linux x64:
> 1. Modify z_x86_64.ad and add 3 temporary registers of type rRegN to 
> zLoadP(If this change is illegal, please let me know)
> ```
> diff --git a/src/hotspot/cpu/x86/gc/z/z_x86_64.ad 
> b/src/hotspot/cpu/x86/gc/z/z_x86_64.ad
> index f3e19b41733..129c93d3da8 100644
> --- a/src/hotspot/cpu/x86/gc/z/z_x86_64.ad
> +++ b/src/hotspot/cpu/x86/gc/z/z_x86_64.ad
> @@ -63,11 +63,11 @@ static void z_load_barrier_cmpxchg(MacroAssembler& 
> _masm, const MachNode* node,
> %}
> // Load Pointer
> -instruct zLoadP(rRegP dst, memory mem, rFlagsReg cr)
> +instruct zLoadP(rRegP dst, memory mem, rRegN tmp1, rRegN tmp2, rRegN 
> tmp3, rFlagsReg cr)
> %{
>    predicate(UseZGC && n->as_Load()->barrier_data() != 0);
>    match(Set dst (LoadP mem));
> -  effect(KILL cr, TEMP dst);
> +  effect(KILL cr, TEMP dst, TEMP tmp1, TEMP tmp2, TEMP tmp3);
>    ins_cost(125);
> ```
> 2. Run the following code 
> (./build/linux-x86_64-server-fastdebug/images/jdk/bin/java -XX:-Inline 
> -XX:CompileCommand=compileonly,Test::call -XX:+PrintAssembly 
> -XX:PrintIdealGraphLevel=2 -XX:PrintIdealGraphFile=call.xml -XX:+UseZGC 
> -XX:+UseNewCode -XX:+PrintGC -Xmx500m -Xms500m -XX:-Inline Test)
> ```
> import java.util.concurrent.locks.Lock;
> import java.util.concurrent.locks.ReentrantLock;
> class Test {
>    private Lock lock = new ReentrantLock();
>    public static void main(String... args) throws Exception {
>      new Thread(() -> {
>        while (true) {
>          byte[] b = new byte[10 * 1024 * 1024];
>        }
>      }).start();
>      while (true) {
>        new Test().call(() -> { new Object(); });
>      }
>    }
>    public void call(Runnable cb) {
>      lock.lock();
>      try {
>        cb.run();
>      } finally {
>        lock.unlock();
>      }
>    }
> }
> ```
> 
> It can be observed through the IGV Final Code that zLoadP(202, 204) and 
> a MachTemp(81) it uses are placed in different blocks. When processing 
> CallStaticJavaDirect(74) in the c2 buildOopMap step, MachTemp(81) is 
> considered to be alive. Because the register type specified by the above 
> modification is rRegN (if it is specified as rRegI or rRegP, no crash 
> will occur), so the type of MachTemp(81) is narrowOop, which will 
> eventually be added to the oopMap of CallStaticJavaDirect(74). But 
> logically, zLoadP doesn't depend on the original value of MachTemp(81), 
> so I don't think it should be added to oopMap.
> 
> I put the ideal graph file to http://cr.openjdk.java.net/~ddong/call.xml 
> <http://cr.openjdk.java.net/~ddong/call.xml>
> 
> I think the problem may lie in two places:
> 1. gcm and regAlloc do not put MachTemp(81) in the correct location
> 2. buildOopMap should not treat MachTemp as Oop because its original 
> value is meaningless
> 
> I'm not a JIT expert and sorry if there is anything unnatural or 
> non-standard in the above description.
> 
> Any input is appreciated.
> 
> Denghui Dong
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-compiler-dev/attachments/20220722/98245c25/attachment-0001.htm>