Stream: git-wasmtime

Topic: wasmtime / issue #6208 Chaos mode: add fuel parameter

Wasmtime GitHub notifications bot (Apr 13 2023 at 10:28):

remlse commented on issue #6208:

@cfallin I don't think I've done it the way you expected me to. I might need a little more help.

In my mind, the fuel parameter cannot be constructed in the same way as the other flags (see here). The other flags are generated by the fuzzer, but we specifically don't want that for the fuel parameter. That will pretty much always be manually set, right?

I also don't see a way to explicitly pass the fuel parameter to the construction of control planes (or cranelift flags for that matter) in the Arbitrary implementation of TestCase... so I simply set the fuel manually after construction.

I've tried to add a new cranelift flag but it's not even being used at this stage.

Wasmtime GitHub notifications bot (Apr 13 2023 at 12:51):

github-actions[bot] commented on issue #6208:

Subscribe to Label Action

cc @fitzgen

<details>
This issue or pull request has been labeled: "cranelift", "cranelift:meta", "fuzzing"

Thus the following users have been cc'd because of the following labels:

• fitzgen: fuzzing

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

Wasmtime GitHub notifications bot (Apr 27 2023 at 13:52):

remlse commented on issue #6208:

Hi @cfallin , we've added the control plane to the printed test case output, example:

        set enable_table_access_spectre_mitigation=false
        target x86_64 has_sse42 has_avx has_avx2 has_fma has_popcnt has_bmi1 has_bmi2 has_lzcnt

        control plane:
            data: true
            fuel: None

        function u1:0() system_v {
            sig0 = (f32) -> f32 system_v

I'll try to add some context about what we've done so far in this PR.

The fuel parameter is manually set when fuzzing via a command line argument, like so:

cargo fuzz run --features chaos cranelift-fuzzgen -- --fuel=16

This is read (very crudely) in the fuzz target (here). The same fuel parameter is set for all control planes, but I think that should be fine if functions compile independently from each other.

The fuel is stored in the control plane alongside the data received from the fuzzer. It's represented as an Option<u32> to indicate if the fuel parameter has even been activated. Calling a control plane method consumes one unit of fuel.

With the default value of the fuel parameter (None) consuming fuel always succeeds.

The crate documentation now includes a section about the fuel parameter.

What do you think about this?

Wasmtime GitHub notifications bot (Apr 27 2023 at 17:17):

jameysharp commented on issue #6208:

To provide a little context to Chris' comments, the reason he suggested printing the control-plane info like

test compile chaos=true,true,false

is because the Debug output for these fuzz cases is supposed to be valid input for clif-util test, which helps us to reproduce and diagnose fuzz failures. (I usually run this as cargo run -p cranelift-tools test and you can find it in cranelift/src/.)

Adding text like this in the middle won't parse:

        control plane:
            data: true
            fuel: None

I'm not sure his suggested syntax of chaos=... will parse right now either, but it's more consistent with how we've done other things. We can certainly discuss other choices of syntax, but we should pick something with a structure that we can parse easily, not just something for human consumption.

That said, personally I think it would be fine to print this information in a comment (each line starting with ;) at first, and figure out the syntax details later.

Wasmtime GitHub notifications bot (Apr 27 2023 at 18:03):

remlse commented on issue #6208:

That makes sense! I added a ; at the start of each line.

output:

        set enable_table_access_spectre_mitigation=false
        target x86_64 has_sse42 has_avx has_avx2 has_fma has_popcnt has_bmi1 has_bmi2 has_lzcnt

        ; control plane:
        ;    data: true
        ;    fuel: None

        function u1:0() system_v {
            sig0 = (f32) -> f32 system_v

Wasmtime GitHub notifications bot (May 05 2023 at 03:26):

remlse commented on issue #6208:

Still missing from the parsing:

• [ ] using the cranelift setting to set the fuel limit on all parsed control planes
• [ ] making the parsing logic less barbaric / fit with the rest

I might not get around to that before the weekend.

Wasmtime GitHub notifications bot (May 05 2023 at 03:26):

remlse edited a comment on issue #6208:

Still missing from the parsing:

• [ ] using the cranelift setting to set the fuel limit on all parsed control planes
• [ ] making the parsing logic less barbaric / fit with the rest
• [ ] fix tests

I might not get around to that before the weekend.

Last updated: Nov 22 2024 at 16:03 UTC