Add memory barrier for final instance fields.

Change-Id: I1a02f0f75e974f4c84d61254da05480c20ff881c

6 files changed, 74 insertions, 27 deletions

diff --git a/src/compiler.cc b/src/compiler.cc
index b125fdf68c..aa2ec4012a 100644
--- a/src/compiler.cc
+++ b/src/compiler.cc
@@ -285,6 +285,7 @@ Compiler::Compiler(CompilerBackend compiler_backend, InstructionSet instruction_
                    bool dump_stats, bool dump_timings)
     : compiler_backend_(compiler_backend),
       instruction_set_(instruction_set),
+      freezing_constructor_lock_("freezing constructor lock"),
       compiled_classes_lock_("compiled classes lock"),
       compiled_methods_lock_("compiled method lock"),
       compiled_invoke_stubs_lock_("compiled invoke stubs lock"),
@@ -500,8 +501,8 @@ void Compiler::CompileOne(const AbstractMethod* method) {
   DCHECK(!Runtime::Current()->IsStarted());
   Thread* self = Thread::Current();
   jobject class_loader;
-  const DexCache* dex_cache;
   const DexFile* dex_file;
+  uint32_t class_def_idx;
   {
     ScopedObjectAccessUnchecked soa(self);
     ScopedLocalRef<jobject>
@@ -509,8 +510,9 @@ void Compiler::CompileOne(const AbstractMethod* method) {
                     soa.AddLocalReference<jobject>(method->GetDeclaringClass()->GetClassLoader()));
     class_loader = soa.Env()->NewGlobalRef(local_class_loader.get());
     // Find the dex_file
-    dex_cache = method->GetDeclaringClass()->GetDexCache();
-    dex_file = dex_cache->GetDexFile();
+    MethodHelper mh(method);
+    dex_file = &mh.GetDexFile();
+    class_def_idx = mh.GetClassDefIndex();
   }
   self->TransitionFromRunnableToSuspended(kNative);
 
@@ -524,7 +526,7 @@ void Compiler::CompileOne(const AbstractMethod* method) {
   uint32_t method_idx = method->GetDexMethodIndex();
   const DexFile::CodeItem* code_item = dex_file->GetCodeItem(method->GetCodeItemOffset());
   CompileMethod(code_item, method->GetAccessFlags(), method->GetInvokeType(),
-                method_idx, class_loader, *dex_file);
+                class_def_idx, method_idx, class_loader, *dex_file);
 
   self->GetJniEnv()->DeleteGlobalRef(class_loader);
 
@@ -1124,7 +1126,14 @@ static void ResolveClassFieldsAndMethods(const CompilationContext* context, size
     }
     it.Next();
   }
+  // If an instance field is final then we need to have a barrier on the return, static final
+  // fields are assigned within the lock held for class initialization.
+  bool requires_constructor_barrier = false;
   while (it.HasNextInstanceField()) {
+    if ((it.GetMemberAccessFlags() & kAccFinal) != 0) {
+      requires_constructor_barrier = true;
+    }
+
     Field* field = class_linker->ResolveField(dex_file, it.GetMemberIndex(), dex_cache,
                                               class_loader, false);
     if (field == NULL) {
@@ -1133,9 +1142,14 @@ static void ResolveClassFieldsAndMethods(const CompilationContext* context, size
     }
     it.Next();
   }
+  if (requires_constructor_barrier) {
+    context->GetCompiler()->AddRequiresConstructorBarrier(soa.Self(), context->GetDexFile(),
+                                                          class_def_index);
+  }
   while (it.HasNextDirectMethod()) {
     AbstractMethod* method = class_linker->ResolveMethod(dex_file, it.GetMemberIndex(), dex_cache,
-                                                 class_loader, NULL, it.GetMethodInvokeType(class_def));
+                                                         class_loader, NULL,
+                                                         it.GetMethodInvokeType(class_def));
     if (method == NULL) {
       CHECK(self->IsExceptionPending());
       self->ClearException();
@@ -1144,7 +1158,8 @@ static void ResolveClassFieldsAndMethods(const CompilationContext* context, size
   }
   while (it.HasNextVirtualMethod()) {
     AbstractMethod* method = class_linker->ResolveMethod(dex_file, it.GetMemberIndex(), dex_cache,
-                                                 class_loader, NULL, it.GetMethodInvokeType(class_def));
+                                                         class_loader, NULL,
+                                                         it.GetMethodInvokeType(class_def));
     if (method == NULL) {
       CHECK(self->IsExceptionPending());
       self->ClearException();
@@ -1570,8 +1585,8 @@ void Compiler::CompileClass(const CompilationContext* context, size_t class_def_
     }
     previous_direct_method_idx = method_idx;
     context->GetCompiler()->CompileMethod(it.GetMethodCodeItem(), it.GetMemberAccessFlags(),
-                                          it.GetMethodInvokeType(class_def), method_idx,
-                                          class_loader, dex_file);
+                                          it.GetMethodInvokeType(class_def), class_def_index,
+                                          method_idx, class_loader, dex_file);
     it.Next();
   }
   // Compile virtual methods
@@ -1586,8 +1601,8 @@ void Compiler::CompileClass(const CompilationContext* context, size_t class_def_
     }
     previous_virtual_method_idx = method_idx;
     context->GetCompiler()->CompileMethod(it.GetMethodCodeItem(), it.GetMemberAccessFlags(),
-                                          it.GetMethodInvokeType(class_def), method_idx,
-                                          class_loader, dex_file);
+                                          it.GetMethodInvokeType(class_def), class_def_index,
+                                          method_idx, class_loader, dex_file);
     it.Next();
   }
   DCHECK(!it.HasNext());
