Skip to content

Commit

Permalink
Wrap all javaparser getType calls in Try (#4666)
Browse files Browse the repository at this point in the history
* Wrap all javaparser getType calls in Try

* Log failures

* Switch missed try

* Limit logging counts

* Fix one more crash and limit log count

* Use atomic operation for updating counts
  • Loading branch information
johannescoetzee committed Jun 14, 2024
1 parent 4c1e044 commit a65a3b2
Show file tree
Hide file tree
Showing 10 changed files with 137 additions and 77 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import io.shiftleft.codepropertygraph.generated.nodes.{NewClosureBinding, NewFil
import org.slf4j.LoggerFactory
import overflowdb.BatchedUpdate.DiffGraphBuilder

import java.util.concurrent.ConcurrentHashMap
import scala.collection.mutable
import scala.jdk.CollectionConverters.*
import scala.jdk.OptionConverters.RichOptional
Expand Down Expand Up @@ -87,7 +88,8 @@ class AstCreator(
fileContent: Option[String],
global: Global,
val symbolSolver: JavaSymbolSolver,
protected val keepTypeArguments: Boolean
protected val keepTypeArguments: Boolean,
val loggedExceptionCounts: scala.collection.concurrent.Map[Class[?], Int]
)(implicit val withSchemaValidation: ValidationMode)
extends AstCreatorBase(filename)
with AstNodeBuilder[Node, AstCreator]
Expand Down Expand Up @@ -244,7 +246,28 @@ class AstCreator(

private[astcreation] def tryWithSafeStackOverflow[T](expr: => T): Try[T] = {
try {
Try(expr)

/** JavaParser throws UnsolvedSymbolExceptions if a type cannot be solved, which is usually an expected occurrence
* that does not warrant specific failure logging. Since it's impossible to tell whether these are legitimately
* unresolved types or a bug, don't log them.
*/
Try(expr) match {
case success: Success[_] => success
case Failure(exception: UnsolvedSymbolException) => Failure(exception)
case failure: Failure[_] =>
val exceptionType = failure.exception.getClass

val loggedCount = loggedExceptionCounts.updateWith(exceptionType) {
case Some(value) => Some(value + 1)
case None => Some(1)
}

if (loggedCount.exists(_ <= 3)) {
logger.debug("tryWithFailureLogging encountered exception", failure.exception)
}

failure
}
} catch {
// This is really, really ugly, but there's a bug in the JavaParser symbol solver that can lead to
// unterminated recursion in some cases where types cannot be resolved.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import io.joern.javasrc2cpg.scope.JavaScopeElement.fullName

import scala.jdk.CollectionConverters.*
import scala.jdk.OptionConverters.RichOptional
import scala.util.Try
import scala.util.{Failure, Success, Try}
import io.shiftleft.codepropertygraph.generated.nodes.AstNodeNew
import io.shiftleft.codepropertygraph.generated.nodes.NewCall
import io.shiftleft.codepropertygraph.generated.Operators
Expand All @@ -42,24 +42,29 @@ import io.shiftleft.codepropertygraph.generated.EdgeTypes
import com.github.javaparser.ast.Node
import com.github.javaparser.symbolsolver.javaparsermodel.declarations.JavaParserParameterDeclaration
import io.joern.javasrc2cpg.astcreation.declarations.AstForMethodsCreator.PartialConstructorDeclaration
import io.joern.javasrc2cpg.util.Util
import io.joern.javasrc2cpg.util.{NameConstants, Util}

private[declarations] trait AstForMethodsCreator { this: AstCreator =>
def astForMethod(methodDeclaration: MethodDeclaration): Ast = {
val methodNode = createPartialMethod(methodDeclaration)

val typeParameters = getIdentifiersForTypeParameters(methodDeclaration)

val maybeResolved = tryWithSafeStackOverflow(methodDeclaration.resolve())
val expectedReturnType = Try(symbolSolver.toResolvedType(methodDeclaration.getType, classOf[ResolvedType])).toOption
val simpleMethodReturnType = Util.stripGenericTypes(methodDeclaration.getTypeAsString())
val maybeResolved = tryWithSafeStackOverflow(methodDeclaration.resolve())
val expectedReturnType = tryWithSafeStackOverflow(
symbolSolver.toResolvedType(methodDeclaration.getType, classOf[ResolvedType])
).toOption
val simpleMethodReturnType =
tryWithSafeStackOverflow(methodDeclaration.getTypeAsString).map(Util.stripGenericTypes).toOption
val returnTypeFullName = expectedReturnType
.flatMap(typeInfoCalc.fullName)
.orElse(scope.lookupType(simpleMethodReturnType))
.orElse(simpleMethodReturnType.flatMap(scope.lookupType(_)))
.orElse(
Try(methodDeclaration.getType.asClassOrInterfaceType).toOption.flatMap(t => scope.lookupType(t.getNameAsString))
tryWithSafeStackOverflow(methodDeclaration.getType.asClassOrInterfaceType).toOption.flatMap(t =>
scope.lookupType(t.getNameAsString)
)
)
.orElse(typeParameters.find(_.name == simpleMethodReturnType).map(_.typeFullName))
.orElse(typeParameters.find(typeParam => simpleMethodReturnType.contains(typeParam.name)).map(_.typeFullName))

scope.pushMethodScope(
methodNode,
Expand Down Expand Up @@ -89,12 +94,11 @@ private[declarations] trait AstForMethodsCreator { this: AstCreator =>
}

val bodyAst = methodDeclaration.getBody.toScala.map(astForBlockStatement(_)).getOrElse(Ast(NewBlock()))
val methodReturn = newMethodReturnNode(
returnTypeFullName.getOrElse(TypeConstants.Any),
None,
line(methodDeclaration.getType),
column(methodDeclaration.getType)
)
val (lineNr, columnNr) = tryWithSafeStackOverflow(methodDeclaration.getType) match {
case Success(typ) => (line(typ), column(typ))
case Failure(_) => (line(methodDeclaration), column(methodDeclaration))
}
val methodReturn = newMethodReturnNode(returnTypeFullName.getOrElse(TypeConstants.Any), None, lineNr, columnNr)

val annotationAsts = methodDeclaration.getAnnotations.asScala.map(astForAnnotationExpr).toSeq

Expand Down Expand Up @@ -147,7 +151,7 @@ private[declarations] trait AstForMethodsCreator { this: AstCreator =>
private def getIdentifiersForTypeParameters(methodDeclaration: MethodDeclaration): List[NewIdentifier] = {
methodDeclaration.getTypeParameters.asScala.map { typeParameter =>
val name = typeParameter.getNameAsString
val typeFullName = typeParameter.getTypeBound.asScala.headOption
val typeFullName = tryWithSafeStackOverflow(typeParameter.getTypeBound.asScala.headOption).toOption.flatten
.flatMap(typeInfoCalc.fullName)
.getOrElse(TypeConstants.Object)
typeInfoCalc.registerType(typeFullName)
Expand Down Expand Up @@ -219,16 +223,18 @@ private[declarations] trait AstForMethodsCreator { this: AstCreator =>
}

private def astForParameter(parameter: Parameter, childNum: Int): Ast = {
val maybeArraySuffix = if (parameter.isVarArgs) "[]" else ""
val rawParameterTypeName = Util.stripGenericTypes(parameter.getTypeAsString)
val maybeArraySuffix = if (parameter.isVarArgs) "[]" else ""
val rawParameterTypeName =
tryWithSafeStackOverflow(parameter.getTypeAsString).map(Util.stripGenericTypes).getOrElse(NameConstants.Unknown)
val parameterType = tryWithSafeStackOverflow(parameter.getType).toOption
val typeFullName =
typeInfoCalc
.fullName(parameter.getType)
parameterType
.flatMap(typeInfoCalc.fullName)
.orElse(scope.lookupType(rawParameterTypeName))
.map(_ ++ maybeArraySuffix)
.getOrElse(s"${Defines.UnresolvedNamespace}.$rawParameterTypeName")
val evalStrat =
if (parameter.getType.isPrimitiveType) EvaluationStrategies.BY_VALUE else EvaluationStrategies.BY_SHARING
if (parameterType.exists(_.isPrimitiveType)) EvaluationStrategies.BY_VALUE else EvaluationStrategies.BY_SHARING
typeInfoCalc.registerType(typeFullName)

val parameterNode = NewMethodParameterIn()
Expand Down Expand Up @@ -258,7 +264,7 @@ private[declarations] trait AstForMethodsCreator { this: AstCreator =>
Try(methodLike.getParam(index)).toOption
}
.map { param =>
Try(param.getType).toOption
tryWithSafeStackOverflow(param.getType).toOption
.flatMap(paramType => typeInfoCalc.fullName(paramType, typeParamValues))
// In a scenario where we have an import of an external type e.g. `import foo.bar.Baz` and
// this parameter's type is e.g. `Baz<String>`, the lookup will fail. However, if we lookup
Expand Down Expand Up @@ -292,7 +298,7 @@ private[declarations] trait AstForMethodsCreator { this: AstCreator =>

val maybeReturnType =
Try(method.getReturnType).toOption
.flatMap(returnType => typeInfoCalc.fullName(returnType, typeParamValues))
.flatMap(typeInfoCalc.fullName(_, typeParamValues))

composeSignature(maybeReturnType, maybeParameterTypes, method.getNumberOfParams)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -568,18 +568,21 @@ private[declarations] trait AstForTypeDeclsCreator { this: AstCreator =>
// TODO: Should be able to find expected type here
val annotations = fieldDeclaration.getAnnotations

val rawTypeName = Util.stripGenericTypes(v.getTypeAsString)
val rawTypeName =
tryWithSafeStackOverflow(v.getTypeAsString).map(Util.stripGenericTypes).getOrElse(NameConstants.Unknown)

val typeFullName = typeInfoCalc
.fullName(v.getType)
.orElse(scope.lookupType(rawTypeName))
.getOrElse(s"${Defines.UnresolvedNamespace}.$rawTypeName")
val typeFullName =
tryWithSafeStackOverflow(v.getType).toOption
.flatMap(typeInfoCalc.fullName)
.orElse(scope.lookupType(rawTypeName))
.getOrElse(s"${Defines.UnresolvedNamespace}.$rawTypeName")

val name = v.getName.toString
// Use type name without generics stripped in code
val node = memberNode(v, name, s"${v.getTypeAsString} $name", typeFullName)
val memberAst = Ast(node)
val annotationAsts = annotations.asScala.map(astForAnnotationExpr)
val variableTypeString = tryWithSafeStackOverflow(v.getTypeAsString).getOrElse("")
val node = memberNode(v, name, s"$variableTypeString $name", typeFullName)
val memberAst = Ast(node)
val annotationAsts = annotations.asScala.map(astForAnnotationExpr)

val fieldDeclModifiers = modifiersForFieldDeclaration(fieldDeclaration)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,12 @@ trait AstForCallExpressionsCreator { this: AstCreator =>

val anonymousClassBody = expr.getAnonymousClassBody.toScala.map(_.asScala.toList)
val nameSuffix = if (anonymousClassBody.isEmpty) "" else s"$$${scope.getNextAnonymousClassIndex()}"
val rawType = Util.stripGenericTypes(expr.getTypeAsString)
val typeName = s"$rawType$nameSuffix"
val rawType =
tryWithSafeStackOverflow(expr.getTypeAsString)
.map(Util.stripGenericTypes)
.toOption
.getOrElse(NameConstants.Unknown)
val typeName = s"$rawType$nameSuffix"

val baseTypeFromScope = scope.lookupScopeType(rawType)
// These will be the same for non-anonymous type decls, but in that case only the typeFullName will be used.
Expand Down Expand Up @@ -314,9 +318,9 @@ trait AstForCallExpressionsCreator { this: AstCreator =>
val paramCount = methodDecl.getNumberOfParams

val resolvedType = if (idx < paramCount) {
Some(methodDecl.getParam(idx).getType)
tryWithSafeStackOverflow(methodDecl.getParam(idx).getType).toOption
} else if (paramCount > 0 && methodDecl.getParam(paramCount - 1).isVariadic) {
Some(methodDecl.getParam(paramCount - 1).getType)
tryWithSafeStackOverflow(methodDecl.getParam(paramCount - 1).getType).toOption
} else {
None
}
Expand Down Expand Up @@ -457,7 +461,7 @@ trait AstForCallExpressionsCreator { this: AstCreator =>

case objectCreationExpr: ObjectCreationExpr =>
// Use type name with generics for code
val typeName = objectCreationExpr.getTypeAsString
val typeName = tryWithSafeStackOverflow(objectCreationExpr.getTypeAsString).getOrElse(NameConstants.Unknown)
val argumentsString = getArgumentCodeString(objectCreationExpr.getArguments)
someWithDotSuffix(s"new $typeName($argumentsString)")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -352,8 +352,7 @@ private[expressions] trait AstForLambdasCreator { this: AstCreator =>
// this will yield the erased types which is why the actual lambda
// expression parameters are only used as a fallback.
lambdaParameters
.map(_.getType)
.map(typeInfoCalc.fullName)
.flatMap(param => tryWithSafeStackOverflow(typeInfoCalc.fullName(param.getType)).toOption)
}

if (paramTypesList.sizeIs != lambdaParameters.size) {
Expand All @@ -368,7 +367,8 @@ private[expressions] trait AstForLambdasCreator { this: AstCreator =>
val typeFullName = maybeType.getOrElse(TypeConstants.Any)
val code = s"$typeFullName $name"
val evalStrat =
if (param.getType.isPrimitiveType) EvaluationStrategies.BY_VALUE else EvaluationStrategies.BY_SHARING
if (tryWithSafeStackOverflow(param.getType).toOption.exists(_.isPrimitiveType)) EvaluationStrategies.BY_VALUE
else EvaluationStrategies.BY_SHARING
val paramNode = NewMethodParameterIn()
.name(name)
.index(idx + 1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import io.shiftleft.codepropertygraph.generated.{EdgeTypes, Operators}

import scala.jdk.CollectionConverters.*
import scala.jdk.OptionConverters.RichOptional
import scala.util.{Failure, Success}
import scala.util.{Failure, Success, Try}

trait AstForSimpleExpressionsCreator { this: AstCreator =>

Expand Down Expand Up @@ -174,8 +174,8 @@ trait AstForSimpleExpressionsCreator { this: AstCreator =>

private[expressions] def astForCastExpr(expr: CastExpr, expectedType: ExpectedType): Ast = {
val typeFullName =
typeInfoCalc
.fullName(expr.getType)
tryWithSafeStackOverflow(expr.getType).toOption
.flatMap(typeInfoCalc.fullName)
.orElse(expectedType.fullName)
.getOrElse(TypeConstants.Any)

Expand All @@ -188,7 +188,7 @@ trait AstForSimpleExpressionsCreator { this: AstCreator =>
)

val typeNode = NewTypeRef()
.code(expr.getType.toString)
.code(tryWithSafeStackOverflow(expr.getType.toString).getOrElse(code(expr)))
.lineNumber(line(expr))
.columnNumber(column(expr))
.typeFullName(typeFullName)
Expand All @@ -203,13 +203,13 @@ trait AstForSimpleExpressionsCreator { this: AstCreator =>
val someTypeFullName = Some(TypeConstants.Class)
val callNode = newOperatorCallNode(Operators.fieldAccess, expr.toString, someTypeFullName, line(expr), column(expr))

val identifierType = typeInfoCalc.fullName(expr.getType)
val identifier = identifierNode(
expr,
Util.stripGenericTypes(expr.getTypeAsString),
expr.getTypeAsString,
identifierType.getOrElse("ANY")
)
val identifierType = tryWithSafeStackOverflow(expr.getType).toOption.flatMap(typeInfoCalc.fullName)
val exprTypeString =
tryWithSafeStackOverflow(expr.getTypeAsString).toOption
.orElse(identifierType)
.getOrElse(code(expr).stripSuffix(".class"))
val identifier =
identifierNode(expr, Util.stripGenericTypes(exprTypeString), exprTypeString, identifierType.getOrElse("ANY"))
val idAst = Ast(identifier)

val fieldIdentifier = NewFieldIdentifier()
Expand Down Expand Up @@ -273,12 +273,13 @@ trait AstForSimpleExpressionsCreator { this: AstCreator =>
newOperatorCallNode(Operators.instanceOf, expr.toString, booleanTypeFullName, line(expr), column(expr))

val exprAst = astsForExpression(expr.getExpression, ExpectedType.empty)
val typeFullName = typeInfoCalc.fullName(expr.getType).getOrElse(TypeConstants.Any)
val exprType = tryWithSafeStackOverflow(expr.getType).toOption
val typeFullName = exprType.flatMap(typeInfoCalc.fullName).getOrElse(TypeConstants.Any)
val typeNode =
NewTypeRef()
.code(expr.getType.toString)
.code(exprType.map(_.toString).getOrElse(code(expr).split("instanceof").lastOption.getOrElse("")))
.lineNumber(line(expr))
.columnNumber(column(expr.getType))
.columnNumber(exprType.map(column(_)).getOrElse(column(expr)))
.typeFullName(typeFullName)
val typeAst = Ast(typeNode)

Expand Down Expand Up @@ -393,9 +394,9 @@ trait AstForSimpleExpressionsCreator { this: AstCreator =>
private[expressions] def astForMethodReferenceExpr(expr: MethodReferenceExpr, expectedType: ExpectedType): Ast = {
val typeFullName = expr.getScope match {
case typeExpr: TypeExpr =>
val rawType = Util.stripGenericTypes(typeExpr.getTypeAsString)
val rawType = tryWithSafeStackOverflow(typeExpr.getTypeAsString).map(Util.stripGenericTypes).toOption
// JavaParser wraps the "type" scope of a MethodReferenceExpr in a TypeExpr, but this also catches variable names.
scope.lookupVariableOrType(rawType).orElse(expressionReturnTypeFullName(typeExpr))
rawType.flatMap(scope.lookupVariableOrType).orElse(expressionReturnTypeFullName(typeExpr))
case scopeExpr => expressionReturnTypeFullName(scopeExpr)
}

Expand All @@ -406,7 +407,7 @@ trait AstForSimpleExpressionsCreator { this: AstCreator =>
case Failure(_) => Defines.UnresolvedSignature

case Success(resolvedMethod) =>
val returnType = typeInfoCalc.fullName(resolvedMethod.getReturnType)
val returnType = tryWithSafeStackOverflow(resolvedMethod.getReturnType).toOption.flatMap(typeInfoCalc.fullName)
val parameterTypes = argumentTypesForMethodLike(Success(resolvedMethod))
composeSignature(returnType, parameterTypes, resolvedMethod.getNumberOfParams)
}
Expand Down
Loading

0 comments on commit a65a3b2

Please sign in to comment.