ruffle-rs / ruffle

A Flash Player emulator written in Rust
https://ruffle.rs
Other
15.53k stars 805 forks source link

Test failing on Apple Silicon: avm2/amf_vector (Serialization of NaN) #14269

Open rogual opened 10 months ago

rogual commented 10 months ago

Describe the bug

The test avm2/amf_vector fails on my machine (M2 Mac).

Test result:

 Original: [0,0,-1,Infinity,5,NaN] fixed: false class: __AS3__.vec::Vector.<Number>
<Serialized: 15,13,0,128,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,191,240,0,0,0,0,0,0,127,240,0,0,0,0,0,0,64,20,0,0,0,0,0,0,127,248,0,0,0,0,0,0
>Serialized: 15,13,0,128,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,191,240,0,0,0,0,0,0,127,240,0,0,0,0,0,0,64,20,0,0,0,0,0,0,255,248,0,0,0,0,0,0
 Deserialized: [0,0,-1,Infinity,5,NaN] fixed: false class: __AS3__.vec::Vector.<Number>

It's the NaN that's serializing differently from in Flash Player.

The test is expecting the NaN to be serialized as 0xfff8000000000000, but Ruffle for me is giving 0x7ff8000000000000.

These are both valid NaNs.

I would suggest either:

  1. Amending the test so it doesn't care about the precise serialization of NaN (it would still check the encode/decode round-trip), or
  2. Ensuring NaN is always encoded with the sign bit set.

I'd be surprised if this broke any real content, so would probably lean towards 1?

As for option 2, if we wanted to fix it: I'm not quite sure how the NaN is stored in the test SWF. I guess it's probably a reference to the precompiled NaN in playerglobals.swf? In that case it's the compiler that compiles playerglobals.swf that's causing the difference. But I didn't look into it much.

Expected behavior

NaN serialized with sign bit set.

Content Location

tests/tests/swfs/avm2/amf_vector/test.swf

Affected platform

Desktop app

Operating system

13.5 (22G74)

Browser

No response

Additional information

No response

Dinnerbone commented 10 months ago

@torokati44: You noticed an AMF test failing on android/ARM - did you ever open an issue for that?

torokati44 commented 10 months ago

Yeah, but over there: https://github.com/torokati44/ruffle-android/issues/63

torokati44 commented 10 months ago

But AFAIK GH now has M1 CI runners, we can try enabling those too.

torokati44 commented 10 months ago

Oooh, but:

Who can use this feature Larger runners are only available for organizations and enterprises using the GitHub Team or GitHub Enterprise Cloud plans.

https://docs.github.com/en/actions/using-github-hosted-runners/about-larger-runners/about-larger-runners

How dare they... Lemme fetch my RPi then...

torokati44 commented 10 months ago

@rogual: Would you mind cloning this repo, and running cargo test --all in it, and see whether anything fails? https://github.com/ruffle-rs/rust-flash-lso

rogual commented 10 months ago

All passed.

