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

Suspicious logic in C2

    XMLWordPrintable

Details

    Description

      While working on JDK-8298705, ASan revealed some extremely suspicious logic in C2 regarding PhaseTransform and its derived classes, VectorSet, Node_List and its derived classes, Type_Array, and NodeHash. Additionally with Node::destruct.

      There are constructors on PhaseTransform and its derive classes which copy the aforementioned classes by value, resulting in VectorSet, Node_List, Type_Array, and NodeHash which point to the exact same data. This is suspicious because any modifications made by one PhaseTransform may or may not be visible to the other, depending on whether the underlying storage was resized or not. If it was, one PhaseTransform will be looking at the old copy while the other will have the new copy. Additionally in compile.cpp `igvn = ccp;` exists which copies PhaseCCP into PhaseGVN by value. PhaseCCP is never referenced after, so that seems acceptable, but its suspicious none-the-less due to the overriding of the other mentioned classes by value. VectorSet also has a destructor which attempts to release memory to the arena, but since VectorSet is copied by value multiple times this memory will attempt to be freed multiple times, potentially leading to corruption depending on luck.

      Lastly I discovered that even after Node::destruct is called, some pointers to the node still exist in some copies of worklists in some PhaseTransform. This is suspicious because Node::destruct attempts to return memory to the arena. If it did, some PhaseTransform could be looking at something entirely different as the memory has been overridden. It is hard to tell if the affected PhaseTransform are actually used again.

      All in all this seems extremely suspicious and potentially brittle depending on what is changed. Should this be looked at more? I am not super familiar with C2 to know what was intentional vs. what was not.

      Happy to discuss more.

      Attachments

        Issue Links

          Activity

            People

              Unassigned Unassigned
              jcking Justin King
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

                Created:
                Updated: