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

avoid overflow on functionality to print backtrace of safepoint straggler #57579

Merged
merged 1 commit into from
Mar 1, 2025

Conversation

d-netto
Copy link
Member

@d-netto d-netto commented Feb 28, 2025

In the line of C code:

const int64_t timeout = jl_options.timeout_for_safepoint_straggler_s * 1000000000;

jl_options.timeout_for_safepoint_straggler_s is an int16_t and 1000000000 is an int32_t.

The result of jl_options.timeout_for_safepoint_straggler_s * 1000000000 will be an int32_t which may not be large enough to hold the value of jl_options.timeout_for_safepoint_straggler_s after converting to nanoseconds, leading to overflow.

@d-netto d-netto added embarrassing-bugfix Whoops! needs tests Unit tests are required for this change labels Feb 28, 2025
Copy link
Member

@NHDaly NHDaly left a comment

Choose a reason for hiding this comment

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

LGTM

@d-netto d-netto force-pushed the dcn-avoid-overflow-on-safepoint-straggler branch from c75da4a to 89bc82b Compare February 28, 2025 23:14
@d-netto d-netto removed the needs tests Unit tests are required for this change label Feb 28, 2025
@d-netto d-netto force-pushed the dcn-avoid-overflow-on-safepoint-straggler branch 2 times, most recently from 9a57c9f to 273556c Compare March 1, 2025 11:41
@d-netto d-netto force-pushed the dcn-avoid-overflow-on-safepoint-straggler branch from 273556c to 1995bac Compare March 1, 2025 11:41
@d-netto d-netto added the merge me PR is reviewed. Merge when all tests are passing label Mar 1, 2025
@d-netto d-netto merged commit 9cafd35 into master Mar 1, 2025
8 checks passed
@d-netto d-netto deleted the dcn-avoid-overflow-on-safepoint-straggler branch March 1, 2025 14:30
@d-netto d-netto removed the merge me PR is reviewed. Merge when all tests are passing label Mar 1, 2025
@nsajko
Copy link
Contributor

nsajko commented Mar 1, 2025

I guess this has to be backported to v1.12?

@nsajko nsajko added the backport 1.12 Change should be backported to release-1.12 label Mar 1, 2025
@d-netto
Copy link
Member Author

d-netto commented Mar 1, 2025

Yes, it does.

Thanks for noticing that.

d-netto added a commit to RelationalAI/julia that referenced this pull request Mar 2, 2025
…gler (JuliaLang#57579)

In the line of C code:

```C
const int64_t timeout = jl_options.timeout_for_safepoint_straggler_s * 1000000000;
```

`jl_options.timeout_for_safepoint_straggler_s` is an `int16_t` and
`1000000000` is an `int32_t`.

The result of `jl_options.timeout_for_safepoint_straggler_s *
1000000000` will be an `int32_t` which may not be large enough to hold
the value of `jl_options.timeout_for_safepoint_straggler_s` after
converting to nanoseconds, leading to overflow.
d-netto added a commit to RelationalAI/julia that referenced this pull request Mar 2, 2025
…gler (JuliaLang#57579)

In the line of C code:

```C
const int64_t timeout = jl_options.timeout_for_safepoint_straggler_s * 1000000000;
```

`jl_options.timeout_for_safepoint_straggler_s` is an `int16_t` and
`1000000000` is an `int32_t`.

The result of `jl_options.timeout_for_safepoint_straggler_s *
1000000000` will be an `int32_t` which may not be large enough to hold
the value of `jl_options.timeout_for_safepoint_straggler_s` after
converting to nanoseconds, leading to overflow.
KristofferC pushed a commit that referenced this pull request Mar 3, 2025
…gler (#57579)

In the line of C code:

```C
const int64_t timeout = jl_options.timeout_for_safepoint_straggler_s * 1000000000;
```

`jl_options.timeout_for_safepoint_straggler_s` is an `int16_t` and
`1000000000` is an `int32_t`.

The result of `jl_options.timeout_for_safepoint_straggler_s *
1000000000` will be an `int32_t` which may not be large enough to hold
the value of `jl_options.timeout_for_safepoint_straggler_s` after
converting to nanoseconds, leading to overflow.

(cherry picked from commit 9cafd35)
@KristofferC KristofferC mentioned this pull request Mar 3, 2025
39 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.12 Change should be backported to release-1.12 embarrassing-bugfix Whoops!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants