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.

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

Remove recursion in bun shell interpreter to fix stack overflow #15323

Draft
wants to merge 21 commits into
base: main
Choose a base branch
from
Draft

Remove recursion in bun shell interpreter to fix stack overflow #15323

wants to merge 21 commits into from
+662 −315

Conversation

pfgithub
Copy link
Contributor

What does this PR do?

Fixes #15189

@pfgithub
add failing test
eb15e13
@pfgithub
don't initialize something to undefined that gets immediately used ne…
a06d678 
…xt loop?
@pfgithub
add more failing tests
128038d
@pfgithub
needsEscape(utf16,ascii) -> single unified needsEscape
cc9b371
@pfgithub
fix \b__bunstr_0 reading the next number too even though it was unrel…
19ceb43 
…ated. fix some errors writing to the str pool, returning their memory, and then invalidating it. fix an error panicing.
@pfgithub
another test
55a554e
@pfgithub
another test
fa3c7cd
@pfgithub
another test
18f5b75
@pfgithub
redo ~ expansion & fix it
0ae03c7
@pfgithub
debug log shell tokens
ed31a48
@pfgithub
add failing tests. string is closer to working than substitution but …
1c6c3d9 
…neither work
@pfgithub
remove the ~ '/' / '\\' logic in interpreter.zig as it's now handled …
8f50ca7 
…in shell.zig
@pfgithub
smoe more tests
d714937
@pfgithub
debug log bun shell command ast
069b963
@pfgithub
debug log the raw string too
12465ef
@pfgithub
fix '\' handling, still working on ast of "${""}" and ${""} and then …
38c41bc 
…have to fix execution of ["echo", "", ""]
@pfgithub
the big array doesn't segfault anymore, but there's lots more to convert
b43547b
@pfgithub
Merge branch 'main' into pfg/issue-15189-3
de1ab94
@pfgithub
more conversions to nextexec
41ba9f0
@pfgithub
add more tests and mark failing ones as todo
8b2b6fa
@pfgithub
more unrecursion progress
10ae1c4
@robobun
Copy link

robobun commented Nov 22, 2024 •
edited
Loading

❌ @pfgithub, your commit 10ae1c4 has 19 failures in #6596:

  • test/cli/install/registry/bun-install-registry.test.ts - timeout on 🐧 3.20 x64-baseline
  • test/integration/next-pages/test/dev-server.test.ts - 1 failing on 🐧 3.20 x64
  • test/integration/next-pages/test/dev-server.test.ts - 1 failing on 🐧 3.20 x64-baseline
  • test/v8/v8.test.ts - 22 failing on 🐧 3.20 aarch64
  • test/v8/v8.test.ts - 22 failing on 🐧 3.20 x64
  • test/v8/v8.test.ts - 22 failing on 🐧 3.20 x64-baseline
  • test/cli/install/bun-link.test.ts - 4 failing on 🐧 3.20 aarch64
  • test/cli/install/bun-link.test.ts - 4 failing on 🐧 3.20 x64
  • test/cli/install/bun-link.test.ts - 4 failing on 🐧 3.20 x64-baseline
  • test/integration/next-pages/test/next-build.test.ts - 1 failing on 🐧 20.04 x64
  • test/js/node/child_process/child_process.test.ts - timeout on 🐧 2023 aarch64
  • test/js/node/child_process/child_process.test.ts - 1 failing on 🐧 3.20 aarch64
  • test/js/node/child_process/child_process.test.ts - timeout on 🐧 2023 x64-baseline
  • test/js/node/child_process/child_process.test.ts - timeout on 🐧 2023 x64
  • test/js/node/child_process/child_process.test.ts - 1 failing on 🐧 3.20 x64
  • test/js/node/child_process/child_process.test.ts - 1 failing on 🐧 3.20 x64-baseline
  • test/cli/run/run-crash-handler.test.ts - timeout on 🐧 2023 aarch64
  • test/cli/run/run-crash-handler.test.ts - timeout on 🐧 2023 x64-baseline
  • test/cli/run/run-crash-handler.test.ts - timeout on 🐧 2023 x64
  • test/cli/install/bun-run.test.ts - timeout on 🐧 2023 aarch64
  • test/js/web/fetch/fetch-preconnect.test.ts - 1 failing on 🍎 14 aarch64
  • test/js/web/timers/setTimeout.test.js - 1 failing on 🐧 22.04 aarch64
  • test/js/node/test/parallel/fs-watch-recursive-linux-parallel-remove.test.js - 1 failing on 🐧 11 aarch64
  • test/js/node/test/parallel/fs-watch-recursive-linux-parallel-remove.test.js - 1 failing on 🐧 20.04 aarch64
  • test/js/node/test/parallel/fs-watch-recursive-linux-parallel-remove.test.js - 1 failing on 🐧 11 x64-baseline
  • test/js/node/test/parallel/fs-watch-recursive-linux-parallel-remove.test.js - 1 failing on 🐧 12 x64-baseline
  • test/js/node/test/parallel/fs-watch-recursive-linux-parallel-remove.test.js - 1 failing on 🐧 11 x64
  • test/js/node/test/parallel/fs-watch-recursive-linux-parallel-remove.test.js - 1 failing on 🐧 12 x64
  • test/js/node/test/parallel/fs-watch-recursive-linux-parallel-remove.test.js - 1 failing on 🐧 22.04 x64-baseline
  • test/js/node/test/parallel/fs-watch-recursive-linux-parallel-remove.test.js - 1 failing on 🐧 2023 aarch64
  • test/js/node/test/parallel/fs-watch-recursive-linux-parallel-remove.test.js - 1 failing on 🐧 20.04 x64-baseline
  • test/js/node/test/parallel/fs-watch-recursive-linux-parallel-remove.test.js - 1 failing on 🐧 20.04 x64
  • test/js/node/test/parallel/fs-watch-recursive-linux-parallel-remove.test.js - 1 failing on 🐧 22.04 x64
  • test/js/node/test/parallel/fs-watch-recursive-linux-parallel-remove.test.js - 1 failing on 🐧 2023 x64-baseline
  • test/js/node/test/parallel/fs-watch-recursive-linux-parallel-remove.test.js - 1 failing on 🐧 3.20 aarch64
  • test/js/node/test/parallel/fs-watch-recursive-linux-parallel-remove.test.js - 1 failing on 🐧 2023 x64
  • test/js/node/test/parallel/fs-watch-recursive-linux-parallel-remove.test.js - 1 failing on 🐧 3.20 x64
  • test/js/node/test/parallel/fs-watch-recursive-linux-parallel-remove.test.js - 1 failing on 🐧 3.20 x64-baseline
  • test/js/third_party/body-parser/express-bun-build-compile.test.ts - 1 failing on 🐧 2023 aarch64
  • test/js/third_party/body-parser/express-bun-build-compile.test.ts - 1 failing on 🐧 2023 x64-baseline
  • test/js/third_party/body-parser/express-bun-build-compile.test.ts - 1 failing on 🐧 2023 x64
  • test/regression/issue/ctrl-c.test.ts - 1 failing on 🍎 13 aarch64
  • test/js/node/test/parallel/http2-pipe.test.js - code 1 on 🍎 14 aarch64
  • test/bundler/cli.test.ts - 1 failing on 🐧 2023 aarch64
  • test/bundler/cli.test.ts - 1 failing on 🐧 2023 x64-baseline
  • test/bundler/cli.test.ts - 1 failing on 🐧 2023 x64
  • test/js/node/test/parallel/child-process-exec-timeout-kill.test.js - 1 failing on 🍎 13 aarch64
  • test/js/bun/http/serve.test.ts - 1 failing on 🍎 13 aarch64
  • test/js/bun/http/serve.test.ts - segmentation fault on 🐧 3.20 aarch64
  • test/js/bun/http/serve.test.ts - segmentation fault on 🐧 3.20 x64
  • test/js/bun/http/serve.test.ts - segmentation fault on 🐧 3.20 x64-baseline
  • test/js/node/dns/node-dns.test.js - 5 failing on 🪟 2019 x64-baseline
  • test/js/node/dns/node-dns.test.js - 4 failing on 🪟 2019 x64
  • test/js/bun/spawn/spawn-signal.test.ts - 2 failing on 🪟 2019 x64-baseline
    		•

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Reviewers

    @zackradisic zackradisic Awaiting requested review from zackradisic

    Assignees
    No one assigned
    Labels
    None yet
    Projects
    None yet
    Milestone
    No milestone
    Development

    Successfully merging this pull request may close these issues.

    Bun.$ crashes on macOS ARM64 with large array of non-empty strings
    2 participants
    @pfgithub @robobun