@@ -1609,8 +1624,8 @@ static std::string MakeInvokeStubKey(bool is_static, const char* shorty) {
 }
 
 void Compiler::CompileMethod(const DexFile::CodeItem* code_item, uint32_t access_flags,
-                             InvokeType invoke_type, uint32_t method_idx, jobject class_loader,
-                             const DexFile& dex_file) {
+                             InvokeType invoke_type, uint32_t class_def_idx, uint32_t method_idx,
+                             jobject class_loader, const DexFile& dex_file) {
   CompiledMethod* compiled_method = NULL;
   uint64_t start_ns = NanoTime();
 
@@ -1619,8 +1634,8 @@ void Compiler::CompileMethod(const DexFile::CodeItem* code_item, uint32_t access
     CHECK(compiled_method != NULL);
   } else if ((access_flags & kAccAbstract) != 0) {
   } else {
-    compiled_method = (*compiler_)(*this, code_item, access_flags, invoke_type, method_idx,
-                                   class_loader, dex_file);
+    compiled_method = (*compiler_)(*this, code_item, access_flags, invoke_type, class_def_idx,
+                                   method_idx, class_loader, dex_file);
     CHECK(compiled_method != NULL) << PrettyMethod(method_idx, dex_file);
   }
   uint64_t duration_ns = NanoTime() - start_ns;
@@ -1748,4 +1763,17 @@ void Compiler::SetBitcodeFileName(std::string const& filename) {
   set_bitcode_file_name(*this, filename);
 }
 
+
+void Compiler::AddRequiresConstructorBarrier(Thread* self, const DexFile* dex_file,
+                                             size_t class_def_index) {
+  MutexLock mu(self, freezing_constructor_lock_);
+  freezing_constructor_classes_.insert(ClassReference(dex_file, class_def_index));
+}
+
+bool Compiler::RequiresConstructorBarrier(Thread* self, const DexFile* dex_file,
+                                          size_t class_def_index) {
+  MutexLock mu(self, freezing_constructor_lock_);
+  return freezing_constructor_classes_.count(ClassReference(dex_file, class_def_index)) != 0;
+}
+
 }  // namespace art
diff --git a/src/compiler.h b/src/compiler.h
index 8facc9623c..ad2a254f18 100644
--- a/src/compiler.h
+++ b/src/compiler.h
@@ -131,6 +131,9 @@ class Compiler {
 
   const CompiledInvokeStub* FindProxyStub(const char* shorty) const;
 
+  void AddRequiresConstructorBarrier(Thread* self, const DexFile* dex_file, size_t class_def_index);
+  bool RequiresConstructorBarrier(Thread* self, const DexFile* dex_file, size_t class_def_index);
+
   // Callbacks from compiler to see what runtime checks must be generated.
 
   bool CanAssumeTypeIsPresentInDexCache(const DexFile& dex_file, uint32_t type_idx)
@@ -296,7 +299,7 @@ class Compiler {
                       ThreadPool& thread_pool, TimingLogger& timings)
       LOCKS_EXCLUDED(Locks::mutator_lock_);
   void CompileMethod(const DexFile::CodeItem* code_item, uint32_t access_flags,
-                     InvokeType invoke_type, uint32_t method_idx,
+                     InvokeType invoke_type, uint32_t class_def_idx, uint32_t method_idx,
                      jobject class_loader, const DexFile& dex_file)
       LOCKS_EXCLUDED(compiled_methods_lock_);
 
@@ -315,18 +318,22 @@ class Compiler {
 
   InstructionSet instruction_set_;
 
+  // All class references that require
+  mutable Mutex freezing_constructor_lock_ DEFAULT_MUTEX_ACQUIRED_AFTER;
+  std::set<ClassReference> freezing_constructor_classes_ GUARDED_BY(freezing_constructor_lock_);
+
   typedef SafeMap<const ClassReference, CompiledClass*> ClassTable;
-  // All class references that this compiler has compiled
+  // All class references that this compiler has compiled.
   mutable Mutex compiled_classes_lock_ DEFAULT_MUTEX_ACQUIRED_AFTER;
   ClassTable compiled_classes_ GUARDED_BY(compiled_classes_lock_);
 
   typedef SafeMap<const MethodReference, CompiledMethod*> MethodTable;
-  // All method references that this compiler has compiled
+  // All method references that this compiler has compiled.
   mutable Mutex compiled_methods_lock_ DEFAULT_MUTEX_ACQUIRED_AFTER;
   MethodTable compiled_methods_ GUARDED_BY(compiled_methods_lock_);
 
   typedef SafeMap<std::string, const CompiledInvokeStub*> InvokeStubTable;
-  // Invocation stubs created to allow invocation of the compiled methods
+  // Invocation stubs created to allow invocation of the compiled methods.
   mutable Mutex compiled_invoke_stubs_lock_ DEFAULT_MUTEX_ACQUIRED_AFTER;
   InvokeStubTable compiled_invoke_stubs_ GUARDED_BY(compiled_invoke_stubs_lock_);
 
@@ -355,8 +362,8 @@ class Compiler {
   typedef CompiledMethod* (*CompilerFn)(Compiler& compiler,
                                         const DexFile::CodeItem* code_item,
                                         uint32_t access_flags, InvokeType invoke_type,
-                                        uint32_t method_idx, jobject class_loader,
-                                        const DexFile& dex_file);
+                                        uint32_t class_dex_idx, uint32_t method_idx,
+                                        jobject class_loader, const DexFile& dex_file);
   CompilerFn compiler_;
 
   void* compiler_context_;
diff --git a/src/compiler/codegen/mir_to_lir.cc b/src/compiler/codegen/mir_to_lir.cc
index acdeafe32c..1d64661c5d 100644
--- a/src/compiler/codegen/mir_to_lir.cc
+++ b/src/compiler/codegen/mir_to_lir.cc
@@ -86,6 +86,11 @@ static bool CompileDalvikInstruction(CompilationUnit* cu, MIR* mir, BasicBlock*
       cg->GenMoveException(cu, rl_dest);
       break;
     case Instruction::RETURN_VOID:
+      if (((cu->access_flags & kAccConstructor) != 0) &&
+          cu->compiler->RequiresConstructorBarrier(Thread::Current(), cu->dex_file,
+                                                   cu->class_def_idx)) {
+        cg->GenMemBarrier(cu, kStoreStore);
+      }
       if (!(cu->attrs & METHOD_IS_LEAF)) {
         cg->GenSuspendTest(cu, opt_flags);
       }
diff --git a/src/compiler/compiler_ir.h b/src/compiler/compiler_ir.h
index 9c67cd567c..c5f5107eed 100644
--- a/src/compiler/compiler_ir.h
+++ b/src/compiler/compiler_ir.h
@@ -277,6 +277,7 @@ struct CompilationUnit {
       class_linker(NULL),
       dex_file(NULL),
       class_loader(NULL),
+      class_def_idx(0),
       method_idx(0),
       code_item(NULL),
       access_flags(0),
@@ -345,6 +346,7 @@ struct CompilationUnit {
       mstats(NULL),
       checkstats(NULL),
       gen_bitcode(false),
+      llvm_info(NULL),
       context(NULL),
       module(NULL),
       func(NULL),
@@ -368,6 +370,7 @@ struct CompilationUnit {
   ClassLinker* class_linker;           // Linker to resolve fields and methods.
   const DexFile* dex_file;             // DexFile containing the method being compiled.
   jobject class_loader;                // compiling method's class loader.
+  uint32_t class_def_idx;              // compiling method's defining class definition index.
   uint32_t method_idx;                 // compiling method's index into method_ids of DexFile.
   const DexFile::CodeItem* code_item;  // compiling method's DexFile code_item.
   uint32_t access_flags;               // compiling method's access flags.
diff --git a/src/compiler/frontend.cc b/src/compiler/frontend.cc
index 9d30b6ad0b..e6f29ef465 100644
--- a/src/compiler/frontend.cc
+++ b/src/compiler/frontend.cc
@@ -773,8 +773,8 @@ static CompiledMethod* CompileMethod(Compiler& compiler,
                                      const CompilerBackend compiler_backend,
                                      const DexFile::CodeItem* code_item,
                                      uint32_t access_flags, InvokeType invoke_type,
-                                     uint32_t method_idx, jobject class_loader,
-                                     const DexFile& dex_file,
+                                     uint32_t class_def_idx, uint32_t method_idx,
+                                     jobject class_loader, const DexFile& dex_file,
                                      LLVMInfo* llvm_info)
 {
   VLOG(compiler) << "Compiling " << PrettyMethod(method_idx, dex_file) << "...";
@@ -792,6 +792,7 @@ static CompiledMethod* CompileMethod(Compiler& compiler,
   cu->compiler = &compiler;
   cu->class_linker = class_linker;
   cu->dex_file = &dex_file;
+  cu->class_def_idx = class_def_idx;
   cu->method_idx = method_idx;
   cu->code_item = code_item;
   cu->access_flags = access_flags;
@@ -1219,12 +1220,12 @@ CompiledMethod* CompileOneMethod(Compiler& compiler,
                                  const CompilerBackend backend,
                                  const DexFile::CodeItem* code_item,
                                  uint32_t access_flags, InvokeType invoke_type,
-                                 uint32_t method_idx, jobject class_loader,
+                                 uint32_t class_def_idx, uint32_t method_idx, jobject class_loader,
                                  const DexFile& dex_file,
                                  LLVMInfo* llvm_info)
 {
-  return CompileMethod(compiler, backend, code_item, access_flags, invoke_type, method_idx, class_loader,
-                       dex_file, llvm_info);
+  return CompileMethod(compiler, backend, code_item, access_flags, invoke_type, class_def_idx,
+                       method_idx, class_loader, dex_file, llvm_info);
 }
 
 }  // namespace art
@@ -1233,11 +1234,12 @@ extern "C" art::CompiledMethod*
     ArtQuickCompileMethod(art::Compiler& compiler,
                           const art::DexFile::CodeItem* code_item,
                           uint32_t access_flags, art::InvokeType invoke_type,
-                          uint32_t method_idx, jobject class_loader,
+                          uint32_t class_def_idx, uint32_t method_idx, jobject class_loader,
                           const art::DexFile& dex_file)
 {
   // TODO: check method fingerprint here to determine appropriate backend type.  Until then, use build default
   art::CompilerBackend backend = compiler.GetCompilerBackend();
   return art::CompileOneMethod(compiler, backend, code_item, access_flags, invoke_type,
-                               method_idx, class_loader, dex_file, NULL /* use thread llvm_info */);
+                               class_def_idx, method_idx, class_loader, dex_file,
+                               NULL /* use thread llvm_info */);
 }
diff --git a/src/compiler_llvm/compiler_llvm.cc b/src/compiler_llvm/compiler_llvm.cc
index 954e2f420b..6de10e6940 100644
--- a/src/compiler_llvm/compiler_llvm.cc
+++ b/src/compiler_llvm/compiler_llvm.cc
@@ -259,9 +259,11 @@ extern "C" art::CompiledMethod* ArtCompileMethod(art::Compiler& compiler,
                                                  const art::DexFile::CodeItem* code_item,
                                                  uint32_t access_flags,
                                                  art::InvokeType invoke_type,
+                                                 uint32_t class_def_idx,
                                                  uint32_t method_idx,
                                                  jobject class_loader,
                                                  const art::DexFile& dex_file) {
+  UNUSED(class_def_idx);  // TODO: this is used with Compiler::RequiresConstructorBarrier.
   art::ClassLinker *class_linker = art::Runtime::Current()->GetClassLinker();
 
   art::OatCompilationUnit oat_compilation_unit(