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

Fixed overflow bug affecting 32-bit Linux platforms #288

Merged
merged 6 commits into from
Oct 14, 2023

Conversation

erlingrj
Copy link
Collaborator

@erlingrj erlingrj commented Oct 7, 2023

This bug was reported in this issue: lf-lang/lingua-franca#2034

Another reminder of how difficult it is to juggle time representations on different architectures. In general, unless we are dealing with a type of explicit widht, like uint64_t or instant_t, then we must consider that it might be 32-bit on a 32-bit architecture. This goes for any literal value also.

@erlingrj
Copy link
Collaborator Author

erlingrj commented Oct 7, 2023

The test: DecentralizedP2PUnbalancedTimeoutPhysical fails non-deterministically. Also before this PR. So, I think we should consider this a different bug and address it separately

@erlingrj erlingrj requested a review from lhstrh October 11, 2023 08:07
@erlingrj
Copy link
Collaborator Author

Should we get this little fix merged, @edwardalee? I have tested in on a RPi4 (with a 32bit OS).

Copy link
Contributor

@edwardalee edwardalee left a comment

Choose a reason for hiding this comment

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

LGTM!

@erlingrj erlingrj merged commit 654fa0c into main Oct 14, 2023
25 checks passed
@lhstrh lhstrh changed the title Fix overflow bug affecting 32-bit Linux platforms Fixed overflow bug affecting 32-bit Linux platforms Jan 23, 2024
@lhstrh lhstrh added the bugfix label Jan 23, 2024
@lhstrh lhstrh deleted the 32-bit-time-overflow branch February 1, 2024 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants