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 infinite loop when doing SIGTRAP in arm64-apple #51284

Merged
merged 2 commits into from
Sep 14, 2023
Merged

Conversation

gbaraldi
Copy link
Member

@gbaraldi gbaraldi commented Sep 12, 2023

The guard instruction for unreachables and other crashes in aarch64 is brk, in macos there isn't a distinction between a brk for a breakpoint and one for a crash, as an attempt we check the value of pc when the signal is triggered, if it is
brk #0x1 we say that it is a crash and go into the sigdie_handler.

We should probably do the same in aarch64 linux, though I haven't dug too deep into what values it uses for traps, and if what compiler used matters, on apple I assumed we use clang/LLVM

It might be possible to test this by calling some inline assembly.

This means that something like #51267 actually crashes with

[16908] signal (5): Trace/BPT trap: 5
in expression starting at /Users/gabrielbaraldi/julia/test.jl:2
_collect at ./array.jl:768
collect at ./array.jl:757
top-level scope at /Users/gabrielbaraldi/julia/test.jl:5
_jl_invoke at /Users/gabrielbaraldi/julia/src/gf.c:2892
jl_toplevel_eval_flex at /Users/gabrielbaraldi/julia/src/toplevel.c:925
jl_toplevel_eval_flex at /Users/gabrielbaraldi/julia/src/toplevel.c:877
ijl_toplevel_eval at /Users/gabrielbaraldi/julia/src/toplevel.c:943 [inlined]
ijl_toplevel_eval_in at /Users/gabrielbaraldi/julia/src/toplevel.c:985
eval at ./boot.jl:383 [inlined]
include_string at ./loading.jl:2070
_jl_invoke at /Users/gabrielbaraldi/julia/src/gf.c:2873
ijl_apply_generic at /Users/gabrielbaraldi/julia/src/gf.c:3074
_include at ./loading.jl:2130
include at ./Base.jl:494
jfptr_include_46486 at /Users/gabrielbaraldi/julia/usr/lib/julia/sys.dylib (unknown line)
_jl_invoke at /Users/gabrielbaraldi/julia/src/gf.c:2873
ijl_apply_generic at /Users/gabrielbaraldi/julia/src/gf.c:3074
exec_options at ./client.jl:317
_start at ./client.jl:552
jfptr__start_83179 at /Users/gabrielbaraldi/julia/usr/lib/julia/sys.dylib (unknown line)
_jl_invoke at /Users/gabrielbaraldi/julia/src/gf.c:2873
ijl_apply_generic at /Users/gabrielbaraldi/julia/src/gf.c:3074
jl_apply at /Users/gabrielbaraldi/julia/src/./julia.h:1970 [inlined]
true_main at /Users/gabrielbaraldi/julia/src/jlapi.c:582
jl_repl_entrypoint at /Users/gabrielbaraldi/julia/src/jlapi.c:731
Allocations: 570978 (Pool: 570031; Big: 947); GC: 1
fish: Job 1, './julia test.jl' terminated by signal SIGTRAP (Trace or breakpoint trap)

instead of hanging silently

@gbaraldi gbaraldi added the system:apple silicon Affects Apple Silicon only (Darwin/ARM64) - e.g. M1 and other M-series chips label Sep 12, 2023
Copy link
Sponsor Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

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

LGTM

Co-authored-by: Jameson Nash <vtjnash@gmail.com>
@vchuravy vchuravy merged commit d51ad06 into master Sep 14, 2023
6 checks passed
@vchuravy vchuravy deleted the gb/arm64-trap branch September 14, 2023 20:13
NHDaly pushed a commit that referenced this pull request Sep 20, 2023
The guard instruction for unreachables and other crashes in aarch64 is
`brk`, in macos there isn't a distinction between a brk for a breakpoint
and one for a crash, as an attempt we check the value of `pc` when the
signal is triggered, if it is
`brk #0x1` we say that it is a crash and go into the sigdie_handler. 

We should probably do the same in aarch64 linux, though I haven't dug
too deep into what values it uses for traps, and if what compiler used
matters, on apple I assumed we use clang/LLVM

It might be possible to test this by calling some inline assembly.

This means that something like
#51267 actually crashes with
```c
[16908] signal (5): Trace/BPT trap: 5
in expression starting at /Users/gabrielbaraldi/julia/test.jl:2
_collect at ./array.jl:768
collect at ./array.jl:757
top-level scope at /Users/gabrielbaraldi/julia/test.jl:5
_jl_invoke at /Users/gabrielbaraldi/julia/src/gf.c:2892
jl_toplevel_eval_flex at /Users/gabrielbaraldi/julia/src/toplevel.c:925
jl_toplevel_eval_flex at /Users/gabrielbaraldi/julia/src/toplevel.c:877
ijl_toplevel_eval at /Users/gabrielbaraldi/julia/src/toplevel.c:943 [inlined]
ijl_toplevel_eval_in at /Users/gabrielbaraldi/julia/src/toplevel.c:985
eval at ./boot.jl:383 [inlined]
include_string at ./loading.jl:2070
_jl_invoke at /Users/gabrielbaraldi/julia/src/gf.c:2873
ijl_apply_generic at /Users/gabrielbaraldi/julia/src/gf.c:3074
_include at ./loading.jl:2130
include at ./Base.jl:494
jfptr_include_46486 at /Users/gabrielbaraldi/julia/usr/lib/julia/sys.dylib (unknown line)
_jl_invoke at /Users/gabrielbaraldi/julia/src/gf.c:2873
ijl_apply_generic at /Users/gabrielbaraldi/julia/src/gf.c:3074
exec_options at ./client.jl:317
_start at ./client.jl:552
jfptr__start_83179 at /Users/gabrielbaraldi/julia/usr/lib/julia/sys.dylib (unknown line)
_jl_invoke at /Users/gabrielbaraldi/julia/src/gf.c:2873
ijl_apply_generic at /Users/gabrielbaraldi/julia/src/gf.c:3074
jl_apply at /Users/gabrielbaraldi/julia/src/./julia.h:1970 [inlined]
true_main at /Users/gabrielbaraldi/julia/src/jlapi.c:582
jl_repl_entrypoint at /Users/gabrielbaraldi/julia/src/jlapi.c:731
Allocations: 570978 (Pool: 570031; Big: 947); GC: 1
fish: Job 1, './julia test.jl' terminated by signal SIGTRAP (Trace or breakpoint trap)
```
instead of hanging silently

---------

Co-authored-by: Jameson Nash <vtjnash@gmail.com>
@gbaraldi gbaraldi added backport 1.9 Change should be backported to release-1.9 backport 1.10 Change should be backported to the 1.10 release labels Sep 23, 2023
KristofferC pushed a commit that referenced this pull request Oct 3, 2023
The guard instruction for unreachables and other crashes in aarch64 is
`brk`, in macos there isn't a distinction between a brk for a breakpoint
and one for a crash, as an attempt we check the value of `pc` when the
signal is triggered, if it is
`brk #0x1` we say that it is a crash and go into the sigdie_handler.

We should probably do the same in aarch64 linux, though I haven't dug
too deep into what values it uses for traps, and if what compiler used
matters, on apple I assumed we use clang/LLVM

It might be possible to test this by calling some inline assembly.

This means that something like
#51267 actually crashes with
```c
[16908] signal (5): Trace/BPT trap: 5
in expression starting at /Users/gabrielbaraldi/julia/test.jl:2
_collect at ./array.jl:768
collect at ./array.jl:757
top-level scope at /Users/gabrielbaraldi/julia/test.jl:5
_jl_invoke at /Users/gabrielbaraldi/julia/src/gf.c:2892
jl_toplevel_eval_flex at /Users/gabrielbaraldi/julia/src/toplevel.c:925
jl_toplevel_eval_flex at /Users/gabrielbaraldi/julia/src/toplevel.c:877
ijl_toplevel_eval at /Users/gabrielbaraldi/julia/src/toplevel.c:943 [inlined]
ijl_toplevel_eval_in at /Users/gabrielbaraldi/julia/src/toplevel.c:985
eval at ./boot.jl:383 [inlined]
include_string at ./loading.jl:2070
_jl_invoke at /Users/gabrielbaraldi/julia/src/gf.c:2873
ijl_apply_generic at /Users/gabrielbaraldi/julia/src/gf.c:3074
_include at ./loading.jl:2130
include at ./Base.jl:494
jfptr_include_46486 at /Users/gabrielbaraldi/julia/usr/lib/julia/sys.dylib (unknown line)
_jl_invoke at /Users/gabrielbaraldi/julia/src/gf.c:2873
ijl_apply_generic at /Users/gabrielbaraldi/julia/src/gf.c:3074
exec_options at ./client.jl:317
_start at ./client.jl:552
jfptr__start_83179 at /Users/gabrielbaraldi/julia/usr/lib/julia/sys.dylib (unknown line)
_jl_invoke at /Users/gabrielbaraldi/julia/src/gf.c:2873
ijl_apply_generic at /Users/gabrielbaraldi/julia/src/gf.c:3074
jl_apply at /Users/gabrielbaraldi/julia/src/./julia.h:1970 [inlined]
true_main at /Users/gabrielbaraldi/julia/src/jlapi.c:582
jl_repl_entrypoint at /Users/gabrielbaraldi/julia/src/jlapi.c:731
Allocations: 570978 (Pool: 570031; Big: 947); GC: 1
fish: Job 1, './julia test.jl' terminated by signal SIGTRAP (Trace or breakpoint trap)
```
instead of hanging silently

---------

Co-authored-by: Jameson Nash <vtjnash@gmail.com>
(cherry picked from commit d51ad06)
@KristofferC KristofferC mentioned this pull request Oct 3, 2023
31 tasks
KristofferC pushed a commit that referenced this pull request Oct 11, 2023
The guard instruction for unreachables and other crashes in aarch64 is
`brk`, in macos there isn't a distinction between a brk for a breakpoint
and one for a crash, as an attempt we check the value of `pc` when the
signal is triggered, if it is
`brk #0x1` we say that it is a crash and go into the sigdie_handler.

We should probably do the same in aarch64 linux, though I haven't dug
too deep into what values it uses for traps, and if what compiler used
matters, on apple I assumed we use clang/LLVM

It might be possible to test this by calling some inline assembly.

This means that something like
#51267 actually crashes with
```c
[16908] signal (5): Trace/BPT trap: 5
in expression starting at /Users/gabrielbaraldi/julia/test.jl:2
_collect at ./array.jl:768
collect at ./array.jl:757
top-level scope at /Users/gabrielbaraldi/julia/test.jl:5
_jl_invoke at /Users/gabrielbaraldi/julia/src/gf.c:2892
jl_toplevel_eval_flex at /Users/gabrielbaraldi/julia/src/toplevel.c:925
jl_toplevel_eval_flex at /Users/gabrielbaraldi/julia/src/toplevel.c:877
ijl_toplevel_eval at /Users/gabrielbaraldi/julia/src/toplevel.c:943 [inlined]
ijl_toplevel_eval_in at /Users/gabrielbaraldi/julia/src/toplevel.c:985
eval at ./boot.jl:383 [inlined]
include_string at ./loading.jl:2070
_jl_invoke at /Users/gabrielbaraldi/julia/src/gf.c:2873
ijl_apply_generic at /Users/gabrielbaraldi/julia/src/gf.c:3074
_include at ./loading.jl:2130
include at ./Base.jl:494
jfptr_include_46486 at /Users/gabrielbaraldi/julia/usr/lib/julia/sys.dylib (unknown line)
_jl_invoke at /Users/gabrielbaraldi/julia/src/gf.c:2873
ijl_apply_generic at /Users/gabrielbaraldi/julia/src/gf.c:3074
exec_options at ./client.jl:317
_start at ./client.jl:552
jfptr__start_83179 at /Users/gabrielbaraldi/julia/usr/lib/julia/sys.dylib (unknown line)
_jl_invoke at /Users/gabrielbaraldi/julia/src/gf.c:2873
ijl_apply_generic at /Users/gabrielbaraldi/julia/src/gf.c:3074
jl_apply at /Users/gabrielbaraldi/julia/src/./julia.h:1970 [inlined]
true_main at /Users/gabrielbaraldi/julia/src/jlapi.c:582
jl_repl_entrypoint at /Users/gabrielbaraldi/julia/src/jlapi.c:731
Allocations: 570978 (Pool: 570031; Big: 947); GC: 1
fish: Job 1, './julia test.jl' terminated by signal SIGTRAP (Trace or breakpoint trap)
```
instead of hanging silently

---------

Co-authored-by: Jameson Nash <vtjnash@gmail.com>
(cherry picked from commit d51ad06)
KristofferC added a commit that referenced this pull request Nov 2, 2023
Backported PRs:
- [x] #50932 <!-- types: fix hash values of Vararg -->
- [x] #50975 <!-- Use rr-safe `nopl; rdtsc` sequence -->
- [x] #50989 <!-- fix incorrect results in `expm1(::Union{Float16,
Float32})` -->
- [x] #51284 <!-- Avoid infinite loop when doing SIGTRAP in arm64-apple
-->
- [x] #51332 <!-- Add s4 field to Xoshiro -->
- [x] #51397 <!-- call Pkg precompile hook in latest world -->
- [x] #51405 <!-- Remove fallback that assigns a module to inlined
frames. -->
- [x] #51491 <!-- Throw clearer ArgumentError for strip with two string
args -->
- [x] #51531 <!-- fix `_tryonce_download_from_cache` (busybox.exe
download error) -->
- [x] #51541 <!-- Fix string index error in tab completion code -->
- [x] #51530 <!-- Don't mark nonlocal symbols as hidden -->
- [x] #51557 <!-- Fix last startup & shutdown precompiles -->
- [x] #51512 <!-- avoid limiting Type{Any} to Type -->
- [x] #51595 <!-- reset `maxprobe` on `empty!` -->
- [x] #51582 <!-- Aggressive constprop in LinearAlgebra.wrap -->
- [x] #51592 <!-- correctly track element pointer in heap snapshot -->
- [x] #51326 <!-- complete false & true more generally as vals -->
- [x] #51376 <!-- make `hash(::Xoshiro)` compatible with `==` -->
- [x] #51557 <!-- Fix last startup & shutdown precompiles -->
- [x] #51845 
- [x] #51840 
- [x] #50663 <!-- Fix Expr(:loopinfo) codegen -->
- [x] #51863 <!-- LLVM 15.0.7-9 -->

Contains multiple commits, manual intervention needed:

- [ ] #51035 <!-- refactor GC scanning code to reflect jl_binding_t are
now first class -->
- [ ] #51092 <!-- inference: fix bad effects for recursion -->

Non-merged PRs with backport label:
- [ ] #51479 <!-- prevent code loading from lookin in the versioned
environment when building Julia -->
- [ ] #51414 <!-- improvements on GC scheduler shutdown -->
- [ ] #51366 <!-- Handle infix operators in REPL completion -->
- [ ] #50919 <!-- Code loading: do the "skipping mtime check for stdlib"
check regardless of the value of `ispath(f)` -->
- [ ] #50824 <!-- Add some aliasing warnings to docstrings for mutating
functions in Base -->
- [ ] #49805 <!-- Limit TimeType subtraction to AbstractDateTime -->
@KristofferC KristofferC removed the backport 1.10 Change should be backported to the 1.10 release label Nov 2, 2023
nalimilan pushed a commit that referenced this pull request Nov 5, 2023
The guard instruction for unreachables and other crashes in aarch64 is
`brk`, in macos there isn't a distinction between a brk for a breakpoint
and one for a crash, as an attempt we check the value of `pc` when the
signal is triggered, if it is
`brk #0x1` we say that it is a crash and go into the sigdie_handler.

We should probably do the same in aarch64 linux, though I haven't dug
too deep into what values it uses for traps, and if what compiler used
matters, on apple I assumed we use clang/LLVM

