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

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

Open
rogual opened this issue Dec 1, 2023 · 11 comments
Open

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

rogual opened this issue Dec 1, 2023 · 11 comments
Labels
bug Something isn't working

Comments

@rogual
Copy link
Contributor

rogual commented Dec 1, 2023

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

@rogual rogual added the bug Something isn't working label Dec 1, 2023
@Dinnerbone
Copy link
Contributor

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

@torokati44
Copy link
Member

Yeah, but over there: ruffle-rs/ruffle-android#63

@torokati44
Copy link
Member

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

@torokati44
Copy link
Member

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
Copy link
Member

@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
Copy link
Contributor Author

rogual commented Dec 6, 2023

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
Copy link
Member

Thank you!

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

  • Serializing any f64::is_nan() as a fixed "canonical" sequence of bytes.
  • Setting Number.NaN in AVM2 to a fixed "canonical" constant (f64::NAN isn't actually fully defined, especially the sign bit).

But neither fixed all tests.

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

@rogual
Copy link
Contributor Author

rogual commented Dec 7, 2023

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
Copy link
Member

torokati44 commented Dec 7, 2023

Well the NaN builtin for AVM2 is set here (AFAIK):

("MIN_VALUE", f64::MIN_POSITIVE),
("NaN", f64::NAN),
("NEGATIVE_INFINITY", f64::NEG_INFINITY),

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
Copy link
Contributor Author

rogual commented Dec 7, 2023

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:

public const NaN: Number = 0 / 0;

@torokati44
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants