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

Assertion failures in mruby-pack on big endian #4190

Closed
jeremyevans opened this Issue Dec 17, 2018 · 4 comments

Comments

Projects
None yet
2 participants
@jeremyevans
Copy link

commented Dec 17, 2018

The good news is mruby 2.0.0 works on OpenBSD/sparc64 (previous versions failed to build). However, running the tests shows the following failures:

Fail: pack float (mrbgems: mruby-pack)
 - Assertion[1] Failed: Expected to be equal
    Expected: "\x00\x00@@"
      Actual: "@@\x00\x00"
 - Assertion[2] Failed: Expected to be equal
    Expected: [3]
      Actual: [2.304855714121459e-41]
 - Assertion[3] Failed: Expected to be equal
    Expected: "@@\x00\x00"
      Actual: "\x00\x00@@"
 - Assertion[4] Failed: Expected to be equal
    Expected: [3]
      Actual: [2.304855714121459e-41]
 - Assertion[5] Failed: Expected to be equal
    Expected: "@@\x00\x00"
      Actual: "\x00\x00@@"
 - Assertion[6] Failed: Expected to be equal
    Expected: [3]
      Actual: [2.304855714121459e-41]
 - Assertion[7] Failed: Expected to be equal
    Expected: "@@\x00\x00"
      Actual: "\x00\x00@@"
 - Assertion[8] Failed: Expected to be equal
    Expected: [3]
      Actual: [2.304855714121459e-41]
Fail: pack double (mrbgems: mruby-pack)
 - Assertion[1] Failed: Expected to be equal
    Expected: "\x00\x00\x00\x00\x00\x00\b@"
      Actual: "@\b\x00\x00\x00\x00\x00\x00"
 - Assertion[2] Failed: Expected to be equal
    Expected: [3]
      Actual: [1.043466644016713e-320]
 - Assertion[3] Failed: Expected to be equal
    Expected: "@\b\x00\x00\x00\x00\x00\x00"
      Actual: "\x00\x00\x00\x00\x00\x00\b@"
 - Assertion[4] Failed: Expected to be equal
    Expected: [3]
      Actual: [1.043466644016713e-320]
 - Assertion[5] Failed: Expected to be equal
    Expected: "@\b\x00\x00\x00\x00\x00\x00"
      Actual: "\x00\x00\x00\x00\x00\x00\b@"
 - Assertion[6] Failed: Expected to be equal
    Expected: [3]
      Actual: [1.043466644016713e-320]
 - Assertion[7] Failed: Expected to be equal
    Expected: "@\b\x00\x00\x00\x00\x00\x00"
      Actual: "\x00\x00\x00\x00\x00\x00\b@"
 - Assertion[8] Failed: Expected to be equal
    Expected: [3]
      Actual: [1.043466644016713e-320]

These failures do not occur on OpenBSD/amd64, and the test output leads me to believe this may be a general mruby-pack issue on big endian systems.

@matz

This comment has been minimized.

Copy link
Member

commented Dec 20, 2018

Do you have MRB_ENDIAN_BIG defined?

@jeremyevans

This comment has been minimized.

Copy link
Author

commented Dec 20, 2018

I'm guessing it isn't explicitly defined. I'm going to close this now, and if there are still assertion failures with it defined, I'll reopen.

@jeremyevans

This comment has been minimized.

Copy link
Author

commented Dec 20, 2018

I just wanted to confirm that defining MRB_ENDIAN_BIG did fix the problem. We ended up doing something similar to this so the same code works on both big endian and little endian:

 /* define on big endian machines; used by MRB_NAN_BOXING */
-//#define MRB_ENDIAN_BIG
+#include <endian.h>
+#if (BYTE_ORDER == BIG_ENDIAN)
+#define MRB_ENDIAN_BIG
+#endif
@matz

This comment has been minimized.

Copy link
Member

commented Dec 20, 2018

That would be a nice idea. I was working on a different solution that relies on the dynamic endian check.
But this one is better.

matz added a commit that referenced this issue Dec 20, 2018

`mruby-pack` should not rely on `MRB_ENDIAN_BIG` macro; fix #4190
The `MRB_ENDIAN_BIG` macro is originally used for `NaN` boxing.
We cannot assume it is defined on every big endian platform (#4190
is the case). So instead of relying on untrusted `MRB_ENDIAN_BIG`, we
use `BYTE_ORDER` macro with a fallback function to check endian in
runtime.

matz added a commit that referenced this issue Dec 20, 2018

Define `MRB_ENDIAN_BIG` automatically; ref #4190
You had to define this macro on big endian platforms, but it is very
error-prone. So define the macro automatically if possible.

matz added a commit that referenced this issue Dec 21, 2018

Simplify `MRB_ENDIAN_BIG` macro definition; ref #4190
`cpp` does not raise error on undefined macro access in condition.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.