Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8147645

get_ctrl_no_update() code is wrong

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: P3
    • Resolution: Fixed
    • Affects Version/s: 9
    • Fix Version/s: 9
    • Component/s: hotspot
    • Labels:
    • Subcomponent:
    • Resolved In Build:
      b106
    • CPU:
      generic
    • OS:
      generic

      Backports

        Description

        The problem is that somehow we start processing a dead control node (!n->in(0)). All control nodes should have in(0) not NULL unless it is dead and was cut from graph. So it is very rare case which we seems never hit before.

        The loop in get_ctrl_no_update() which goes through dead CFG nodes (lines 701-703) is wrong:

        698 Node *n = (Node*)(((intptr_t)_nodes[i->_idx]) & ~1);
        699 if (!n->in(0)) {
        700 // Skip dead CFG nodes
        701 do {
        702 n = (Node*)(((intptr_t)_nodes[n->_idx]) & ~1);
        703 } while (!n->in(0));
        704 n = find_non_split_ctrl(n);
        705 }

        _nodes[] points to IdealLoopTree* for control nodes and not to other control nodes so we can't use _nodes[] to scan previous control nodes.

        First, we need to find why graph still points to dead control node. And second, we should assert/guarantee (or bailout compilation) when we hit such case.
        1. Test.java
          0.6 kB
          Vladimir Kozlov

          Issue Links

            Activity

            Hide
            kvn Vladimir Kozlov added a comment -
            On 1/18/16 5:31 AM, Shafi Ahmad wrote:
            > Hi All,
            >
            > I am new and have little knowledge of IdealGraph IR as well as its creation/traversal and hence I need some help from
            > the experts for fixing this issue towards this I have asked few of my queries at the end of this mail.
            >
            > I am analyzing the crash and my findings are as follows-
            >
            > While building loop tree, the _nodes[] array is mapped by Node index [in PhaseIdealLoop::build_loop_tree()].
            >
            > In function PhaseIdealLoop::build_loop_tree_impl, **set_loop**is getting called with object of type ‘*IdealLoopTree **’.
            >
            > PhaseIdealLoop::set_loop (this=0x7fffb952de40, n=0x7fff9805db40, loop=0x7ffff0208dc0) at
            > /home/shafi/Java/jdk9/jdk9-dev/hotspot/src/share/vm/opto/loopnode.hpp:714
            >
            > 713 void set_loop( Node *n, IdealLoopTree *loop ) {
            >
            > *714 _nodes.map(n->_idx, (Node*)loop);*
            >
            > 715 }
            >
            > Here classes *Node* and *IdealLoopTree *are not related i.e. there is no derived-base class relationship,so this C like
            > **type-cast** is dangerous unless we take care at the time of accessin method **PhaseIdealLoop::get_ctrl_no_update**.
            >
            > Ideally while accessing this we have to type cast back to *‘IdealLoopTree **’ [in this particular case] and then we
            > should access the fields else we will get wrong data. In fact in our case same thing is happening.
            >
            > Node_Array::map (this=0x7fffb952de60, i=0, n=0x7ffff0208dc0) at
            > /home/shafi/Java/jdk9/jdk9-dev/hotspot/src/share/vm/opto/node.hpp:1414
            >
            > *1414 void map( uint i, Node *n ) { if( i>=_max ) grow(i); _nodes[i] = n; }*
            >
            > Above function fills the _nodes[] with different type of Nodes of Ideal Graph.
            >
            > #10 0x00007ffff649f1a0 in PhaseIdealLoop::get_ctrl_no_update (this=0x7fffb952f470, i=0x7fff9810cf80) at
            > /home/shafi/Java/jdk9/jdk9-dev/hotspot/src/share/vm/opto/loopnode.hpp:700
            >
            > 700 } while (!n->in(0));
            >
            > (gdb) l
            >
            > 695 Node *n = (Node*)(((intptr_t)_nodes[i->_idx]) & ~1);
            >
            > 696 if (!n->in(0)) {
            >
            > 697 // Skip dead CFG nodes
            >
            > 698 do {
            >
            > 699 *n = (Node*)(((intptr_t)_nodes[n->_idx]) & ~1);*
            >
            > 700 } while (!n->in(0));
            >
            > 701 n = find_non_split_ctrl(n);
            >
            > 702 }
            >
            > 703 return n;
            >
            > 704 }
            >
            > (gdb) p n
            >
            > $4 = (Node *) 0x7ffff02267c0
            >
            > (gdb) p *n
            >
            > $5 = {
            >
            > *_vptr.Node = 0x7ffff72ce950 <vtable for IdealLoopTree+16>, // ----------------------------(C)*
            >
            > * _in = 0xffff80000fdd983e, // ----------------------- (D)*
            >
            > _out = 0x0,
            >
            > _cnt = 4028779728,
            >
            > _max = 32767,
            >
            > _outcnt = 0,
            >
            > _outmax = 0,
            >
            > _idx = 4028786208,
            >
            > _parse_idx = 32767,
            >
            > _class_id = 37400,
            >
            > _flags = 38915,
            >
            > static NotAMachineReg = 4294901760,
            >
            > _debug_idx = 32767,
            >
            > _debug_orig = 0x7fff980359d8,
            >
            > _hash_lock = -1185746576,
            >
            > _last_del = 0x0,
            >
            > _del_tick = 4146653104
            >
            > }
            >
            > Here we are not type casting back to “*IdealLoopTree *”**.*** From *(C)* it is clear that this node ‘n’ is of type
            > ‘’*IdealLoopTree **”.
            >
            > Filed *‘_in’ *is not pointing to valid memory address and it crashes in below function.
            >
            > (gdb)
            >
            > #9 0x00007ffff5a5f3f7 in Node::in (this=0x7ffff02267c0, i=0) at
            > /home/shafi/Java/jdk9/jdk9-dev/hotspot/src/share/vm/opto/node.hpp:363
            >
            > *363 Node* in(uint i) const { assert(i < _max, "oob: i=%d, _max=%d", i, _max); return **_in**[i]; } // _in is
            > not pointing to valid memory address and it is crashing here. ------------ (A)*
            >
            > (gdb)
            >
            > For more details please see below few relevant gdb command output.
            >
            > (gdb) up
            >
            > #1 0x00007ffff64bd08c in PhaseIdealLoop::build_loop_tree_impl (this=0x7fffb952d2c0, n=0x7fff98043630, pre_order=11) at
            > /home/shafi/Java/jdk9/jdk9-dev/hotspot/src/share/vm/opto/loopnode.cpp:2884
            >
            > 2884 set_loop(m, l); // Set loop header to loop now
            >
            > (gdb) p l
            >
            > *$6 = (IdealLoopTree *) 0x7fff9806b3f0*
            >
            > (gdb) down
            >
            > #0 PhaseIdealLoop::set_loop (this=0x7fffb952d2c0, n=0x7fff98022150, loop=0x7fff9806b3f0) at
            > /home/shafi/Java/jdk9/jdk9-dev/hotspot/src/share/vm/opto/loopnode.hpp:713
            >
            > 713 void set_loop( Node *n, IdealLoopTree *loop ) {
            >
            > (gdb) n
            >
            > 714 _nodes.map(n->_idx, (Node*)loop);
            >
            > *(gdb) p sizeof(Node)*
            >
            > $7 = 88
            >
            > *(gdb) p sizeof(IdealLoopTree)*
            >
            > $8 = 168
            >
            > *(gdb) p *loop*
            >
            > $12 = {
            >
            > <ResourceObj> = {
            >
            > <AllocatedObj> = {
            >
            > *_vptr.AllocatedObj = 0x7ffff72ce950 <vtable for IdealLoopTree+16>*
            >
            > },
            >
            > members of ResourceObj:
            >
            > _allocation_t = {18446603337898218990, 0}
            >
            > },
            >
            > members of IdealLoopTree:
            >
            > _parent = 0x7fff9c0a8f20,
            >
            > _next = 0x0,
            >
            > _child = 0x7fff9c0aa870,
            >
            > _head = 0x7fff9c0971c8,
            >
            > _tail = 0x7fff9c093988,
            >
            > _phase = 0x7fffb952f4f0,
            >
            > _local_loop_unroll_limit = 0,
            >
            > _local_loop_unroll_factor = 0,
            >
            > _body = {
            >
            > <Node_Array> = {
            >
            > <ResourceObj> = {
            >
            > <AllocatedObj> = {
            >
            > _vptr.AllocatedObj = 0x7ffff728e7b0 <vtable for Node_List+16>
            >
            > },
            >
            > members of ResourceObj:
            >
            > _allocation_t = {18446603337898218911, 0}
            >
            > },
            >
            > members of Node_Array:
            >
            > _a = 0x7ffff01fd520,
            >
            > _max = 4,
            >
            > _nodes = 0x7fff9c0aaac0
            >
            > },
            >
            > members of Node_List:
            >
            > _cnt = 0
            >
            > },
            >
            > _nest = 1 '\001',
            >
            > _irreducible = 0 '\000',
            >
            > _has_call = 1 '\001',
            >
            > _has_sfpt = 0 '\000',
            >
            > _rce_candidate = 0 '\000',
            >
            > _safepts = 0x7fff9c0aaae0,
            >
            > _required_safept = 0x0,
            >
            > _allow_optimizations = true
            >
            > }
            >
            > (gdb) s
            >
            > Node_Array::map (this=0x7fffb952d2e0, i=0, n=0x7fff9806b3f0) at
            > /home/shafi/Java/jdk9/jdk9-dev/hotspot/src/share/vm/opto/node.hpp:1414
            >
            > 1414 void map( uint i, Node *n ) { if( i>=_max ) grow(i); _nodes[i] = n; }
            >
            > (gdb) p *n
            >
            > * _vptr.Node = 0x7ffff72ce950 <vtable for IdealLoopTree+16>, *
            >
            > * _in = 0xffff800063f555ee, *
            >
            > _out = 0x0,
            >
            > _cnt = 2617937696,
            >
            > _max = 32767,
            >
            > _outcnt = 0,
            >
            > _outmax = 0,
            >
            > _idx = 2617944176,
            >
            > _parse_idx = 32767,
            >
            > _class_id = 29128,
            >
            > _flags = 39945,
            >
            > static NotAMachineReg = 4294901760,
            >
            > _debug_idx = 32767,
            >
            > _debug_orig = 0x7fff9c093988,
            >
            > _hash_lock = -1185745680,
            >
            > _last_del = 0x0,
            >
            > _del_tick = 4146653104
            >
            > }
            >
            > (gdb)
            >
            > It will be great if some expert answer my following queries-
            >
            > 1.Why this un-related node is added to the _nodes array. Please note I have seen many such nodes are added to this list?
            >
            > 2.If at all we have to add to this list then why we are not type-casting back to original type?
            >
            > 3.Please correct me if I am wrong, signature of method “set_loop” should be “set_loop( Node *n, Node *loop )”. That way
            > we can avoid such problem.
            >
            > 4.As I am new so not sure about complete logic if someone help in understanding will be a great help.
            >
            > Thanks in advance.
            >
            > Regards,
            >
            > Shafi
            >
            Show
            kvn Vladimir Kozlov added a comment - On 1/18/16 5:31 AM, Shafi Ahmad wrote: > Hi All, > > I am new and have little knowledge of IdealGraph IR as well as its creation/traversal and hence I need some help from > the experts for fixing this issue towards this I have asked few of my queries at the end of this mail. > > I am analyzing the crash and my findings are as follows- > > While building loop tree, the _nodes[] array is mapped by Node index [in PhaseIdealLoop::build_loop_tree()]. > > In function PhaseIdealLoop::build_loop_tree_impl, **set_loop**is getting called with object of type ‘*IdealLoopTree **’. > > PhaseIdealLoop::set_loop (this=0x7fffb952de40, n=0x7fff9805db40, loop=0x7ffff0208dc0) at > /home/shafi/Java/jdk9/jdk9-dev/hotspot/src/share/vm/opto/loopnode.hpp:714 > > 713 void set_loop( Node *n, IdealLoopTree *loop ) { > > *714 _nodes.map(n->_idx, (Node*)loop);* > > 715 } > > Here classes *Node* and *IdealLoopTree *are not related i.e. there is no derived-base class relationship,so this C like > **type-cast** is dangerous unless we take care at the time of accessin method **PhaseIdealLoop::get_ctrl_no_update**. > > Ideally while accessing this we have to type cast back to *‘IdealLoopTree **’ [in this particular case] and then we > should access the fields else we will get wrong data. In fact in our case same thing is happening. > > Node_Array::map (this=0x7fffb952de60, i=0, n=0x7ffff0208dc0) at > /home/shafi/Java/jdk9/jdk9-dev/hotspot/src/share/vm/opto/node.hpp:1414 > > *1414 void map( uint i, Node *n ) { if( i>=_max ) grow(i); _nodes[i] = n; }* > > Above function fills the _nodes[] with different type of Nodes of Ideal Graph. > > #10 0x00007ffff649f1a0 in PhaseIdealLoop::get_ctrl_no_update (this=0x7fffb952f470, i=0x7fff9810cf80) at > /home/shafi/Java/jdk9/jdk9-dev/hotspot/src/share/vm/opto/loopnode.hpp:700 > > 700 } while (!n->in(0)); > > (gdb) l > > 695 Node *n = (Node*)(((intptr_t)_nodes[i->_idx]) & ~1); > > 696 if (!n->in(0)) { > > 697 // Skip dead CFG nodes > > 698 do { > > 699 *n = (Node*)(((intptr_t)_nodes[n->_idx]) & ~1);* > > 700 } while (!n->in(0)); > > 701 n = find_non_split_ctrl(n); > > 702 } > > 703 return n; > > 704 } > > (gdb) p n > > $4 = (Node *) 0x7ffff02267c0 > > (gdb) p *n > > $5 = { > > *_vptr.Node = 0x7ffff72ce950 <vtable for IdealLoopTree+16>, // ----------------------------(C)* > > * _in = 0xffff80000fdd983e, // ----------------------- (D)* > > _out = 0x0, > > _cnt = 4028779728, > > _max = 32767, > > _outcnt = 0, > > _outmax = 0, > > _idx = 4028786208, > > _parse_idx = 32767, > > _class_id = 37400, > > _flags = 38915, > > static NotAMachineReg = 4294901760, > > _debug_idx = 32767, > > _debug_orig = 0x7fff980359d8, > > _hash_lock = -1185746576, > > _last_del = 0x0, > > _del_tick = 4146653104 > > } > > Here we are not type casting back to “*IdealLoopTree *”**.*** From *(C)* it is clear that this node ‘n’ is of type > ‘’*IdealLoopTree **”. > > Filed *‘_in’ *is not pointing to valid memory address and it crashes in below function. > > (gdb) > > #9 0x00007ffff5a5f3f7 in Node::in (this=0x7ffff02267c0, i=0) at > /home/shafi/Java/jdk9/jdk9-dev/hotspot/src/share/vm/opto/node.hpp:363 > > *363 Node* in(uint i) const { assert(i < _max, "oob: i=%d, _max=%d", i, _max); return **_in**[i]; } // _in is > not pointing to valid memory address and it is crashing here. ------------ (A)* > > (gdb) > > For more details please see below few relevant gdb command output. > > (gdb) up > > #1 0x00007ffff64bd08c in PhaseIdealLoop::build_loop_tree_impl (this=0x7fffb952d2c0, n=0x7fff98043630, pre_order=11) at > /home/shafi/Java/jdk9/jdk9-dev/hotspot/src/share/vm/opto/loopnode.cpp:2884 > > 2884 set_loop(m, l); // Set loop header to loop now > > (gdb) p l > > *$6 = (IdealLoopTree *) 0x7fff9806b3f0* > > (gdb) down > > #0 PhaseIdealLoop::set_loop (this=0x7fffb952d2c0, n=0x7fff98022150, loop=0x7fff9806b3f0) at > /home/shafi/Java/jdk9/jdk9-dev/hotspot/src/share/vm/opto/loopnode.hpp:713 > > 713 void set_loop( Node *n, IdealLoopTree *loop ) { > > (gdb) n > > 714 _nodes.map(n->_idx, (Node*)loop); > > *(gdb) p sizeof(Node)* > > $7 = 88 > > *(gdb) p sizeof(IdealLoopTree)* > > $8 = 168 > > *(gdb) p *loop* > > $12 = { > > <ResourceObj> = { > > <AllocatedObj> = { > > *_vptr.AllocatedObj = 0x7ffff72ce950 <vtable for IdealLoopTree+16>* > > }, > > members of ResourceObj: > > _allocation_t = {18446603337898218990, 0} > > }, > > members of IdealLoopTree: > > _parent = 0x7fff9c0a8f20, > > _next = 0x0, > > _child = 0x7fff9c0aa870, > > _head = 0x7fff9c0971c8, > > _tail = 0x7fff9c093988, > > _phase = 0x7fffb952f4f0, > > _local_loop_unroll_limit = 0, > > _local_loop_unroll_factor = 0, > > _body = { > > <Node_Array> = { > > <ResourceObj> = { > > <AllocatedObj> = { > > _vptr.AllocatedObj = 0x7ffff728e7b0 <vtable for Node_List+16> > > }, > > members of ResourceObj: > > _allocation_t = {18446603337898218911, 0} > > }, > > members of Node_Array: > > _a = 0x7ffff01fd520, > > _max = 4, > > _nodes = 0x7fff9c0aaac0 > > }, > > members of Node_List: > > _cnt = 0 > > }, > > _nest = 1 '\001', > > _irreducible = 0 '\000', > > _has_call = 1 '\001', > > _has_sfpt = 0 '\000', > > _rce_candidate = 0 '\000', > > _safepts = 0x7fff9c0aaae0, > > _required_safept = 0x0, > > _allow_optimizations = true > > } > > (gdb) s > > Node_Array::map (this=0x7fffb952d2e0, i=0, n=0x7fff9806b3f0) at > /home/shafi/Java/jdk9/jdk9-dev/hotspot/src/share/vm/opto/node.hpp:1414 > > 1414 void map( uint i, Node *n ) { if( i>=_max ) grow(i); _nodes[i] = n; } > > (gdb) p *n > > * _vptr.Node = 0x7ffff72ce950 <vtable for IdealLoopTree+16>, * > > * _in = 0xffff800063f555ee, * > > _out = 0x0, > > _cnt = 2617937696, > > _max = 32767, > > _outcnt = 0, > > _outmax = 0, > > _idx = 2617944176, > > _parse_idx = 32767, > > _class_id = 29128, > > _flags = 39945, > > static NotAMachineReg = 4294901760, > > _debug_idx = 32767, > > _debug_orig = 0x7fff9c093988, > > _hash_lock = -1185745680, > > _last_del = 0x0, > > _del_tick = 4146653104 > > } > > (gdb) > > It will be great if some expert answer my following queries- > > 1.Why this un-related node is added to the _nodes array. Please note I have seen many such nodes are added to this list? > > 2.If at all we have to add to this list then why we are not type-casting back to original type? > > 3.Please correct me if I am wrong, signature of method “set_loop” should be “set_loop( Node *n, Node *loop )”. That way > we can avoid such problem. > > 4.As I am new so not sure about complete logic if someone help in understanding will be a great help. > > Thanks in advance. > > Regards, > > Shafi >
            Hide
            kvn Vladimir Kozlov added a comment -
            Hi Vladimir,

            Thank you for looking into it.
            Please find attached reproducer.

            Command:
              $javac Test.java
              $java Test

            This is reproducible with jdk8u-dev and jdku9-dev.

            I debugged with below java version [internal].

            JRE version: Java(TM) SE Runtime Environment (9.0) (build 9-internal+0-2015-12-17-102255.shafi.jdk9-dev)
            Java VM: Java HotSpot(TM) 64-Bit Server VM (9-internal+0-2015-12-17-102255.shafi.jdk9-dev, mixed mode, tiered, compressed oops, g1 gc, linux-amd64)

            Let me know if more info is required.

            Regards,
            Shafi
            Show
            kvn Vladimir Kozlov added a comment - Hi Vladimir, Thank you for looking into it. Please find attached reproducer. Command:   $javac Test.java   $java Test This is reproducible with jdk8u-dev and jdku9-dev. I debugged with below java version [internal]. JRE version: Java(TM) SE Runtime Environment (9.0) (build 9-internal+0-2015-12-17-102255.shafi.jdk9-dev) Java VM: Java HotSpot(TM) 64-Bit Server VM (9-internal+0-2015-12-17-102255.shafi.jdk9-dev, mixed mode, tiered, compressed oops, g1 gc, linux-amd64) Let me know if more info is required. Regards, Shafi
            Hide
            hgupdate HG Updates added a comment -
            URL: http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/rev/796b8077f6e6
            User: roland
            Date: 2016-02-03 14:08:38 +0000
            Show
            hgupdate HG Updates added a comment - URL: http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/rev/796b8077f6e6 User: roland Date: 2016-02-03 14:08:38 +0000
            Hide
            hgupdate HG Updates added a comment -
            URL: http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/796b8077f6e6
            User: lana
            Date: 2016-02-17 20:42:36 +0000
            Show
            hgupdate HG Updates added a comment - URL: http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/796b8077f6e6 User: lana Date: 2016-02-17 20:42:36 +0000

              People

              • Assignee:
                roland Roland Westrelin
                Reporter:
                kvn Vladimir Kozlov
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: