Skip to content

Conversation

@amarekano
Copy link
Contributor

This PR adds support for finally blocks in try-catch statements.

@amarekano amarekano force-pushed the try-catch-finaliser branch from aecdc51 to ddf1367 Compare May 21, 2021 15:56
Copy link
Collaborator

@saelo saelo left a comment

Choose a reason for hiding this comment

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

Cool, thanks for implementing this! Left some comments, should be good to merge after addressing those :)

@amarekano
Copy link
Contributor Author

I've addressed your review comments in the latest commit.

Just a heads up, I have some concerns around the tryCatchFinally reducer and I'm seeing intermittent crashes when reducing try-catch-finally blocks. Consider the following program:

let v0 = 1337
try {
//... number of instructions
} catch {
let v0 = "foobar"
//... more catch block instructions
}

When the reducer tries to minimise this program, it attempts to Nop out the try block and the catch statement. So we end up with

let v0 = 1337
Noped BeginTry
Noped instructions
Noped BeginCatch
let v0 = "foobar"
//... catch block instructions
Noped EndTry

This generates invalid FuzzIL and fails static validation (due to v0 being re-defined) which triggers an assertion at runtime. I'm not sure if this is due to the bug in my code updates to the reducer (i.e. from the commit before addressing the review comments) or an issue elsewhere (e.g. should we Nop out the catch and finally blocks when reducing?). Thoughts?

I'm going to run the fuzzer with the latest changes to this PR and see if I can recreate these crashes and report back,

@saelo
Copy link
Collaborator

saelo commented May 31, 2021

Ok interesting. So I think

let v0 = 1337
try {
//... number of instructions
} catch {
let v0 = "foobar"
//... more catch block instructions
}

should already be invalid FuzzIL since v0 is declared two time (this should catch it). So I'm not sure what might have happened there. I'll take another look at the reducer, but I agree with you, running it for a bit and monitoring the outputs is probably the best approach here.

@amarekano amarekano force-pushed the try-catch-finaliser branch from 4abd82b to 706d886 Compare June 1, 2021 11:06
@amarekano
Copy link
Contributor Author

I've run the fuzzer built from commit ad7449d for a few hours with the following configuration and bumping up the weight on the TryCatchGenerator:

.build/debug/FuzzilliCli --profile=jsc --engine=multi --timeout=800 --logLevel=info --jobs=16 --storagePath=../fuzz_test --overwrite ./jsc

I haven't noticed a crash/debug assertion related to try catch generation or reduction but the debug build did crash with the following assertion:

Fuzzilli/ProgramBuilder.swift:508: Fatal error: Unexpectedly found nil while unwrapping an Optional value
Current stack trace:

0    libswiftCore.so                    0x00007ffff7dfd210 swift_reportError + 50
1    libswiftCore.so                    0x00007ffff7e71fc0 _swift_stdlib_reportFatalErrorInFile + 112
2    libswiftCore.so                    0x00007ffff7b5bf56 <unavailable> + 1425238
3    libswiftCore.so                    0x00007ffff7b5bb7f <unavailable> + 1424255
4    libswiftCore.so                    0x00007ffff7b5b91c <unavailable> + 1423644
5    libswiftCore.so                    0x00007ffff7b5b430 _assertionFailure(_:_:file:line:flags:) + 441
6    FuzzilliCli                        0x00005555557a5bbf <unavailable> + 2431935
7    FuzzilliCli                        0x00005555557a5c44 <unavailable> + 2432068
8    FuzzilliCli                        0x00005555557b6f04 <unavailable> + 2502404
9    libswiftCore.so                    0x00007ffff7b428d0 Collection.map<A>(_:) + 524
10   FuzzilliCli                        0x00005555557a1400 ProgramBuilder.generateVariable(ofType:) + 11788
11   FuzzilliCli                        0x00005555556fdd63 <unavailable> + 1744227
12   FuzzilliCli                        0x00005555556f6657 <unavailable> + 1713751
13   FuzzilliCli                        0x00005555556f6691 <unavailable> + 1713809
14   FuzzilliCli                        0x00005555556f6a40 CodeGenerator.run(in:with:) + 146
15   FuzzilliCli                        0x00005555557aaf40 ProgramBuilder.run(_:recursiveCodegenBudget:) + 2214
16   FuzzilliCli                        0x00005555557ac08f <unavailable> + 2457743
17   FuzzilliCli                        0x00005555557038bc <unavailable> + 1767612
18   FuzzilliCli                        0x00005555557b8871 <unavailable> + 2508913
19   FuzzilliCli                        0x0000555555a2df60 withEqualProbability<A>(_:) + 103
20   FuzzilliCli                        0x00005555557ab9b0 ProgramBuilder.generateInternal() + 878
21   FuzzilliCli                        0x00005555557ac0c0 ProgramBuilder.generate(n:) + 219
22   FuzzilliCli                        0x00005555557bdfdc <unavailable> + 2531292
23   FuzzilliCli                        0x00005555557b9a00 ProgramTemplate.generate(in:) + 281
24   FuzzilliCli                        0x0000555555717ee0 HybridEngine.generateTemplateProgram(baseTemplate:) + 849
25   FuzzilliCli                        0x00005555557183f0 HybridEngine.fuzzOne(_:) + 740
26   FuzzilliCli                        0x00005555557194f1 <unavailable> + 1856753
27   FuzzilliCli                        0x0000555555790210 MultiEngine.fuzzOne(_:) + 240
28   FuzzilliCli                        0x0000555555790b91 <unavailable> + 2345873
29   FuzzilliCli                        0x000055555587bf90 Fuzzer.fuzzOne() + 893

From the stack trace it would seem that this is an issue unrelated to the current PR but it's probably worth investigating and tracking as a separate issue.

I've rebased this PR with the latest changes from main so this should be good to merge.

@saelo saelo merged commit 2e8f648 into googleprojectzero:master Jun 2, 2021
@saelo
Copy link
Collaborator

saelo commented Jun 2, 2021

Cool! Yeah that crash indeed looks unrelated. It's unfortunately not really actionable without line numbers I guess, but if I get some time I'll try to reproduce it. Thanks!

@amarekano
Copy link
Contributor Author

quick update on the crashes. I've upgraded my setup from Swift 5.4 to 5.4.1 and I no longer see crashes. I think this was a Swift quirk that was addressed with the update. Apologies, if I've set you on a wild goose chase. I don't think this requires any further investigation :)

@amarekano amarekano deleted the try-catch-finaliser branch July 25, 2021 12:02
Dudcom pushed a commit to VRIG-RITSEC/fuzzillai that referenced this pull request Oct 29, 2025
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.

2 participants