[optimizing] Add memory barriers in constructors when needed

If a class has final fields we must add a memory barrier before
returning from constructor. This makes sure the fields are visible to
other threads.

Bug: 19851497
Change-Id: If8c485092fc512efb9636cd568cb0543fb27688e

8 files changed, 72 insertions, 0 deletions

diff --git a/compiler/optimizing/builder.cc b/compiler/optimizing/builder.cc
index 8a64d81485..fff2906a04 100644
--- a/compiler/optimizing/builder.cc
+++ b/compiler/optimizing/builder.cc
@@ -520,8 +520,19 @@ void HGraphBuilder::Binop_22b(const Instruction& instruction, bool reverse) {
   UpdateLocal(instruction.VRegA(), current_block_->GetLastInstruction());
 }
 
+static bool RequiresConstructorBarrier(const DexCompilationUnit& cu, const CompilerDriver& driver) {
+  Thread* self = Thread::Current();
+  return cu.IsConstructor()
+      && driver.RequiresConstructorBarrier(self, cu.GetDexFile(), cu.GetClassDefIndex());
+}
+
 void HGraphBuilder::BuildReturn(const Instruction& instruction, Primitive::Type type) {
   if (type == Primitive::kPrimVoid) {
+    // Note that we might insert redundant barriers when inlining `super` calls.
+    // TODO: add a data flow analysis to get rid of duplicate barriers.
+    if (RequiresConstructorBarrier(*dex_compilation_unit_, *compiler_driver_)) {
+      current_block_->AddInstruction(new (arena_) HMemoryBarrier(kStoreStore));
+    }
     current_block_->AddInstruction(new (arena_) HReturnVoid());
   } else {
     HInstruction* value = LoadLocal(instruction.VRegA(), type);
diff --git a/compiler/optimizing/code_generator_arm.cc b/compiler/optimizing/code_generator_arm.cc
index 2ea920310a..037a4aef4d 100644
--- a/compiler/optimizing/code_generator_arm.cc
+++ b/compiler/optimizing/code_generator_arm.cc
@@ -1214,6 +1214,14 @@ void InstructionCodeGeneratorARM::VisitDoubleConstant(HDoubleConstant* constant)
   UNUSED(constant);
 }
 
+void LocationsBuilderARM::VisitMemoryBarrier(HMemoryBarrier* memory_barrier) {
+  memory_barrier->SetLocations(nullptr);
+}
+
+void InstructionCodeGeneratorARM::VisitMemoryBarrier(HMemoryBarrier* memory_barrier) {
+  GenerateMemoryBarrier(memory_barrier->GetBarrierKind());
+}
+
 void LocationsBuilderARM::VisitReturnVoid(HReturnVoid* ret) {
   ret->SetLocations(nullptr);
 }
diff --git a/compiler/optimizing/code_generator_arm64.cc b/compiler/optimizing/code_generator_arm64.cc
index efc41e7c06..779340dfa5 100644
--- a/compiler/optimizing/code_generator_arm64.cc
+++ b/compiler/optimizing/code_generator_arm64.cc
@@ -2430,6 +2430,14 @@ void InstructionCodeGeneratorARM64::VisitRem(HRem* rem) {
   }
 }
 
+void LocationsBuilderARM64::VisitMemoryBarrier(HMemoryBarrier* memory_barrier) {
+  memory_barrier->SetLocations(nullptr);
+}
+
+void InstructionCodeGeneratorARM64::VisitMemoryBarrier(HMemoryBarrier* memory_barrier) {
+  GenerateMemoryBarrier(memory_barrier->GetBarrierKind());
+}
+
 void LocationsBuilderARM64::VisitReturn(HReturn* instruction) {
   LocationSummary* locations = new (GetGraph()->GetArena()) LocationSummary(instruction);
   Primitive::Type return_type = instruction->InputAt(0)->GetType();
diff --git a/compiler/optimizing/code_generator_x86.cc b/compiler/optimizing/code_generator_x86.cc
index 879216d59b..6ac4ad798c 100644
--- a/compiler/optimizing/code_generator_x86.cc
+++ b/compiler/optimizing/code_generator_x86.cc
@@ -1120,6 +1120,14 @@ void InstructionCodeGeneratorX86::VisitDoubleConstant(HDoubleConstant* constant)
   UNUSED(constant);
 }
 
+void LocationsBuilderX86::VisitMemoryBarrier(HMemoryBarrier* memory_barrier) {
+  memory_barrier->SetLocations(nullptr);
+}
+
+void InstructionCodeGeneratorX86::VisitMemoryBarrier(HMemoryBarrier* memory_barrier) {
+  GenerateMemoryBarrier(memory_barrier->GetBarrierKind());
+}
+
 void LocationsBuilderX86::VisitReturnVoid(HReturnVoid* ret) {
   ret->SetLocations(nullptr);
 }
