Skip to content

Improve interface of reachable method analysis and related states to prevent misuse #220

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jun 12, 2025
Merged

Improve interface of reachable method analysis and related states to prevent misuse #220

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3f8735c
Improve misuse handling of reachable method analysis
maximilianruesch Sep 16, 2024
30bec19
Refactor reachable method analyses and their state / TAC handling
maximilianruesch Sep 18, 2024
65647b2
Merge branch 'develop' into improvement/better-error-for-reachable-me…
errt May 6, 2025
dee8a44
Merge branch 'develop' into improvement/better-error-for-reachable-me…
Jun 4, 2025
2c20434
Move logic for handling methods without body out of ReachableMethodAn…
Jun 4, 2025
8aa16f2
Merge branch 'develop' into improvement/better-error-for-reachable-me…
maximilianruesch Jun 5, 2025
6742c9b
Reduce null escape surface in reachable method analysis
maximilianruesch Jun 5, 2025
d5e32d1
Merge branch 'develop' into improvement/better-error-for-reachable-me…
maximilianruesch Jun 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 6 additions & 13 deletions OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/TACAIBasedAnalysisState.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,18 @@ trait TACAIBasedAnalysisState[TheContextType <: Context]
def callContext: ContextType

protected[this] var _tacDependee: EOptionP[Method, TACAI]
assert((_tacDependee eq null) || (_tacDependee.hasUBP && _tacDependee.ub.tac.isDefined))
assert(_tacDependee.hasUBP && _tacDependee.ub.tac.isDefined)

abstract override def hasOpenDependencies: Boolean = {
(_tacDependee ne null) && _tacDependee.isRefinable || super.hasOpenDependencies
}
final def hasTACDependee: Boolean = _tacDependee.isRefinable
abstract override def hasOpenDependencies: Boolean = _tacDependee.isRefinable || super.hasOpenDependencies
final def isTACDependeeRefinable: Boolean = _tacDependee.isRefinable

/**
* Inherited classes that introduce new dependencies must override this method and call add a
* call to super!
*/
abstract override def dependees: Set[SomeEOptionP] = {
val otherDependees = super.dependees
if ((_tacDependee ne null) && _tacDependee.isRefinable)
if (_tacDependee.isRefinable)
otherDependees + _tacDependee
else
otherDependees
Expand All @@ -49,12 +47,7 @@ trait TACAIBasedAnalysisState[TheContextType <: Context]
_tacDependee = tacDependee
}

final def tac: TACode[TACMethodParameter, DUVar[ValueInformation]] = {
_tacDependee.ub.tac.get
}

final def tacDependee: EOptionP[Method, TACAI] = {
_tacDependee
}
final def tac: TACode[TACMethodParameter, DUVar[ValueInformation]] = _tacDependee.ub.tac.get

final def tacDependee: EOptionP[Method, TACAI] = _tacDependee
}
17 changes: 13 additions & 4 deletions OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/cg/CGState.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@ import org.opalj.tac.fpcf.properties.TACAI
/**
* @author Florian Kuebler
*/
class CGState[ContextType <: Context](
override val callContext: ContextType,
override protected[this] var _tacDependee: EOptionP[Method, TACAI]
) extends BaseAnalysisState with TypeIteratorState with TACAIBasedAnalysisState[ContextType] {
class CGState[TheContextType <: Context](
val callContext: TheContextType
) extends BaseAnalysisState with TypeIteratorState with ContextualAnalysis {

override type ContextType = TheContextType

// maps a definition site to the receiver var
private[this] val _virtualCallSites: mutable.Map[CallSite, (V, Set[ReferenceType])] =
Expand All @@ -38,3 +39,11 @@ class CGState[ContextType <: Context](

def hasNonFinalCallSite: Boolean = _virtualCallSites.nonEmpty
}

/**
* @author Maximilian Rüsch
*/
class TACAIBasedCGState[ContextType <: Context](
callContext: ContextType,
override protected[this] var _tacDependee: EOptionP[Method, TACAI]
) extends CGState[ContextType](callContext) with TACAIBasedAnalysisState[ContextType]
30 changes: 14 additions & 16 deletions OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/cg/CallGraphAnalysis.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,13 @@ import org.opalj.value.IsSObjectValue
class CallGraphAnalysis private[cg] (
override val project: SomeProject
) extends ReachableMethodAnalysis with TypeConsumerAnalysis {
type LocalTypeInformation

private[this] val isMethodOverridable: Method => Answer = project.get(IsOverridableMethodKey)
private[this] lazy val getCBSTargets = project.get(CallBySignatureKey)
private[this] val resovleCallBySignature =
project.config.getBoolean("org.opalj.br.analyses.cg.callBySignatureResolution")

def c(state: CGState[ContextType])(eps: SomeEPS): ProperPropertyComputationResult = {
def c(state: TACAIBasedCGState[ContextType])(eps: SomeEPS): ProperPropertyComputationResult = {
eps match {
case UBP(tacai: TACAI) if tacai.tac.isDefined =>
state.updateTACDependee(eps.asInstanceOf[EPS[Method, TACAI]])
Expand All @@ -74,7 +73,7 @@ class CallGraphAnalysis private[cg] (
case UBP(_: TACAI) =>
throw new IllegalStateException("there was already a tac defined")

case EPS(e) =>
case EPS(_) =>
val relevantCallSites = state.dependersOf(eps.toEPK).asInstanceOf[Set[CallSite]]

// ensures, that we only add new calls
Expand Down Expand Up @@ -119,27 +118,26 @@ class CallGraphAnalysis private[cg] (
}
}

override final def processMethodWithoutBody(callContext: ContextType): ProperPropertyComputationResult = {
Results(new DirectCalls().partialResults(callContext, enforceCalleesResult = true))
}

override final def processMethod(
callContext: ContextType,
tacEP: EPS[Method, TACAI]
): ProperPropertyComputationResult = {
val state = new CGState[ContextType](callContext, tacEP)
if (tacEP ne null)
processMethod(state, new DirectCalls())
else
returnResult(new DirectCalls(), enforceCalleesResult = true)(state)
val state = new TACAIBasedCGState[ContextType](callContext, tacEP)
processMethod(state, new DirectCalls())
}

override final val processesMethodsWithoutBody = true

protected[this] def doHandleVirtualCall(
callContext: ContextType,
call: Call[V] with VirtualCall[V],
pc: Int,
specializedDeclaringClassType: ReferenceType,
isPrecise: Boolean,
calleesAndCallers: DirectCalls
)(implicit state: CGState[ContextType]): Unit = {
)(implicit state: TACAIBasedCGState[ContextType]): Unit = {
val callerType = callContext.method.declaringClassType
val callSite = CallSite(pc, call.name, call.descriptor, call.declaringClass)

Expand Down Expand Up @@ -209,7 +207,7 @@ class CallGraphAnalysis private[cg] (
}

protected final def processMethod(
state: CGState[ContextType],
state: TACAIBasedCGState[ContextType],
calls: DirectCalls
): ProperPropertyComputationResult = {
val tac = state.tac
Expand Down Expand Up @@ -294,13 +292,13 @@ class CallGraphAnalysis private[cg] (
case _ => // nothing to do
}

returnResult(calls, true)(state)
returnResult(calls, enforceCalleesResult = true)(state)
}

protected[this] def returnResult(
calleesAndCallers: DirectCalls,
enforceCalleesResult: Boolean = false
)(implicit state: CGState[ContextType]): ProperPropertyComputationResult = {
)(implicit state: TACAIBasedCGState[ContextType]): ProperPropertyComputationResult = {
val results = calleesAndCallers.partialResults(state.callContext, enforceCalleesResult)

// FIXME: This won't work for refinable TACs as state.hasNonFinalCallSite may return false
Expand Down Expand Up @@ -388,7 +386,7 @@ class CallGraphAnalysis private[cg] (
call: Call[V] with VirtualCall[V],
pc: Int,
calleesAndCallers: DirectCalls
)(implicit state: CGState[ContextType]): Unit = {
)(implicit state: TACAIBasedCGState[ContextType]): Unit = {
val rvs = call.receiver.asVar.value.asReferenceValue.allValues
for (rv <- rvs) rv match {
case _: IsSArrayValue =>
Expand Down Expand Up @@ -422,7 +420,7 @@ class CallGraphAnalysis private[cg] (
call: Call[V] with VirtualCall[V],
pc: Int,
calleesAndCallers: DirectCalls
)(implicit state: CGState[ContextType]): Unit = {
)(implicit state: TACAIBasedCGState[ContextType]): Unit = {
doHandleVirtualCall(callContext, call, pc, calleeType, isPrecise = true, calleesAndCallers)
}
}
Expand Down
11 changes: 6 additions & 5 deletions OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/cg/DoPrivilegedCGAnalysis.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,10 @@ class DoPrivilegedMethodAnalysis private[cg] (
if (params.nonEmpty && params.head.isDefined) {
val param = params.head.get.asVar

implicit val state: CGState[ContextType] =
new CGState[ContextType](callerContext, FinalEP(callerContext.method.definedMethod, TheTACAI(tac)))
implicit val state: TACAIBasedCGState[ContextType] = new TACAIBasedCGState[ContextType](
callerContext,
FinalEP(callerContext.method.definedMethod, TheTACAI(tac))
)

val thisActual = persistentUVar(param)(state.tac.stmts)

Expand All @@ -93,8 +95,7 @@ class DoPrivilegedMethodAnalysis private[cg] (
thisVar: V,
thisActual: Some[(ValueInformation, IntTrieSet)],
calls: IndirectCalls
)(implicit state: CGState[ContextType]): ProperPropertyComputationResult = {

)(implicit state: TACAIBasedCGState[ContextType]): ProperPropertyComputationResult = {
val partialResults = calls.partialResults(state.callContext)
if (state.hasOpenDependencies)
Results(
Expand Down Expand Up @@ -127,7 +128,7 @@ class DoPrivilegedMethodAnalysis private[cg] (
}

def c(
state: CGState[ContextType],
state: TACAIBasedCGState[ContextType],
thisVar: V,
thisActual: Some[(ValueInformation, IntTrieSet)]
)(eps: SomeEPS): ProperPropertyComputationResult = {
Expand Down
79 changes: 34 additions & 45 deletions OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/cg/ReachableMethodAnalysis.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import org.opalj.br.fpcf.properties.cg.Callers
import org.opalj.br.fpcf.properties.cg.NoCallers
import org.opalj.fpcf.EOptionP
import org.opalj.fpcf.EPS
import org.opalj.fpcf.FinalP
import org.opalj.fpcf.InterimPartialResult
import org.opalj.fpcf.NoResult
import org.opalj.fpcf.ProperPropertyComputationResult
Expand All @@ -28,6 +27,8 @@ import org.opalj.tac.fpcf.properties.TACAI
* Base trait for analyses that are executed for every method that is reachable.
* The analysis is performed by `processMethod`.
*
* Note that methods without a body are not processed unless `processMethodWithoutBody` is overridden.
*
* @author Florian Kuebler
*/
trait ReachableMethodAnalysis extends FPCFAnalysis with TypeConsumerAnalysis {
Expand All @@ -36,73 +37,66 @@ trait ReachableMethodAnalysis extends FPCFAnalysis with TypeConsumerAnalysis {

final def analyze(declaredMethod: DeclaredMethod): PropertyComputationResult = {
val callersEOptP = propertyStore(declaredMethod, Callers.key)
(callersEOptP: @unchecked) match {
case FinalP(NoCallers) =>
// nothing to do, since there is no caller
return NoResult;

case eps: EPS[_, _] =>
if (eps.ub eq NoCallers) {
// we can not create a dependency here, so the analysis is not allowed to create
// such a result
throw new IllegalStateException("illegal immediate result for callers")
}
// the method is reachable, so we analyze it!

if (callersEOptP.isFinal && callersEOptP.ub == NoCallers) {
return NoResult;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we keep the check for an illegal NoCallers interim state at least as an assertion? Right now it is not clearly visible that the method is reachable and should be analyzed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The NoCallers interim state was illegal since the code at the top did not know the class could use the continuation without a depending caller. Since the class can invoke and handle continuations just fine without using a depender constructed from a caller, the NoCallers interim state is not illegal anymore.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it's an illegal state in general, isn't it? I don't think call graph analyses are supposed to ever create such an interim state.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it is an illegal state in general, then it should be asserted by the call graph generating framework and not the framework for analysis that use the call graph (even if they at least partially overlap). This class seems like the wrong area of code to make such an assertion (e.g. it would only be triggered when using such an analysis even though the result should be illegal regardless of using this class or not).

}

// we only allow defined methods
if (!declaredMethod.hasSingleDefinedMethod)
return processMethodWithoutBody(callersEOptP);
return processNewContexts(callersEOptP, NoCallers) { processMethodWithoutBody };

val method = declaredMethod.definedMethod

// we only allow defined methods with declared type eq. to the class of the method
if (method.classFile.thisType != declaredMethod.declaringClassType)
return NoResult;

if (method.body.isEmpty)
if (method.body.isEmpty) {
// happens in particular for native methods
return processMethodWithoutBody(callersEOptP);
return processNewContexts(callersEOptP, NoCallers) { processMethodWithoutBody };
};

val tacEP = propertyStore(method, TACAI.key)

if (tacEP.hasUBP && tacEP.ub.tac.isDefined) {
processMethod(callersEOptP, null, tacEP.asEPS)
processMethodWithTAC(callersEOptP, NoCallers, tacEP.asEPS)
} else {
InterimPartialResult(Set(tacEP), continuationForTAC(declaredMethod))
}
}

protected val processesMethodsWithoutBody = false

protected def processMethodWithoutBody(
eOptP: EOptionP[DeclaredMethod, Callers]
): PropertyComputationResult = {
if (processesMethodsWithoutBody) {
processMethod(eOptP, null, null)
} else
NoResult
private def processMethodWithTAC(
eOptP: EOptionP[DeclaredMethod, Callers],
oldCallers: Callers,
tacEP: EPS[Method, TACAI]
): ProperPropertyComputationResult = {
processNewContexts(eOptP, oldCallers) { processMethod(_, tacEP) }
}

private[this] def processMethod(
eOptP: EOptionP[DeclaredMethod, Callers],
seen: Callers,
tacEP: EPS[Method, TACAI]
): ProperPropertyComputationResult = {
private def processNewContexts(
eOptP: EOptionP[DeclaredMethod, Callers],
oldCallers: Callers
)(processMethod: ContextType => ProperPropertyComputationResult): ProperPropertyComputationResult = {
val newCallers = if (eOptP.hasUBP) eOptP.ub else NoCallers
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be more sensible to turn this into an if, only executing the forNewCalleeContexts if there is a UBP?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we need the newCallers for constructing a continuation and forNewCalleeContexts does not do anything when NoCallers is given, I view this as reasonable and less verbose as well.

var results: List[ProperPropertyComputationResult] = Nil
eOptP.ub.forNewCalleeContexts(seen, eOptP.e) { calleeContext =>

newCallers.forNewCalleeContexts(oldCallers, eOptP.e) { calleeContext =>
val theCalleeContext =
if (calleeContext.hasContext) calleeContext.asInstanceOf[ContextType]
else typeIterator.newContext(eOptP.e)
results ::= processMethod(theCalleeContext, tacEP)
results ::= processMethod(theCalleeContext)
}

Results(
InterimPartialResult(Set(eOptP), continuationForCallers(eOptP.ub, tacEP)),
InterimPartialResult(Set(eOptP), continuationForCallers(processNewContexts(_, newCallers)(processMethod))),
results
)
}

protected def processMethodWithoutBody(
callContext: ContextType
): ProperPropertyComputationResult = Results()

def processMethod(
callContext: ContextType,
tacEP: EPS[Method, TACAI]
Expand All @@ -113,9 +107,9 @@ trait ReachableMethodAnalysis extends FPCFAnalysis with TypeConsumerAnalysis {
)(someEPS: SomeEPS): ProperPropertyComputationResult = {
someEPS match {
case UBP(tac: TACAI) if tac.tac.isDefined =>
processMethod(
processMethodWithTAC(
propertyStore(declaredMethod, Callers.key),
null,
NoCallers,
someEPS.asInstanceOf[EPS[Method, TACAI]]
)
case _ =>
Expand All @@ -124,13 +118,8 @@ trait ReachableMethodAnalysis extends FPCFAnalysis with TypeConsumerAnalysis {
}

private[this] def continuationForCallers(
oldCallers: Callers,
tacEP: EPS[Method, TACAI]
)(
update: SomeEPS
): ProperPropertyComputationResult = {
val newCallers = update.asInstanceOf[EPS[DeclaredMethod, Callers]]
processMethod(newCallers, oldCallers, tacEP)
processMethod: EOptionP[DeclaredMethod, Callers] => ProperPropertyComputationResult
)(someEPS: SomeEPS): ProperPropertyComputationResult = {
processMethod(someEPS.asInstanceOf[EPS[DeclaredMethod, Callers]])
}

}
8 changes: 4 additions & 4 deletions ...tac/src/main/scala/org/opalj/tac/fpcf/analyses/cg/SerializationRelatedCallsAnalysis.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ class OOSWriteObjectAnalysis private[analyses] (
val receiver = persistentUVar(param)
val parameters = Seq(receiverOption.flatMap(os => persistentUVar(os.asVar)))

implicit val state: CGState[ContextType] = new CGState[ContextType](
implicit val state: TACAIBasedCGState[ContextType] = new TACAIBasedCGState[ContextType](
callerContext,
FinalEP(callerContext.method.definedMethod, TheTACAI(tac))
)
Expand All @@ -101,7 +101,7 @@ class OOSWriteObjectAnalysis private[analyses] (
receiverVar: V,
receiver: Option[(ValueInformation, IntTrieSet)],
parameters: Seq[Option[(ValueInformation, IntTrieSet)]],
state: CGState[ContextType]
state: TACAIBasedCGState[ContextType]
)(eps: SomeEPS): ProperPropertyComputationResult = {
val pc = state.dependersOf(eps.toEPK).head.asInstanceOf[Int]

Expand All @@ -126,7 +126,7 @@ class OOSWriteObjectAnalysis private[analyses] (
receiver: Option[(ValueInformation, IntTrieSet)],
parameters: Seq[Option[(ValueInformation, IntTrieSet)]],
indirectCalls: IndirectCalls
)(implicit state: CGState[ContextType]): ProperPropertyComputationResult = {
)(implicit state: TACAIBasedCGState[ContextType]): ProperPropertyComputationResult = {
val results = indirectCalls.partialResults(state.callContext)
if (state.hasOpenDependencies)
Results(
Expand All @@ -144,7 +144,7 @@ class OOSWriteObjectAnalysis private[analyses] (
receiver: Option[(ValueInformation, IntTrieSet)],
parameters: Seq[Option[(ValueInformation, IntTrieSet)]],
indirectCalls: IndirectCalls
)(implicit state: CGState[ContextType]): Unit = {
)(implicit state: TACAIBasedCGState[ContextType]): Unit = {
typeIterator.foreachType(
param,
typeIterator.typesProperty(param, callContext, callPC.asInstanceOf[Entity], state.tac.stmts)
Expand Down
Loading