From c394199f903e292c67f5a456324fa28eefb664a6 Mon Sep 17 00:00:00 2001 From: Marius Pirvu Date: Mon, 16 Apr 2018 10:52:47 -0400 Subject: [PATCH] Fix devirtualization issues with IlGeneratorMethodDetails When storing a particular variety of IlGeneratorMethodDetails into MethodToBeCompiled, the code uses operator = which transmutes the existing generic IlGeneratorMethodDetails (the base class) into a derived class similar to the parameter that is passed in, by using placement new. The expectation is that any call off the generic IlGeneratorMethodDetails object stored in MethodToBeCompiled is going to be a virtual call. However, the C++ compiler is allowed to devirtualize calls and if that occurs, the JIT is going to invoke the implementation in the generic IlGeneratorMethodDetails. The solution introduced by this commit is to use an object factory that builds a specific IlGeneratorMethodDetails at a designated address and returns a pointer to that address which is then stored in MethodToBeCompiled. Signed-off-by: Marius Pirvu --- .../trj9/control/MethodToBeCompiled.cpp | 4 +-- .../trj9/control/MethodToBeCompiled.hpp | 7 ++-- .../trj9/ilgen/IlGeneratorMethodDetails.hpp | 4 +-- .../trj9/ilgen/J9IlGeneratorMethodDetails.cpp | 36 +++++++------------ .../trj9/ilgen/J9IlGeneratorMethodDetails.hpp | 6 ++-- 5 files changed, 23 insertions(+), 34 deletions(-) diff --git a/runtime/compiler/trj9/control/MethodToBeCompiled.cpp b/runtime/compiler/trj9/control/MethodToBeCompiled.cpp index 9bd24ebccfa..6ac2929e32e 100644 --- a/runtime/compiler/trj9/control/MethodToBeCompiled.cpp +++ b/runtime/compiler/trj9/control/MethodToBeCompiled.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 @@ -50,7 +50,7 @@ TR_MethodToBeCompiled *TR_MethodToBeCompiled::allocate(J9JITConfig *jitConfig) void TR_MethodToBeCompiled::initialize(TR::IlGeneratorMethodDetails & details, void *oldStartPC, CompilationPriority p, TR_OptimizationPlan *optimizationPlan) { - _methodDetails = details; + _methodDetails = TR::IlGeneratorMethodDetails::clone(_methodDetailsStorage, details); _optimizationPlan = optimizationPlan; _next = NULL; _oldStartPC = oldStartPC; diff --git a/runtime/compiler/trj9/control/MethodToBeCompiled.hpp b/runtime/compiler/trj9/control/MethodToBeCompiled.hpp index ee4c4b4babd..2da71174960 100644 --- a/runtime/compiler/trj9/control/MethodToBeCompiled.hpp +++ b/runtime/compiler/trj9/control/MethodToBeCompiled.hpp @@ -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 @@ -52,7 +52,7 @@ struct TR_MethodToBeCompiled void initialize(TR::IlGeneratorMethodDetails & details, void *oldStartPC, CompilationPriority p, TR_OptimizationPlan *optimizationPlan); TR::Monitor *getMonitor() { return _monitor; } - TR::IlGeneratorMethodDetails & getMethodDetails(){ return _methodDetails; } + TR::IlGeneratorMethodDetails & getMethodDetails(){ return *_methodDetails; } bool isDLTCompile() { return getMethodDetails().isMethodInProgress(); } bool isCompiled(); bool isJNINative(); @@ -62,7 +62,8 @@ struct TR_MethodToBeCompiled TR_MethodToBeCompiled *_next; - TR::IlGeneratorMethodDetails _methodDetails; + TR::IlGeneratorMethodDetails _methodDetailsStorage; + TR::IlGeneratorMethodDetails *_methodDetails; void *_oldStartPC; void *_newStartPC; TR::Monitor *_monitor; diff --git a/runtime/compiler/trj9/ilgen/IlGeneratorMethodDetails.hpp b/runtime/compiler/trj9/ilgen/IlGeneratorMethodDetails.hpp index 78e8a61a2ff..0447aece667 100644 --- a/runtime/compiler/trj9/ilgen/IlGeneratorMethodDetails.hpp +++ b/runtime/compiler/trj9/ilgen/IlGeneratorMethodDetails.hpp @@ -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 @@ -50,8 +50,8 @@ class OMR_EXTENSIBLE IlGeneratorMethodDetails : public J9::IlGeneratorMethodDeta IlGeneratorMethodDetails(const TR::IlGeneratorMethodDetails & other) : J9::IlGeneratorMethodDetailsConnector(other) {} - IlGeneratorMethodDetails & operator=(const TR::IlGeneratorMethodDetails & other); }; + } diff --git a/runtime/compiler/trj9/ilgen/J9IlGeneratorMethodDetails.cpp b/runtime/compiler/trj9/ilgen/J9IlGeneratorMethodDetails.cpp index ea475a9e07b..1423a50bc25 100644 --- a/runtime/compiler/trj9/ilgen/J9IlGeneratorMethodDetails.cpp +++ b/runtime/compiler/trj9/ilgen/J9IlGeneratorMethodDetails.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 @@ -35,47 +35,35 @@ #include "ilgen/J9ByteCodeIlGenerator.hpp" #include "trj9/env/VMJ9.h" -namespace TR -{ - -IlGeneratorMethodDetails & IlGeneratorMethodDetails::operator=(const TR::IlGeneratorMethodDetails & other) - { - return self()->J9::IlGeneratorMethodDetailsConnector::operator=(other); - } - -} - namespace J9 { -TR::IlGeneratorMethodDetails & -IlGeneratorMethodDetails::operator=(const TR::IlGeneratorMethodDetails & other) +TR::IlGeneratorMethodDetails * +IlGeneratorMethodDetails::clone(TR::IlGeneratorMethodDetails &storage, const TR::IlGeneratorMethodDetails & other) { - // The purpose of this function is to transmute the receiver object (*this) to make it identical to "other" - // Placement new is used to get the vft updated, and constructors on each of the classes identified here - // take care of copying the appropriate data fields. The if nest below covers every concrete subclass of - // IlGeneratorMethodDetails. If other is not one of these classes, then it will assert. + // The if nest below covers every concrete subclass of IlGeneratorMethodDetails. + // If other is not one of these classes, then it will assert. if (other.isOrdinaryMethod()) - return * new (self()) TR::IlGeneratorMethodDetails(static_cast(other)); + return new (&storage) TR::IlGeneratorMethodDetails(static_cast(other)); else if (other.isDumpMethod()) - return * new (self()) DumpMethodDetails(static_cast(other)); + return new (&storage) DumpMethodDetails(static_cast(other)); else if (other.isNewInstanceThunk()) - return * new (self()) NewInstanceThunkDetails(static_cast(other)); + return new (&storage) NewInstanceThunkDetails(static_cast(other)); else if (other.isMethodInProgress()) - return * new (self()) MethodInProgressDetails(static_cast(other)); + return new (&storage) MethodInProgressDetails(static_cast(other)); else if (other.isMethodHandleThunk()) { if (static_cast(other).isShareable()) - return * new (self()) ShareableInvokeExactThunkDetails(static_cast(other)); + return new (&storage) ShareableInvokeExactThunkDetails(static_cast(other)); else if (static_cast(other).isCustom()) - return * new (self()) CustomInvokeExactThunkDetails(static_cast(other)); + return new (&storage) CustomInvokeExactThunkDetails(static_cast(other)); } TR_ASSERT(0, "Unexpected IlGeneratorMethodDetails object\n"); - return *(self()); + return NULL; // error case } diff --git a/runtime/compiler/trj9/ilgen/J9IlGeneratorMethodDetails.hpp b/runtime/compiler/trj9/ilgen/J9IlGeneratorMethodDetails.hpp index e3a0b4bcb7d..d815f5affc9 100644 --- a/runtime/compiler/trj9/ilgen/J9IlGeneratorMethodDetails.hpp +++ b/runtime/compiler/trj9/ilgen/J9IlGeneratorMethodDetails.hpp @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2017 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 @@ -74,10 +74,10 @@ class OMR_EXTENSIBLE IlGeneratorMethodDetails : public OMR::IlGeneratorMethodDet IlGeneratorMethodDetails(const TR::IlGeneratorMethodDetails & other); - TR::IlGeneratorMethodDetails & operator=(const TR::IlGeneratorMethodDetails & other); - static TR::IlGeneratorMethodDetails & create(TR::IlGeneratorMethodDetails & target, TR_ResolvedMethod *method); + static TR::IlGeneratorMethodDetails * clone(TR::IlGeneratorMethodDetails & storage, const TR::IlGeneratorMethodDetails & source); + virtual const char * name() const { return "OrdinaryMethod"; } virtual bool isOrdinaryMethod() const { return true; }