diff --git a/compiler/optimizing/code_generator_x86_64.cc b/compiler/optimizing/code_generator_x86_64.cc
index a3d3490357..65b128a640 100644
--- a/compiler/optimizing/code_generator_x86_64.cc
+++ b/compiler/optimizing/code_generator_x86_64.cc
@@ -1145,6 +1145,14 @@ void InstructionCodeGeneratorX86_64::VisitDoubleConstant(HDoubleConstant* consta
   UNUSED(constant);
 }
 
+void LocationsBuilderX86_64::VisitMemoryBarrier(HMemoryBarrier* memory_barrier) {
+  memory_barrier->SetLocations(nullptr);
+}
+
+void InstructionCodeGeneratorX86_64::VisitMemoryBarrier(HMemoryBarrier* memory_barrier) {
+  GenerateMemoryBarrier(memory_barrier->GetBarrierKind());
+}
+
 void LocationsBuilderX86_64::VisitReturnVoid(HReturnVoid* ret) {
   ret->SetLocations(nullptr);
 }
diff --git a/compiler/optimizing/dead_code_elimination.cc b/compiler/optimizing/dead_code_elimination.cc
index fc3dd01ef5..94990402e5 100644
--- a/compiler/optimizing/dead_code_elimination.cc
+++ b/compiler/optimizing/dead_code_elimination.cc
@@ -38,6 +38,7 @@ void HDeadCodeElimination::Run() {
       if (!inst->HasSideEffects()
           && !inst->CanThrow()
           && !inst->IsSuspendCheck()
+          && !inst->IsMemoryBarrier()  // If we added an explicit barrier then we should keep it.
           && !inst->HasUses()) {
         block->RemoveInstruction(inst);
       }
diff --git a/compiler/optimizing/graph_visualizer.cc b/compiler/optimizing/graph_visualizer.cc
index 4c283788b5..ca9cbc3d01 100644
--- a/compiler/optimizing/graph_visualizer.cc
+++ b/compiler/optimizing/graph_visualizer.cc
@@ -192,6 +192,10 @@ class HGraphVisualizerPrinter : public HGraphVisitor {
     output_ << " " << phi->GetRegNumber();
   }
 
+  void VisitMemoryBarrier(HMemoryBarrier* barrier) OVERRIDE {
+    output_ << " " << barrier->GetBarrierKind();
+  }
+
   bool IsPass(const char* name) {
     return strcmp(pass_name_, name) == 0;
   }
diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h
index 649038b532..c3c35acb7a 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h
@@ -19,6 +19,7 @@
 
 #include "base/arena_containers.h"
 #include "base/arena_object.h"
+#include "dex/compiler_enums.h"
 #include "entrypoints/quick/quick_entrypoints_enum.h"
 #include "handle.h"
 #include "handle_scope.h"
@@ -718,6 +719,7 @@ class HLoopInformationOutwardIterator : public ValueObject {
   M(LoadString, Instruction)                                            \
   M(Local, Instruction)                                                 \
   M(LongConstant, Constant)                                             \
+  M(MemoryBarrier, Instruction)                                         \
   M(MonitorOperation, Instruction)                                      \
   M(Mul, BinaryOperation)                                               \
   M(Neg, UnaryOperation)                                                \
@@ -908,6 +910,12 @@ class HUserRecord : public ValueObject {
   HUseListNode<T>* use_node_;
 };
 
+// TODO: Add better documentation to this class and maybe refactor with more suggestive names.
+// - Has(All)SideEffects suggests that all the side effects are present but only ChangesSomething
+//   flag is consider.
+// - DependsOn suggests that there is a real dependency between side effects but it only
+//   checks DependendsOnSomething flag.
+//
 // Represents the side effects an instruction may have.
 class SideEffects : public ValueObject {
  public:
@@ -3437,6 +3445,22 @@ class HCheckCast : public HTemplateInstruction<2> {
   DISALLOW_COPY_AND_ASSIGN(HCheckCast);
 };
 
+class HMemoryBarrier : public HTemplateInstruction<0> {
+ public:
+  explicit HMemoryBarrier(MemBarrierKind barrier_kind)
+      : HTemplateInstruction(SideEffects::None()),
+        barrier_kind_(barrier_kind) {}
+
+  MemBarrierKind GetBarrierKind() { return barrier_kind_; }
+
+  DECLARE_INSTRUCTION(MemoryBarrier);
+
+ private:
+  const MemBarrierKind barrier_kind_;
+
+  DISALLOW_COPY_AND_ASSIGN(HMemoryBarrier);
+};
+
 class HMonitorOperation : public HTemplateInstruction<1> {
  public:
   enum OperationKind {