Skip to content
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.

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

Feature/string analysis #2

Open
wants to merge 318 commits into
base: develop
Choose a base branch
from
Open

Feature/string analysis #2

wants to merge 318 commits into from

Conversation

errt
Copy link
Collaborator

@errt errt commented Oct 17, 2019

Implementation of the string analysis (the interprocedural as well as the intraprocedural variant).

Original PR by PatrickN.

Copy link
Collaborator

@florian-kuebler florian-kuebler left a comment

Choose a reason for hiding this comment

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

Please address comments

* often.
*
* The analysis can be configured by passing the following optional parameters: `class` (the class
* to analyze), `granularity` (fine- or coarse-grained; defines which information will be gathered
Copy link
Collaborator

Choose a reason for hiding this comment

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

See comment in #1

analyzeString(sb.toString());
}

// @StringDefinitions(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove code block

"properties/string_analysis/StringDefinitions.class"
) ++ filesToLoad
val basePath = System.getProperty("user.dir")+
"/DEVELOPING_OPAL/validate/target/scala-2.12/test-classes/org/opalj/fpcf/"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Having the fixed path here seems to be dangerous. Why is this needed at all?

* results on irreducible loops. For further information, see
* [[http://www.cs.princeton.edu/courses/archive/spring03/cs320/notes/loops.pdf]].
*/
def findNaturalLoops(): List[List[Int]] = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@errt or @pmuellerda, could you review that method?

PostDominatorTree(
if (exitNodes.length == 1) Some(exitNodes.head) else None,
i ⇒ exitNodes.contains(i),
// TODO: Pass an IntTrieSet if exitNodes contains more than one element
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like these two TODOs should be addressed before merging.

* instance of these classes are passed. Otherwise, an exception will be thrown!
*/
def processConcatOrOrCase(subtree: StringTree): StringTree = {
if (!subtree.isInstanceOf[StringTreeOr] && !subtree.isInstanceOf[StringTreeConcat]) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

There are a couple of isInstanceOf/asInstanceOf that could be replaced by pattern matching.

* the possible string values. This method returns either a final [[Result]] or an
* [[InterimResult]] depending on whether other information needs to be computed first.
*/
private def determinePossibleStrings(
Copy link
Collaborator

Choose a reason for hiding this comment

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

please try to reduce the method size.

* @return Returns a final result if (already) available. Otherwise, an intermediate result will
* be returned.
*/
private def continuation(
Copy link
Collaborator

Choose a reason for hiding this comment

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

please try to reduce the method size

*/
def isStringBuilderBufferToStringCall(expr: Expr[V]): Boolean =
expr match {
case VirtualFunctionCall(_, clazz, _, name, _, _, _) ⇒
Copy link
Collaborator

Choose a reason for hiding this comment

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

you could add "toString" as name directly

private def interpretToStringCall(
call: VirtualFunctionCall[V]
): EOptionP[Entity, StringConstancyProperty] =
// TODO: Can it produce an intermediate result???
Copy link
Collaborator

Choose a reason for hiding this comment

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

add an assertion

Patrick and others added 25 commits January 7, 2020 13:16
… another string variable in order to produce (more) correct results. Test methods were added as well.
…or those reflective calls where strings are input and then approximates these strings values using the string definition analysis.
… should capture all relevant calls) and is much faster).
… changed the the routine that detects dependees as well as the way the intermediate result(s) is handed to have the correct information at the end of the analysis.
…t "public static final") is read, it is not further analyzed but the lower bound is returned. However, fields marked as "public static final String..." can be analyzed (see added test methods).
…ting related files had to be changed. However, the test case 'secondStringBuilderRead' works now.
…d. This required refinements in different routines.
…within a project and how often.

# Conflicts:
#	DEVELOPING_OPAL/tools/src/main/scala/org/opalj/support/info/ClassUsageAnalysis.scala
…d items to the property store (as the interface has changed)
Patrick and others added 24 commits January 7, 2020 13:17
…mediate results to avoid endless loops that arise due to constantly changing upper bounds (for a discussion, see InterproceduralComputationState#appendToInterimFpe2Sci).
… be any string, from "\w" to ".*" as this complies with the regular expression semantics.
…igurable, i.e., a threshold when to approximate field reads as the lower bound.
…ittle bug was introduced which lead to the fact that "^null$" would not be added to the possible strings if there was not field write access. This is now fixed.
…bound. This lead to one little refinement in the VirtualFunctionCallPreparationInterpreter.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants