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

[iOS/32bit/device] mscorlib test failures: DoubleFormatterTest.TestFormatStringsN# and DoubleTest.LongLongValueRoundtrip #11965

Closed
rolfbjarne opened this issue Dec 7, 2018 · 16 comments · Fixed by xamarin/xamarin-android#2729
Assignees

Comments

@rolfbjarne
Copy link
Member

@rolfbjarne rolfbjarne commented Dec 7, 2018

Steps to Reproduce

  1. Checkout & build xamarin-macios/master (this started happening after the bump to mono-2018-08).
  2. Run the 32-bit mscorlib tests (this does not happen in 64-bit)
cd tests
make run-ios-dev32-mscorlib

Current Behavior

Test failures:

	[FAIL] DoubleFormatterTest.TestFormatStringsN1 :   DblFn1 #216
	[FAIL] DoubleFormatterTest.TestFormatStringsN1_Fixed :   DblFn1 #200
	[FAIL] DoubleFormatterTest.TestFormatStringsN2 :   DblFn2 #216
	[FAIL] DoubleTest.LongLongValueRoundtrip :   Expected string length 21 but was 1. Strings differ at index 0.
Tests run: 8095 Passed: 8090 Inconclusive: 1 Failed: 4 Ignored: 37

Expected Behavior

No test failures.

Version Used:

xamarin-macios/master (2e72e1aee04d33e5857981bcd3943db5bf806856)

Detailed test output

DoubleFormatterTest
	[PASS] DoubleFormatterTest.NegativeRoundtrip
	[PASS] DoubleFormatterTest.NonStandardRounding
	[PASS] DoubleFormatterTest.Roundtrip
	[PASS] DoubleFormatterTest.Roundtrip_ExactStringFormat
	[FAIL] DoubleFormatterTest.TestFormatStringsN1 :   DblFn1 #216
  Expected string length 21 but was 1. Strings differ at index 0.
  Expected: "4,94065645841247E-324"
  But was:  "0"
  -----------^

		  at MonoTests.System.DoubleFormatterTest.FormatStringTest (System.String TestNumber, System.Globalization.NumberFormatInfo NumberFormat, System.Double Number, System.String Format, System.String ExpectedResult) [0x00001] in /work/maccore/master/xamarin-macios/external/mono/mcs/class/corlib/Test/System/DoubleFormatterTest.cs:59 
		  at MonoTests.System.DoubleFormatterTest.TestFormatStringsN1 () [0x01608] in /work/maccore/master/xamarin-macios/external/mono/mcs/class/corlib/Test/System/DoubleFormatterTest.cs:420 
		  at (wrapper managed-to-native) System.Reflection.MonoMethod.InternalInvoke(System.Reflection.MonoMethod,object,object[],System.Exception&)
		  at System.Reflection.MonoMethod.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x0003b] in /work/maccore/master/xamarin-macios/external/mono/mcs/class/corlib/System.Reflection/MonoMethod.cs:305 
	[FAIL] DoubleFormatterTest.TestFormatStringsN1_Fixed :   DblFn1 #200
  String lengths are both 13. Strings differ at index 0.
  Expected: "4,940656E-324"
  But was:  "0,000000E+000"
  -----------^

		  at MonoTests.System.DoubleFormatterTest.FormatStringTest (System.String TestNumber, System.Globalization.NumberFormatInfo NumberFormat, System.Double Number, System.String Format, System.String ExpectedResult) [0x00001] in /work/maccore/master/xamarin-macios/external/mono/mcs/class/corlib/Test/System/DoubleFormatterTest.cs:59 
		  at MonoTests.System.DoubleFormatterTest.TestFormatStringsN1_Fixed () [0x00408] in /work/maccore/master/xamarin-macios/external/mono/mcs/class/corlib/Test/System/DoubleFormatterTest.cs:173 
		  at (wrapper managed-to-native) System.Reflection.MonoMethod.InternalInvoke(System.Reflection.MonoMethod,object,object[],System.Exception&)
		  at System.Reflection.MonoMethod.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x0003b] in /work/maccore/master/xamarin-macios/external/mono/mcs/class/corlib/System.Reflection/MonoMethod.cs:305 
	[FAIL] DoubleFormatterTest.TestFormatStringsN2 :   DblFn2 #216
  Expected string length 21 but was 1. Strings differ at index 0.
  Expected: "4.94065645841247E-324"
  But was:  "0"
  -----------^

		  at MonoTests.System.DoubleFormatterTest.FormatStringTest (System.String TestNumber, System.Globalization.NumberFormatInfo NumberFormat, System.Double Number, System.String Format, System.String ExpectedResult) [0x00001] in /work/maccore/master/xamarin-macios/external/mono/mcs/class/corlib/Test/System/DoubleFormatterTest.cs:59 
		  at MonoTests.System.DoubleFormatterTest.TestFormatStringsN2 () [0x01608] in /work/maccore/master/xamarin-macios/external/mono/mcs/class/corlib/Test/System/DoubleFormatterTest.cs:764 
		  at (wrapper managed-to-native) System.Reflection.MonoMethod.InternalInvoke(System.Reflection.MonoMethod,object,object[],System.Exception&)
		  at System.Reflection.MonoMethod.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x0003b] in /work/maccore/master/xamarin-macios/external/mono/mcs/class/corlib/System.Reflection/MonoMethod.cs:305 
	[PASS] DoubleFormatterTest.TestNumberDecimals
	[PASS] DoubleFormatterTest.TestToDecimal
	[PASS] DoubleFormatterTest.TestToHex
	[PASS] DoubleFormatterTest.TestToUnknown
DoubleFormatterTest : 15 ms

DoubleTest
	[PASS] DoubleTest.CompareTo
	[PASS] DoubleTest.Equals
	[PASS] DoubleTest.HexNumber_NoHexToParse
	[PASS] DoubleTest.HexNumber_WithHexToParse
	[PASS] DoubleTest.IsInfinity
	[PASS] DoubleTest.IsNan
	[PASS] DoubleTest.IsNegativeInfinity
	[PASS] DoubleTest.IsPositiveInfinity
	[FAIL] DoubleTest.LongLongValueRoundtrip :   Expected string length 21 but was 1. Strings differ at index 0.
  Expected: "1,97626258336499E-323"
  But was:  "0"
  -----------^

		  at MonoTests.System.DoubleTest.LongLongValueRoundtrip () [0x00027] in /work/maccore/master/xamarin-macios/external/mono/mcs/class/corlib/Test/System/DoubleTest.cs:558 
		  at (wrapper managed-to-native) System.Reflection.MonoMethod.InternalInvoke(System.Reflection.MonoMethod,object,object[],System.Exception&)
		  at System.Reflection.MonoMethod.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x0003b] in /work/maccore/master/xamarin-macios/external/mono/mcs/class/corlib/System.Reflection/MonoMethod.cs:305 
	[PASS] DoubleTest.Parse
	[PASS] DoubleTest.Parse_Infinity
	[PASS] DoubleTest.Parse_TrailingGarbage
	[PASS] DoubleTest.Parse_Whitespace
	[PASS] DoubleTest.ParseAdvanced
	[PASS] DoubleTest.ParseAllowWhitespaces
	[PASS] DoubleTest.ParseCurrency
	[PASS] DoubleTest.ParseEmptyNumberGroupSeparator
	[PASS] DoubleTest.PublicFields
	[PASS] DoubleTest.TestRoundtrip
	[PASS] DoubleTest.TestToString
	[PASS] DoubleTest.TestTypeCode
	[PASS] DoubleTest.ToString_Defaults
	[PASS] DoubleTest.TryParse_NonDigitStrings
	[PASS] DoubleTest.TryParseBug78546
DoubleTest : 28 ms
@VincentDondain

This comment has been minimized.

Copy link
Contributor

@VincentDondain VincentDondain commented Dec 11, 2018

xamarin/xamarin-macios#5258 shows 2 additional issues as part of this test suite:

[FAIL] Rfc2898DeriveBytesTest.ConstructorPasswordSaltLengthZero : An unexpected exception type was thrown
[FAIL] Rfc2898DeriveBytesTest.ConstructorPasswordSaltLengthZeroIterations : An unexpected exception type was thrown

Edit: created a separate issue for that as it seems unrelated #12699

@lewurm

This comment has been minimized.

Copy link
Member

@lewurm lewurm commented Dec 12, 2018

is that Debug (fullAOT) or Release (fullAOT+LLVM)?

@rolfbjarne

This comment has been minimized.

Copy link
Member Author

@rolfbjarne rolfbjarne commented Dec 12, 2018

@lewurm Debug

@lewurm

This comment has been minimized.

Copy link
Member

@lewurm lewurm commented Dec 12, 2018

I can't reproduce it on armv7/linux (with fullAOT) 😕 going to need to check on a 32bit iOS device when I'm home.

@lewurm

This comment has been minimized.

Copy link
Member

@lewurm lewurm commented Dec 23, 2018

can reproduce on armv7/iOS, in both Debug (fullAOT) and Release (fullAOT+LLVM) configurations

@lewurm

This comment has been minimized.

Copy link
Member

@lewurm lewurm commented Jan 30, 2019

I'm pretty certain it's related to swapping the BCL code from ours/referencesource to corefx: 2a4af93 unfortunately can't easily revert it... @marek-safar @EgorBo any idea?

I still can't reproduce it outside of iOS, so it seems like changing the BCL code triggers some issue on the runtime side that is target specific.

@marek-safar is the BCL profile for monotouch built differently than for example linux/armv7 that could trigger such behavior? I have stuff like -d:BIT64 in mind.

@lewurm

This comment has been minimized.

Copy link
Member

@lewurm lewurm commented Jan 30, 2019

It boils down to this being true:

            double f = 4.94065645841247E-324;
            if (f == 0.0)
                Console.WriteLine ("true on darwin/armv7");
            else
                Console.WriteLine ("false on linux/armv7");

It's hitting this path in the new BCL code:
https://github.com/mono/corert/blob/c4ae51646152de0af32b2c85538b6e7317faa7f8/src/System.Private.CoreLib/src/System/Number.Unix.cs#L36

@lewurm

This comment has been minimized.

Copy link
Member

@lewurm lewurm commented Jan 31, 2019

I could reproduce this issue outside of mono, with something like that:

int IsNullDouble (double v)
{
    return v == 0.0;
}
[...]
   IsNullDouble ( 4.94065645841247E-324);
[...]

clang vs. gcc generates slightly different code, but it's semantically the same. On an iPhone 5c:

(lldb) p $d16
(double) $4 = 4.9406564584124654E-324
(lldb) x/20i $pc
->  0xc882c: 0xeef50b40   vcmp.f64 d16, #0
    0xc8830: 0xeef1fa10   vmrs   APSR_nzcv, fpscr
    0xc8834: 0xe3000000   movw   r0, #0x0
    0xc8838: 0x03a00001   moveq  r0, #1
[...]
(lldb) si
(lldb) si
(lldb) si
(lldb) si
(lldb) p $r0
(unsigned int) $7 = 1 # <-- !!! so d16 from above EQUALS 0.0

On an armhf linux env (tegra jetson board, so not a true armv7 device):

(gdb) p $d7.f64
$2 = 4.9406564584124654e-324
(gdb) x/5i $pc
=> 0xaaaaa5e4 <IsNullDouble+20>:        vcmp.f64        d7, #0.0
   0xaaaaa5e8 <IsNullDouble+24>:        vmrs    APSR_nzcv, fpscr
   0xaaaaa5ec <IsNullDouble+28>:        moveq   r3, #1
   0xaaaaa5f0 <IsNullDouble+32>:        movne   r3, #0
   0xaaaaa5f4 <IsNullDouble+36>:        uxtb    r3, r3
(gdb) si
0xaaaaa5e8      5               return f == 0.0;
(gdb)
0xaaaaa5ec      5               return f == 0.0;
(gdb)
0xaaaaa5f0      5               return f == 0.0;
(gdb) si
0xaaaaa5f4      5               return f == 0.0;
(gdb) p $r3
$3 = 0 # <-- d7 does NOT equal to 0.0 (which is correct)

From what I can tell that was the behavior since forever, but the old BCL code never did a double compare with 0.0.

@lewurm

This comment has been minimized.

Copy link
Member

@lewurm lewurm commented Jan 31, 2019

Hah, that is a known issue actually: https://bugzilla.xamarin.com/show_bug.cgi?id=15802#c11

Note that Double.Epsilon is defined as 4.9406564584124654E-324 (which maps to 0x1 in hex representation).

Now I'm not sure how to fix it. The old vs. new BCL code is largely different, so I couldn't spot the difference easily. Is it possible to just ignore those tests on iOS 32bit?

@EgorBo

This comment has been minimized.

Copy link
Member

@EgorBo EgorBo commented Jan 31, 2019

@lewurm I guess that == 0.0 is just a fast path for the most popular value. Will BitConverter.DoubleToInt64Bits(value) == 0 help here?

@lewurm

This comment has been minimized.

Copy link
Member

@lewurm lewurm commented Jan 31, 2019

@EgorBo nope, I tried that. It uses then the icall that goes down to using snprintf which seems to suffer from the same issue 😕

@marek-safar

This comment has been minimized.

Copy link
Member

@marek-safar marek-safar commented Jan 31, 2019

@lewing My suggestion is to extend the 0.0 check with && BitConverter.DoubleToInt64Bits(value) == 0 in BCL to detect this condition then in the native DoubleToString call have ifdef for arm where we hardcode the epsilon string value to the buffer and before calling snprintf

@lewurm

This comment has been minimized.

Copy link
Member

@lewurm lewurm commented Jan 31, 2019

@lewing My suggestion is to extend the 0.0 check with && BitConverter.DoubleToInt64Bits(value) == 0 in BCL to detect this condition then in the native DoubleToString call have ifdef for arm where we hardcode the epsilon string value to the buffer and before calling snprintf

that would mean the icall also needs to hardcode all kind of different format string variants 😕

@EgorBo

This comment has been minimized.

Copy link
Member

@EgorBo EgorBo commented Jan 31, 2019

btw, I had a PR where I replace that icall (snprintf) to C# from the upstream #10763 (just saying 🙂)

@marek-safar

This comment has been minimized.

Copy link
Member

@marek-safar marek-safar commented Jan 31, 2019

@lewurm I don't think so, the call returns just buffer with numbers which is later formatted, @EgorBo can you confirm?

@lewurm

This comment has been minimized.

lewurm added a commit to lewurm/corert that referenced this issue Jan 31, 2019
marek-safar added a commit to mono/corert that referenced this issue Jan 31, 2019
lewurm added a commit to mono/corert that referenced this issue Feb 1, 2019
monojenkins added a commit to monojenkins/mono that referenced this issue Feb 1, 2019
monojenkins added a commit that referenced this issue Feb 1, 2019
[2018-10] [arm/ios] workaround for faulty vcmp.f64 insn

Fixes #11965



Backport of #12704.

/cc @marek-safar @lewurm
marek-safar added a commit that referenced this issue Feb 1, 2019
akoeplinger added a commit that referenced this issue Feb 4, 2019
Fixes #11965

Backport of #12704.
jonpryor added a commit to xamarin/xamarin-android that referenced this issue Feb 6, 2019
Fixes: mono/mono#11615
Fixes: mono/mono#11965
Fixes: mono/mono#12538

Bumps to mono/corert@e229c94.

Don't probe AOT cache directories on Android; they don't exist

Native Crash Stability Fix Batch

Fix deadlock when unwinding with coop enabled.

Fix MERP fix wherein some stack frames were missing.
jonpryor added a commit to jonpryor/xamarin-android that referenced this issue Feb 13, 2019
Fixes: mono/mono#11965
Fixes: mono/mono#12831
Fixes: xamarin/xamarin-macios#5363
Fixes: xamarin/xamarin-macios#5381

Bumps to mono/corert@e229c94

Add back missing stack frames in MERP reports.

Fix building older runtimes with newer system Mono.
(Perhaps this will fix our Linux builds?)
jonpryor added a commit to xamarin/xamarin-android that referenced this issue Feb 13, 2019
Fixes: mono/mono#11965
Fixes: mono/mono#12831
Fixes: xamarin/xamarin-macios#5363
Fixes: xamarin/xamarin-macios#5381

Bumps to mono/corert@e229c94

Add back missing stack frames in MERP reports.

Fix building older runtimes with newer system Mono.
(Perhaps this will fix our Linux builds?)
jonpryor added a commit to xamarin/xamarin-android that referenced this issue Apr 24, 2019
Bumps to mono/api-snapshot@ae01378
Bumps to mono/reference-assemblies@e5173a5
Bumps to mono/bockbuild@d30329d
Bumps to mono/boringssl@3d87996
Bumps to mono/corefx@72f7d76
Bumps to mono/corert@1b7d4a1
Bumps to mono/helix-binaries@7e893ea
Bumps to mono/illinker-test-assets@f21ff68
Bumps to mono/linker@13d864e
Bumps to mono/llvm@1aaaaa5 [mono]
Bumps to mono/llvm@2c2cffe [xamarin-android]
Bumps to mono/NUnitLite@0029561
Bumps to mono/roslyn-binaries@0bbc9b4
Bumps to mono/xunit-binaries@8f6e62e

	$ git diff --shortstat 886c4901..e66c7667      # mono
        3597 files changed, 350850 insertions(+), 91128 deletions(-)
	$ git diff --shortstat 349752c464c5fc93b32e7d45825f2890c85c8b7d..2c2cffedf01e0fe266b9aaad2c2563e05b750ff4
	 240 files changed, 18562 insertions(+), 6581 deletions(-)

Context: dotnet/coreclr#22046

Fixes: CVE 2018-8292 on macOS
Fixes: http://work.devdiv.io/737323
Fixes: dotnet/corefx#33965
Fixes: dotnet/standard#642
Fixes: mono/mono#6997
Fixes: mono/mono#7326
Fixes: mono/mono#7517
Fixes: mono/mono#7750
Fixes: mono/mono#7859
Fixes: mono/mono#8360
Fixes: mono/mono#8460
Fixes: mono/mono#8766
Fixes: mono/mono#8922
Fixes: mono/mono#9418
Fixes: mono/mono#9507
Fixes: mono/mono#9951
Fixes: mono/mono#10024
Fixes: mono/mono#10030
Fixes: mono/mono#10038
Fixes: mono/mono#10448
Fixes: mono/mono#10735
Fixes: mono/mono#10735
Fixes: mono/mono#10737
Fixes: mono/mono#10743
Fixes: mono/mono#10834
Fixes: mono/mono#10837
Fixes: mono/mono#10838
Fixes: mono/mono#10863
Fixes: mono/mono#10945
Fixes: mono/mono#11020
Fixes: mono/mono#11021
Fixes: mono/mono#11021
Fixes: mono/mono#11049
Fixes: mono/mono#11091
Fixes: mono/mono#11095
Fixes: mono/mono#11123
Fixes: mono/mono#11138
Fixes: mono/mono#11146
Fixes: mono/mono#11202
Fixes: mono/mono#11214
Fixes: mono/mono#11317
Fixes: mono/mono#11326
Fixes: mono/mono#11378
Fixes: mono/mono#11385
Fixes: mono/mono#11478
Fixes: mono/mono#11479
Fixes: mono/mono#11488
Fixes: mono/mono#11489
Fixes: mono/mono#11527
Fixes: mono/mono#11529
Fixes: mono/mono#11596
Fixes: mono/mono#11603
Fixes: mono/mono#11613
Fixes: mono/mono#11623
Fixes: mono/mono#11663
Fixes: mono/mono#11681
Fixes: mono/mono#11684
Fixes: mono/mono#11693
Fixes: mono/mono#11697
Fixes: mono/mono#11779
Fixes: mono/mono#11809
Fixes: mono/mono#11858
Fixes: mono/mono#11895
Fixes: mono/mono#11898
Fixes: mono/mono#11898
Fixes: mono/mono#11965
Fixes: mono/mono#12182
Fixes: mono/mono#12193
Fixes: mono/mono#12218
Fixes: mono/mono#12235
Fixes: mono/mono#12263
Fixes: mono/mono#12307
Fixes: mono/mono#12331
Fixes: mono/mono#12362
Fixes: mono/mono#12374
Fixes: mono/mono#12402
Fixes: mono/mono#12421
Fixes: mono/mono#12461
Fixes: mono/mono#12479
Fixes: mono/mono#12479
Fixes: mono/mono#12552
Fixes: mono/mono#12603
Fixes: mono/mono#12747
Fixes: mono/mono#12831
Fixes: mono/mono#12843
Fixes: mono/mono#12881
Fixes: mono/mono#13030
Fixes: mono/mono#13284
Fixes: mono/mono#13297
Fixes: mono/mono#13455
Fixes: mono/mono#13460
Fixes: mono/mono#13478
Fixes: mono/mono#13479
Fixes: mono/mono#13522
Fixes: mono/mono#13607
Fixes: mono/mono#13610
Fixes: mono/mono#13610
Fixes: mono/mono#13639
Fixes: mono/mono#13672
Fixes: mono/mono#13834
Fixes: mono/mono#13878
Fixes: mono/mono#6352
Fixes: mono/monodevelop#6898
Fixes: xamarin/maccore#1069
Fixes: xamarin/maccore#1407
Fixes: xamarin/maccore#604
Fixes: xamarin/xamarin-macios#4984
Fixes: xamarin/xamarin-macios#5289
Fixes: xamarin/xamarin-macios#5363
Fixes: xamarin/xamarin-macios#5381
Fixes: https://issuetracker.unity3d.com/issues/editor-crashes-with-g-logv-when-entering-play-mode-with-active-flowcanvas-script
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.