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

New fuzzing platform #1457

Merged
merged 9 commits into from
Dec 12, 2022
Merged

New fuzzing platform #1457

merged 9 commits into from
Dec 12, 2022

Conversation

Markoutte
Copy link
Collaborator

@Markoutte Markoutte commented Dec 6, 2022

Description

Adds new fuzzing platform that simplifies developing fuzzing for new languages. Old implementation is still in codebase but will be deprecated and removed soon. Only base Java implementation migrated to the new fuzzing platform at the moment.

Please, see Fuzzing Platform Design doc for understanding of basic concepts. In nutshell now any work with fuzzing requires some 'seeds' or 'tasks' which can be 4 types:

  1. Simple seed is just a value with optional mutator
  2. Known [structure] seed is something that has common structure for different types, e.g. integers (byte, short, int, long) which can be represented as a bit vector. This type can be used instead of 1 to faster developing of defaults such as integers, floats, strings, etc.
  3. Recursive seed can represent some recursive structures that require to fuzz some parameters before constructing itself.
  4. Collection seed represents collections such as arrays, lists, sets, maps and others.

There are some demos that clarify these concepts.

This platform is fully implemented for Java and plugin can be tested with these samples.

Fixes #1448

Also note, that after short discussion this implementation doesn't use mocks because it looks wrong concepts in terms of fuzzing. Fuzzing doesn't analyze the code therefore it cannot supply correct mock calls for tests. Therefore #747 is discontinued starting this PR.

Type of Change

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

How Has This Been Tested?

Regression and integration tests

Test samples for manual testing were added. Integration test for fuzzing is being under developing.

Automated Testing

All tests from the utbot-fuzzer and utbot-fuzzing modules must pass.

Manual Scenario

Samples for manual scenario.

Checklist:

  • The change followed the style guidelines of the UTBot project
  • Self-review of the code is passed
  • The change contains enough commentaries, particularly in hard-to-understand areas
  • New documentation is provided or existed one is altered
  • No new warnings
  • New tests have been added
  • All tests pass locally with my changes

@Markoutte Markoutte requested a review from denis-fokin December 7, 2022 08:13
)
}

private fun shouldPass(type: ClassId): Boolean {
Copy link
Collaborator

Choose a reason for hiding this comment

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

What "should pass" means in this context?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed to isIgnored(type: ClassId)

val getter: Method?
)

internal fun findSuitableFields(classId: ClassId, packageName: String?): List<FieldDescription> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would add a piece of documentation here. Suitable for what?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed to findAccessibleModifableFields(...)

@Markoutte Markoutte force-pushed the pelevin/fuzzing_platform branch from 6e39690 to 97d8336 Compare December 8, 2022 05:12
@Markoutte Markoutte requested a review from denis-fokin December 8, 2022 05:20
* @param description contains user-defined information about current run. Can be used as a state of the run.
* @param values current values to run.
*/
suspend fun run(description: DESCRIPTION, values: List<RESULT>): FEEDBACK
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please, find a better naming for the method. 'Run' makes me think that the method is supposed to be called only ones. I would suggest something like: 'hanlde', 'valueUdated' or something similar

@Markoutte Markoutte requested a review from denis-fokin December 8, 2022 12:00
Copy link
Collaborator

@SBOne-Kenobi SBOne-Kenobi left a comment

Choose a reason for hiding this comment

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

Amazing! LGTM :)

builder = PassRoutine("Main Routine"),
state = State(1, typeCache),
)
val dynamicallyGenerated = mutableListOf<Node<T, R>>()
Copy link
Collaborator

Choose a reason for hiding this comment

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

As I understand you use it as a queue, can you replace it with any instance of queue such as LinkedList or Deque.

Copy link
Collaborator

@volivan239 volivan239 left a comment

Choose a reason for hiding this comment

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

Looks monumental!

Comment on lines +13 to +19
fun String.findMaxSubstring(s: String) : Int {
if (s.isEmpty()) return -1
for (i in s.indices) {
if (s[i] != this[i]) return i - 1
}
return s.length
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the name is misleading, because the function looks like a strange variation of CharSequence.commonPrefixWith()

Also, it may throw StringIndexOutOfBoundsException if this is a strict prefix of s. If this is intentional, please, add a comment describing how fuzzing handle exceptions in this example.

/**
* Probability of creating shifted array values instead of generating new values for modification.
*/
var probCollectionMutationInsteadCreateNew: Int = 50,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here and below: imo, if probability is not in [0, 1] it is worth mentioning that it is in percents

Comment on lines 88 to 95
fun main() {
val endian = Endian.BE
println(255.toUByte().toBinaryString(endian))
println(2.toBinaryString(endian))
println(BitVectorValue.fromInt(2).toBinaryString(endian))
print(8.75f.toBinaryString(endian))
print(8.75.toBinaryString(endian))
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this should be either moved to tests or deleted. Same for IEEE754Value.kt

TrieBasedFuzzerStatistics(coveredInstructionValues), methodUnderTestDescription, *defaultModelMutators().toTypedArray()
)
fuzzedValues.forEach { values ->
val names = graph.body.method.tags.filterIsInstance<ParamNamesTag>().firstOrNull()?.names ?: emptyList()
Copy link
Collaborator

@volivan239 volivan239 Dec 12, 2022

Choose a reason for hiding this comment

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

This names initialization looks weird. Can't we just use methodUnderTest.parameters.map { it.name } here? If not, please, add comment reasoning such an initialization

@Markoutte Markoutte force-pushed the pelevin/fuzzing_platform branch from b70b8af to f759f20 Compare December 12, 2022 05:40
@Markoutte Markoutte merged commit 3542afb into main Dec 12, 2022
@Markoutte Markoutte deleted the pelevin/fuzzing_platform branch December 12, 2022 06:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integrate new fuzzing platform for Java
5 participants