Skip to content

Commit

Permalink
Fix devirtualization issues with IlGeneratorMethodDetails
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
mpirvu committed Apr 23, 2018
1 parent 2c9920a commit c394199
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 34 deletions.
4 changes: 2 additions & 2 deletions runtime/compiler/trj9/control/MethodToBeCompiled.cpp
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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;
Expand Down
7 changes: 4 additions & 3 deletions runtime/compiler/trj9/control/MethodToBeCompiled.hpp
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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();
Expand All @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions runtime/compiler/trj9/ilgen/IlGeneratorMethodDetails.hpp
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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);
};

}


Expand Down
36 changes: 12 additions & 24 deletions runtime/compiler/trj9/ilgen/J9IlGeneratorMethodDetails.cpp
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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<const TR::IlGeneratorMethodDetails &>(other));
return new (&storage) TR::IlGeneratorMethodDetails(static_cast<const TR::IlGeneratorMethodDetails &>(other));
else if (other.isDumpMethod())
return * new (self()) DumpMethodDetails(static_cast<const DumpMethodDetails &>(other));
return new (&storage) DumpMethodDetails(static_cast<const DumpMethodDetails &>(other));
else if (other.isNewInstanceThunk())
return * new (self()) NewInstanceThunkDetails(static_cast<const NewInstanceThunkDetails &>(other));
return new (&storage) NewInstanceThunkDetails(static_cast<const NewInstanceThunkDetails &>(other));
else if (other.isMethodInProgress())
return * new (self()) MethodInProgressDetails(static_cast<const MethodInProgressDetails &>(other));
return new (&storage) MethodInProgressDetails(static_cast<const MethodInProgressDetails &>(other));
else if (other.isMethodHandleThunk())
{
if (static_cast<const MethodHandleThunkDetails &>(other).isShareable())
return * new (self()) ShareableInvokeExactThunkDetails(static_cast<const ShareableInvokeExactThunkDetails &>(other));
return new (&storage) ShareableInvokeExactThunkDetails(static_cast<const ShareableInvokeExactThunkDetails &>(other));
else if (static_cast<const MethodHandleThunkDetails &>(other).isCustom())
return * new (self()) CustomInvokeExactThunkDetails(static_cast<const CustomInvokeExactThunkDetails &>(other));
return new (&storage) CustomInvokeExactThunkDetails(static_cast<const CustomInvokeExactThunkDetails &>(other));
}

TR_ASSERT(0, "Unexpected IlGeneratorMethodDetails object\n");
return *(self());
return NULL; // error case
}


Expand Down
6 changes: 3 additions & 3 deletions runtime/compiler/trj9/ilgen/J9IlGeneratorMethodDetails.hpp
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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; }
Expand Down

0 comments on commit c394199

Please sign in to comment.