8000 Fix float/double pack/unpack on s390x by khasinski · Pull Request #6700 · mruby/mruby · GitHub
[go: up one dir, main page]
Skip to content

Fix float/double pack/unpack on s390x #6700

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.

Sign up for GitHub

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

Jump to bottom
Merged
matz merged 1 commit into from
Jan 13, 2026
Merged

Fix float/double pack/unpack on s390x #6700

matz merged 1 commit into from
Jan 13, 2026
+35 −33

Conversation

@khasinski
Copy link
Contributor
@khasinski khasinski commented Jan 12, 2026

The pack_float, pack_double, unpack_float, and unpack_double functions accessed float/double bytes via a union with uint8_t array, assuming bytes[0] is always the LSB. This is only true on little-endian hosts.

Fix by using the same bit-shift approach as the integer pack functions (like pack_quad, unpack_quad). Reinterpret float/double as uint32/uint64 and use shifts to extract/assemble bytes in an endian-independent way.

Tested with QEMU, I do not have a mainframe at home 😅

Fixes: #6698 (s390x test failures)

@khasinski
Fix float/double pack/unpack on big-endian architectures
946e8c2 
The pack_float, pack_double, unpack_float, and unpack_double functions
accessed float/double bytes via a union with uint8_t array, assuming
bytes[0] is always the LSB. This is only true on little-endian hosts.

Fix by using the same bit-shift approach as the integer pack functions
(pack_quad, unpack_quad, etc). Reinterpret float/double as uint32/uint64
and use shifts to extract/assemble bytes in an endian-independent way.

Fixes: mruby#6698 (s390x test failures)
@khasinski khasinski requested a review from matz as a code owner January 12, 2026 02:09
@gemini-code-assist
Copy link
gemini-code-assist bot commented Jan 12, 2026

Summary of Changes

Hello @khasinski, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses and resolves critical endianness issues in the pack_float, pack_double, unpack_float, and unpack_double functions within mrbgems/mruby-pack/src/pack.c. Previously, these functions incorrectly assumed a little-endian byte order when accessing float/double values via a uint8_t array, leading to failures on big-endian systems like s390x. The solution refactors these functions to use bit-shift operations on uint32_t and uint64_t reinterpretations of the float/double values, ensuring correct and endian-independent byte extraction and assembly, consistent with how integer types are handled.

Highlights

  • Endianness Fix for Float/Double Packing: The pack_float, pack_double, unpack_float, and unpack_double functions previously relied on uint8_t array access within a union, which implicitly assumed a little-endian byte order (LSB at bytes[0]). This assumption caused incorrect behavior and test failures on big-endian architectures, specifically s390x.
  • Endian-Independent Byte Handling: The fix refactors these functions to reinterpret float and double values as uint32_t and uint64_t respectively. It then uses explicit bit-shift operations to extract and assemble bytes, ensuring correct and endian-independent byte order regardless of the host system's endianness. This approach aligns with the existing robust implementation for integer packing functions.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detai 8000 led instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist[bot]
gemini-code-assist bot reviewed Jan 12, 2026
View reviewed changes
Copy link
@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request correctly fixes an endianness bug in the float and double packing/unpacking functions. The previous implementation relied on union-based type punning that was dependent on host byte order, causing failures on big-endian systems like s390x. The new implementation uses bit-shifting to construct and deconstruct floating-point numbers, making the process endian-independent. This approach is consistent with how integer types are handled elsewhere in this file. The changes are clear, correct, and effectively resolve the issue.

@mkoncek
Copy link
mkoncek commented Jan 12, 2026

Tested this patch in Fedora Copr and the tests passed: https://copr.fedorainfracloud.org/coprs/mkoncek/mruby/build/10000135/

@matz matz merged commit 97835e5 into mruby:master Jan 13, 2026
18 checks passed
@khasinski khasinski deleted the fix-pack-float-endianness branch January 13, 2026 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@matz matz Awaiting requested review from matz matz is a code owner

1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

mrbgems

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bigint tests fail on architectures other than x86_64 and i386

3 participants

@khasinski @mkoncek @matz
0