From 57ed5c25d2d7451ac54a6098ad73beee81491328 Mon Sep 17 00:00:00 2001
From: Victor Ding <dvictor@ca.ibm.com>
Date: Fri, 5 Oct 2018 11:15:06 -0400
Subject: [PATCH] Fix for incorrectly marked collected reference on X86

The result register of TR::aiadd/TR::aladd was incorrectly marked as a
collected reference, when the aiadd/aladd node is not internal pointer.
As a result, an invalid GC map entry may be introduced and causes GC assertions.
The result of TR::aiadd/TR::aladd can never be a collected reference, fixing it.

Signed-off-by: Victor Ding <dvictor@ca.ibm.com>
---
 .../x/codegen/BinaryCommutativeAnalyser.cpp   | 23 ++++---------------
 compiler/x/codegen/BinaryEvaluator.cpp        | 17 +-------------
 2 files changed, 6 insertions(+), 34 deletions(-)

diff --git a/compiler/x/codegen/BinaryCommutativeAnalyser.cpp b/compiler/x/codegen/BinaryCommutativeAnalyser.cpp
index 0141339d344..1284cd70796 100644
--- a/compiler/x/codegen/BinaryCommutativeAnalyser.cpp
+++ b/compiler/x/codegen/BinaryCommutativeAnalyser.cpp
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2016 IBM Corp. and others
+ * Copyright (c) 2000, 2018 IBM Corp. and others
  *
  * This program and the accompanying materials are made available under
  * the terms of the Eclipse Public License 2.0 which accompanies this
@@ -860,30 +860,17 @@ TR::Register *TR_X86BinaryCommutativeAnalyser::integerAddAnalyserImpl(TR::Node
       }
    else if (getCopyRegs())
       {
-      TR::Register *tempReg;
+      TR::Register* tempReg = _cg->allocateRegister();
       if (firstRegister->containsCollectedReference() ||
           secondRegister->containsCollectedReference() ||
           firstRegister->containsInternalPointer() ||
           secondRegister->containsInternalPointer())
          {
-         if (root->isInternalPointer())
+         if (root->isInternalPointer() && root->getPinningArrayPointer())
             {
-            tempReg = _cg->allocateRegister();
-            if (root->getPinningArrayPointer())
-               {
-               tempReg->setContainsInternalPointer();
-               tempReg->setPinningArrayPointer(root->getPinningArrayPointer());
-               }
+            tempReg->setContainsInternalPointer();
+            tempReg->setPinningArrayPointer(root->getPinningArrayPointer());
             }
-         else if (comp->generateArraylets() && root->getOpCodeValue() == TR::aiadd)
-            // arraylets: aiadd is technically internal pointer into spine object, but isn't marked as internal pointer
-            tempReg = _cg->allocateRegister();
-         else
-            tempReg = _cg->allocateCollectedReferenceRegister();
-         }
-      else
-         {
-         tempReg = _cg->allocateRegister();
          }
 
       targetRegister = tempReg;
diff --git a/compiler/x/codegen/BinaryEvaluator.cpp b/compiler/x/codegen/BinaryEvaluator.cpp
index a6b0f0e56a9..2538927f442 100644
--- a/compiler/x/codegen/BinaryEvaluator.cpp
+++ b/compiler/x/codegen/BinaryEvaluator.cpp
@@ -789,23 +789,8 @@ TR::Register *OMR::X86::TreeEvaluator::integerAddEvaluator(TR::Node *node, TR::C
                (internalPointerMismatch || targetRegister->containsCollectedReference()))))
             {
             TR::Register *firstOperandReg = targetRegister;
+            targetRegister = cg->allocateRegister();
 
-            // For a non-internal pointer TR::aiadd created when merging news (for example)
-            // commoning is permissible across a GC point; however the register
-            // needs to be marked as a collected reference for GC to behave correctly;
-            // note that support for internal pointer TR::aiadd (e.g. array access) is not present
-            // still
-            //
-            if (targetRegister->containsCollectedReference() &&
-                (node->getOpCode().isArrayRef()) &&
-               !node->isInternalPointer())
-               {
-               targetRegister = cg->allocateCollectedReferenceRegister();
-               }
-            else
-               {
-               targetRegister = cg->allocateRegister();
-               }
             // comp()->useCompressedPointers
             // ladd
             //    ==>iu2l