Output ``` $ cargo test --all warning: virtual workspace defaulting to `resolver = "1"` despite one or more workspace members being on edition 2021 which implies `resolver = "2"` note: to keep the current resolver, specify `workspace.resolver = "1"` in the workspace root's manifest note: to use the edition 2021 resolver, specify `workspace.resolver = "2"` in the workspace root's manifest note: for more details see https://doc.rust-lang.org/cargo/reference/resolver.html#resolver-versions Finished test [unoptimized + debuginfo] target(s) in 0.24s Running unittests src/lib.rs (target/debug/deps/flash_lso-4478f56657f15fb4) running 10 tests test amf0::writer::fff ... ok test amf3::read::read_number_tests::read_neg_number ... ok test amf3::read::read_number_tests::test_read_1byte_number_unsigned ... ok test amf3::read::read_number_tests::test_read_4byte_number_unsigned ... ok test amf3::read::read_number_tests::read_neg_number_unsigned ... ok test amf3::read::read_number_tests::test_read_1byte_number ... ok test amf3::read::read_number_tests::test_read_4byte_number ... ok test amf3::write::write_number_tests::test_write_4byte_number ... ok test amf3::write::write_number_tests::test_write_1byte_number ... ok test amf3::write::write_number_tests::write_neg_number ... ok test result: ok. 10 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s Running tests/integration_tests.rs (target/debug/deps/integration_tests-3f8d79016d022966) running 146 tests test as2_integer ... ok test as2_array ... ok test as2_boolean ... ok test as2_date ... ok test as2_ecma_array ... ok test akamai_enterprise_player ... ok test armorgames_auth_request ... ok test armorgames_auth_response ... ok test as2_half_life ... ok test as2_null ... ok test as2_number ... ok test as2_object ... ok test as2_string ... ok test as2_typed_object ... ok test as2_undefined ... ok test as2_xml ... ok test as3_boolean ... ok test as3_byte_array ... ok test as3_date ... ok test as3_null ... ok test as3_demo ... ok test as3_integer ... ok test as3_number ... ok test as3_dictionary ... ok test as3_string ... ok test as3_strict_array ... ok test as3_undefined ... ok test as3_typed_object ... ok test as3_object ... ok test as3_vector_int ... ok test as3_vector_number ... ok test as3_vector_object ... ok test as2_long_string ... ok test areana_madness_game_two ... ok test as3_vector_unsigned_int ... ok test as3_xml ... ok test as3_vector_typed_object ... ok test as3_xml_doc ... ok test canvas ... ok test com_jeroenwijering ... ok test cramjs ... ok test dolphin_show_1 ... ok test clarence_save_slot_1 ... ok test flagstaff_1 ... ok test json_akamai_enterprise_player ... ok test json_as2_boolean ... ok test json_as2_array ... ok test json_as2_date ... ok test json_as2_ecma_array ... ok test json_as2_integer ... ok test json_as2_half_life ... ok test json_as2_number ... ok test json_as2_null ... ok test json_areana_madness_game_two ... ok test json_as2_object ... ok test json_as2_string ... ok test json_as2_typed_object ... ok test json_as2_undefined ... ok test json_as2_xml ... ok test json_as3_boolean ... ok test json_as3_date ... ok test json_as3_byte_array ... ok test json_as3_integer ... ok test json_as3_demo ... ok test json_as3_dictionary ... ok test json_as2_long_string ... ok test json_as3_null ... ok test json_as3_number ... ok test json_as3_object ... ok test json_as3_strict_array ... ok test json_as3_typed_object ... ok test json_as3_string ... ok test json_as3_undefined ... ok test json_as3_vector_int ... ok test json_as3_vector_number ... ok test json_as3_vector_object ... ok test json_as3_vector_unsigned_int ... ok test json_as3_vector_typed_object ... ok test json_as3_xml ... ok test json_as3_xml_doc ... ok test json_canvas ... ok test json_com_jeroenwijering ... ok test json_cramjs ... ok test json_dolphin_show_1 ... ok test json_flagstaff_1 ... ok test json_clarence_save_slot_1 ... ok test flagstaff_2 ... ok test json_flagstaff_2 ... ok test json_flash_viewer ... ok test fish_tycoon ... ok test hiro_network_capping_cookie ... ok test json_labrat_2 ... ok test json_media_player_user_settings ... ok test json_coc_8 ... ok test json_hiro_network_capping_cookie ... ok test json_minimal ... ok test json_minimal_2 ... ok test json_metadata_history ... ok test json_opp_detail_prefs ... ok test json_previous_video ... ok test json_self_referential ... ok test json_settings ... ok test json_mardek_v3_sg_1 ... ok test json_slot_1_party ... ok test json_sound_data ... ok test json_sound_data_level_0 ... ok test json_robokill ... ok test json_space ... ok test json_time_display_config ... ok test json_string_test ... ok test coc_8 ... ok test json_user ... ok test json_user_1 ... ok test as2_demo ... ok test media_player_user_settings ... ok test metadata_history ... ok test json_learntofly3 ... ok test minimal ... ok test flash_viewer ... ok test minimal_2 ... ok test json_as2_demo ... ok test previous_video ... ok test self_referential ... ok test opp_detail_prefs ... ok test json_party_1 ... ok test slot_1_party ... ok test sound_data ... ok test settings ... ok test sound_data_level_0 ... ok test space ... ok test time_display_config ... ok test two ... ok test labrat_2 ... ok test string_test ... ok test user_1 ... ok test robokill ... ok test user ... ok test party_1 ... ok test mardek_v3_sg_1 ... ok test zero_four ... ok test slot_1 ... ok test json_jy1 ... ok test jy1 ... ok test json_slot_1 ... ok test infectonator_survivors_76561198009932603 ... ok test json_infectonator_survivors_76561198009932603 ... ok test result: ok. 146 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.28s Running unittests src/main.rs (target/debug/deps/lso_to_json-e60df5e83f078848) running 0 tests test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s Running unittests src/lib.rs (target/debug/deps/web-a583d99a410de171) running 0 tests test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s Doc-tests flash-lso running 1 test test flash-lso/src/read.rs - read::Reader (line 24) ... ok test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.41s Doc-tests web running 0 tests test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s ```
torokati44 commented 10 months ago

Thank you!

I went a few circles around this, trying the following ideas:

But neither fixed all tests.

So for now, I'm not sure which way to go next.

rogual commented 10 months ago

If ActionScript 3 source code mentions NaN, does that not become a reference to the NaN constant in playerglobals.swf? If so, the bytes of that NaN won't be generated from the Rust compiler at all, it'll be compiled into playerglobals by the Java-based Macromedia compiler that's called from ruffle/core/build_playerglobal. That might be what's behaving differently on different CPUs.

(That's if I understand how playerglobals works... which it's very possible I don't.)

torokati44 commented 9 months ago

Well the NaN builtin for AVM2 is set here (AFAIK): https://github.com/ruffle-rs/ruffle/blob/41ddb316a2b0a91c0d32e8f54eb0f020be42831f/core/src/avm2/globals/number.rs#L400-L402

But there are lots of other mentions of f64::NAN in the AVM2 code too, for example in the native implementations of the Date and Point classes.

rogual commented 9 months ago

Yes, if thoseNaNs are ever observable as bytes, which they probably are, then it might be worth Ruffle defining its own NAN constant and using that everywhere.

But I don't think that will fix this failing test. I might be wrong, but I couldn't find a similar Rust declaration for the value you get by typing NaN in AS3, which is why I suspect it's loaded from playerglobals; specifically, here:

https://github.com/ruffle-rs/ruffle/blob/41ddb316a2b0a91c0d32e8f54eb0f020be42831f/core/src/avm2/globals/Toplevel.as#L4

torokati44 commented 9 months ago

Oh, that's a good point, yeah. I did try to look specifically for the builtin NaN (as opposed to the one in Number), but I was in a hurry and didn't find the entirely logical place for it, Toplevel.as. 😅

I'll hopefully get back to experimenting with this in a couple of days.