It might be possible to test this by calling some inline assembly.

This means that something like
#51267 actually crashes with
```c
[16908] signal (5): Trace/BPT trap: 5
in expression starting at /Users/gabrielbaraldi/julia/test.jl:2
_collect at ./array.jl:768
collect at ./array.jl:757
top-level scope at /Users/gabrielbaraldi/julia/test.jl:5
_jl_invoke at /Users/gabrielbaraldi/julia/src/gf.c:2892
jl_toplevel_eval_flex at /Users/gabrielbaraldi/julia/src/toplevel.c:925
jl_toplevel_eval_flex at /Users/gabrielbaraldi/julia/src/toplevel.c:877
ijl_toplevel_eval at /Users/gabrielbaraldi/julia/src/toplevel.c:943 [inlined]
ijl_toplevel_eval_in at /Users/gabrielbaraldi/julia/src/toplevel.c:985
eval at ./boot.jl:383 [inlined]
include_string at ./loading.jl:2070
_jl_invoke at /Users/gabrielbaraldi/julia/src/gf.c:2873
ijl_apply_generic at /Users/gabrielbaraldi/julia/src/gf.c:3074
_include at ./loading.jl:2130
include at ./Base.jl:494
jfptr_include_46486 at /Users/gabrielbaraldi/julia/usr/lib/julia/sys.dylib (unknown line)
_jl_invoke at /Users/gabrielbaraldi/julia/src/gf.c:2873
ijl_apply_generic at /Users/gabrielbaraldi/julia/src/gf.c:3074
exec_options at ./client.jl:317
_start at ./client.jl:552
jfptr__start_83179 at /Users/gabrielbaraldi/julia/usr/lib/julia/sys.dylib (unknown line)
_jl_invoke at /Users/gabrielbaraldi/julia/src/gf.c:2873
ijl_apply_generic at /Users/gabrielbaraldi/julia/src/gf.c:3074
jl_apply at /Users/gabrielbaraldi/julia/src/./julia.h:1970 [inlined]
true_main at /Users/gabrielbaraldi/julia/src/jlapi.c:582
jl_repl_entrypoint at /Users/gabrielbaraldi/julia/src/jlapi.c:731
Allocations: 570978 (Pool: 570031; Big: 947); GC: 1
fish: Job 1, './julia test.jl' terminated by signal SIGTRAP (Trace or breakpoint trap)
```
instead of hanging silently

---------

Co-authored-by: Jameson Nash <vtjnash@gmail.com>
(cherry picked from commit d51ad06)
KristofferC added a commit that referenced this pull request Nov 7, 2023
Backported PRs:
- [x] #49357 <!-- Fix unclosed code fence in src/manual/methods.md -->
- [x] #50842 <!-- Avoid race conditions with recursive rm -->
- [x] #50858 <!-- Add a `threadpool` parameter to `Channel` constructor
-->
- [x] #50730 <!-- Fix integer overflow in `isapprox` -->
- [x] #50823 <!-- Make ranges more robust with unsigned indexes. -->
- [x] #50915 <!-- Add note the `Task` about sticky bit -->
- [x] #50989 <!-- fix incorrect results in `expm1(::Union{Float16,
Float32})` -->
- [x] #50912 <!-- Separate foreign threads into a :foreign threadpool
-->
- [x] #51019 <!-- fix a case of potentially use of undefined variable
when handling error in distributed message processing -->
- [x] #51222 <!-- Check again if the tty is open inside the IO lock -->
- [x] #51254 <!-- Ryu: make sure adding zeros does not overwrite
trailing dot -->
- [x] #51284 <!-- Avoid infinite loop when doing SIGTRAP in arm64-apple
-->
- [x] #51491 <!-- Throw clearer ArgumentError for strip with two string
args -->
- [x] #51531 <!-- fix `_tryonce_download_from_cache` (busybox.exe
download error) -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.9 Change should be backported to release-1.9 system:apple silicon Affects Apple Silicon only (Darwin/ARM64) - e.g. M1 and other M-series chips
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants