VM crash when adding a double and an object with a toDouble method, in production mode

[DartBot]

This issue was originally filed by [email protected]

---

Reduced test-case (run in VM with type-checking disabled):
class C {
    toDouble() => 0;
}
main() {
    print(2.2 + new C());
}

This is what happens:
~/projects/dart/dart % dart --enable_asserts toDoubleBug.dart
./vm/object.h:2201: error: Handle check failed.
zsh: abort dart --enable_asserts toDoubleBug.dart

What should happen:
Not sure (not crash obviously). Checked mode complains (Failed type check: type C is not assignable to type num of other in bootstrap_impl at line 898, column 25.) but it's not clear to me how toDouble is supposed to be used or not currently.

Stack trace:

­0 0x91fb6332 in __kill ()

­1 0x91fb5932 in kill$UNIX2003 ()

­2 0x9b85ac0c in abort ()

­3 0x000d1df1 in dart::DynamicAssertionHelper::Fail (this=0xbfffeaa0, format=0x184348 "%s") at /Users/olov/projects/dart/dart/runtime/vm/assert.cc:39

­4 0x000be5f5 in dart::Double::CheckedHandle (raw_ptr=0x0) at object.h:2201

­5 0x000bcd2e in dart::DN_HelperDouble_add (arguments=0xbffff1a4) at /Users/olov/projects/dart/dart/runtime/lib/double.cc:26

­6 0x000bcf0b in dart::DN_Double_add (args=0xbffff1a4) at /Users/olov/projects/dart/dart/runtime/lib/double.cc:24

­7 0x00440162 in ?? ()

­8 0x006005fc in ?? ()

­9 0x0060042d in ?? ()

­10 0x006002c2 in ?? ()

­11 0x0060007d in ?? ()

­12 0x000f9dfd in dart::DartEntry::InvokeStatic (function=@0xbe4ae8, arguments=@0xbffff368, optional_arguments_names=@0xbe4af0) at /Users/olov/projects/dart/dart/runtime/vm/dart_entry.cc:83

­13 0x000ae507 in dart::InvokeStatic (function=@0xbe4ae8, args=@0xbffff368, result=0xbffff378) at /Users/olov/projects/dart/dart/runtime/vm/dart_api_impl.cc:844

­14 0x000b012f in Dart_InvokeStatic (library_in=0xbe0410, class_name_in=0xbe0418, function_name_in=0xbe0414, number_of_arguments=0, arguments=0x0) at /Users/olov/projects/dart/dart/runtime/vm/dart_api_impl.cc:958

­15 0x00002bfb in main (argc=2, argv=0xbffffac8) at /Users/olov/projects/dart/dart/runtime/bin/main.cc:224

[iposva-google]

Added Area-VM, Triaged labels.

[DartBot]

This comment was originally written by [email protected]

---

I should also add that if toDouble is changed such that it actually returns a double instead of and int:
    toDouble() => 0.0;

then in production mode the program performs the addition and prints 2.200000, while in checked mode it throws the same error as before (Failed type check: type C is not assignable to type num of other in bootstrap_impl at line 898, column 25).

[floitschG]

The plus-operator on numbers is defined as "operator +(num other)". Since you don't give it a class that implements "num", the behavior is undefined. It just happens that apparently "+" calls "toDouble" on the given argument, and then expects a double. Again: if you violate the assumptions (by not returning a double) the behavior is undefined. Here the vm aborts with an assertion-error.

Note: the static checker would have told you that the following line is invalid:
   print(2.2 + new C()); // "Error: C is not assignable to num".

I leave this bug open in case the VM guys want to improve the error message.

[iposva-google]

Set owner to @iposva-google.
Added Accepted label.

[DartBot]

This comment was originally written by [email protected]

---

I expected an exception to be thrown at worst, not an assertion to be fired. The error message itself isn't a problem as such.

I'm surprised by the description of undefined behavior allowing VM abortion. Can you provide more information about this, specifically what constitutes undefined behavior in that case? The C instance not implementing the num interface (expected by operator +) seems like a plain type error to me.

[iposva-google]

Fix pending: http://codereview.chromium.org/8273001/

[floitschG]

You are right. I have to correct myself: assertion-error is not an acceptable behavior; nor is abortion of the VM.
Also we will have to define the behavior of the library. For now it is undefined, but eventually we must give precise semantics.

[ghost]

Fixed in r1917

---

Set owner to @sgmitrovic.
Added Fixed label.

[DartBot]

This comment was originally written by [email protected]

---

I've verified that it works (r1817 though), thanks.