Page MenuHomeWildfire Games

Reliably serialize NaN values.
Needs ReviewPublic

Authored by wraitii on Dec 10 2020, 5:52 PM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Trac Tickets
#1879
Summary

NaN values can take a number of different balues depending on the architecture and how they came to be. This can OOS spuriously.

This does two things:

  • make the float/double 'put' functions protected, so they can't be called accidentally from sim code, potentially preventing issues.
  • make the script-serializer detect NaN and serialize them specially. I add a boolean for whether the nan is the Number Object or just a primitive number, since that seems more straightforward.
Test Plan

Serialize NaN values.

Unit TestsFailed

TimeTest
0 msJenkins > cxxtest_debug.xml::[failed-to-read]
Failed to read test report file E:\Jenkins\workspace\vs2015-differential\cxxtest_debug.xml org.dom4j.DocumentException: Error on line 347 of document : Content is not allowed in trailing section. at org.dom4j.io.SAXReader.read(SAXReader.java:511)
0 msJenkins > TestAllocators::test_da
0 msJenkins > TestAllocators::test_da
0 msJenkins > TestAllocators::test_da
0 msJenkins > TestAtlasObjectXML::test_parse_attributes1
View Full Test Results (1 Failed · 1,021 Passed)

Event Timeline

wraitii created this revision.Dec 10 2020, 5:52 PM

Build failure - The Moirai have given mortals hearts that can endure.

builderr-debug-macos.txt
fatal error: file '/Users/wfg/Jenkins/workspace/macos-differential/build/workspaces/gcc/../../../libraries/source/spidermonkey/include-unix-debug/mozilla/LinkedList.h' has been modified since the precompiled header 'obj/network_Debug/precompiled.h.gch' was built
note: please rebuild precompiled header 'obj/network_Debug/precompiled.h.gch'
1 error generated.
make[1]: *** [obj/network_Debug/NetMessageSim.o] Error 1
make: *** [network] Error 2

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/2375/display/redirect

Successful build - Chance fights ever on the side of the prudent.

builderr-release-gcc7.txt
In file included from ../../../source/pch/atlas/precompiled.h:26:
../../../source/tools/atlas/GameInterface/Messages.h: In function 'void AtlasMessage::fGetTerrainGroupPreviews(AtlasMessage::qGetTerrainGroupPreviews*)':
../../../source/tools/atlas/GameInterface/Messages.h:315:8: warning: '#'target_mem_ref' not supported by dump_expr#<expression error>' may be used uninitialized in this function [-Wmaybe-uninitialized]
 struct sTerrainTexturePreview
        ^~~~~~~~~~~~~~~~~~~~~~
../../../source/tools/atlas/GameInterface/Messages.h:315:8: warning: '#'target_mem_ref' not supported by dump_expr#<expression error>' may be used uninitialized in this function [-Wmaybe-uninitialized]
../../../source/tools/atlas/GameInterface/Messages.h:315:8: warning: '#'target_mem_ref' not supported by dump_expr#<expression error>' may be used uninitialized in this function [-Wmaybe-uninitialized]
../../../source/tools/atlas/GameInterface/Messages.h:315:8: warning: '#'target_mem_ref' not supported by dump_expr#<expression error>' may be used uninitialized in this function [-Wmaybe-uninitialized]
 struct sTerrainTexturePreview
        ^~~~~~~~~~~~~~~~~~~~~~
../../../source/tools/atlas/GameInterface/Messages.h:315:8: warning: '#'target_mem_ref' not supported by dump_expr#<expression error>' may be used uninitialized in this function [-Wmaybe-uninitialized]
../../../source/tools/atlas/GameInterface/Messages.h:315:8: warning: '#'target_mem_ref' not supported by dump_expr#<expression error>' may be used uninitialized in this function [-Wmaybe-uninitialized]
In file included from ../../../source/lib/os_path.h:26,
                 from ../../../source/lib/self_test.h:28,
                 from ../../../source/simulation2/system/ComponentTest.h:21,
                 from ../../../source/pch/test/precompiled.h:22:
../../../source/lib/path.h: In member function 'virtual void TestMapGenerator::setUp()':
../../../source/lib/path.h:265:68: warning: '<anonymous>.Path::separator' may be used uninitialized in this function [-Wmaybe-uninitialized]
   debug_printf("Path %s, separator %c\n", string8().c_str(), (char)separator);
                                                                    ^~~~~~~~~
In file included from ../../../source/lib/os_path.h:26,
                 from ../../../source/lib/self_test.h:28,
                 from ../../../source/simulation2/system/ComponentTest.h:21,
                 from ../../../source/pch/test/precompiled.h:22:
../../../source/lib/path.h: In member function 'virtual void TestComponentScripts::setUp()':
../../../source/lib/path.h:265:68: warning: '<anonymous>.Path::separator' may be used uninitialized in this function [-Wmaybe-uninitialized]
   debug_printf("Path %s, separator %c\n", string8().c_str(), (char)separator);
                                                                    ^~~~~~~~~
In file included from ../../../source/lib/os_path.h:26,
                 from ../../../source/lib/self_test.h:28,
                 from ../../../source/simulation2/system/ComponentTest.h:21,
                 from ../../../source/pch/test/precompiled.h:22:
../../../source/lib/path.h: In function 'OsPath DataDir()':
../../../source/lib/path.h:265:68: warning: '<anonymous>.Path::separator' may be used uninitialized in this function [-Wmaybe-uninitialized]
   debug_printf("Path %s, separator %c\n", string8().c_str(), (char)separator);
                                                                    ^~~~~~~~~

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/4035/display/redirect

wraitii requested review of this revision.Dec 10 2020, 6:35 PM
Stan added a subscriber: Stan.Dec 10 2020, 6:44 PM

Does it handle QNaN floats too? I don't know if those exist in JS;

source/simulation2/components/CCmpMotionBall.cpp
58

unsafe?

source/simulation2/serialization/BinarySerializer.h
83

+ in the simulation?

187

Doesn't work?

Don't you get NaN only when stuff went wrong?

Don't you get NaN only when stuff went wrong?

I would say generally yes, but that remains a confusing source of OOS and it'd be neater to fix it I suppose.

We can't warn/error on NaN?

We can't warn/error on NaN?

I can add that yes

Stan added a comment.Mon, Jan 4, 3:01 PM

Should we push the ticket or do you plan to update this?

source/simulation2/serialization/BinarySerializer.cpp
1

2021

source/simulation2/serialization/BinarySerializer.h
184

reorder?

source/simulation2/serialization/ISerializer.h
1

2021

source/simulation2/serialization/SerializedScriptTypes.h
1

2021

source/simulation2/serialization/StdDeserializer.cpp
1

2021

source/simulation2/tests/test_Serializer.h
1

2021