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

sql shell: seg fault in libedit when editing a large entry #71207

Closed
knz opened this issue Oct 6, 2021 · 1 comment · Fixed by #86457
Closed

sql shell: seg fault in libedit when editing a large entry #71207

knz opened this issue Oct 6, 2021 · 1 comment · Fixed by #86457
Assignees
@knz
Labels
A-cli-client CLI commands that pertain to using SQL features C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@knz
Copy link
Contributor

knz commented Oct 6, 2021 •
edited by exalate-issue-sync bot
Loading

found by @smcvey

Describe the problem

If the user recalls a history entry that's taller than the terminal and then attempts to edit it, the process crashes.

To Reproduce

  1. type in a tall query for example, select 1, <newline> 1, <newline> ... (10-20 lines), let it run
  2. resize the terminal to become shorter in height than the query
  3. recall the long query
  4. press the left arrow key to try to move "up" in the query. After a while, the shell crashes.

Expected behavior

no crash

Additional data / screenshots

the stack trace from Go's perspective:

fatal error: unexpected signal during runtime execution
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x468a3c7]

runtime stack:
runtime.throw(0x52e4b75, 0x2a)
        /usr/lib/go-1.16/src/runtime/panic.go:1117 +0x72
runtime.sigpanic()
        /usr/lib/go-1.16/src/runtime/signal_unix.go:718 +0x2e5

goroutine 1 [syscall]:
runtime.cgocall(0x452be10, 0xc00201f368, 0xc000000100)
        /usr/lib/go-1.16/src/runtime/cgocall.go:154 +0x5b fp=0xc00201f338 sp=0xc00201f300 pc=0x448f9b
github.com/knz/go-libedit/unix._C2func_go_libedit_gets(0x7fe6e2d51600, 0x7fe6e2deb140, 0x0, 0x0, 0xc002ddded8, 0xc002dddedc, 0x1, 0x0, 0x0, 0x0)
        _cgo_gotypes.go:193 +0x51 fp=0xc00201f368 sp=0xc00201f338 pc=0x427e0d1
github.com/knz/go-libedit/unix.EditLine.GetLine.func1(0xc002499880, 0xc002ddded8, 0xc002dddedc, 0x20, 0x7fe6e2deb140, 0x0)
        /home/spill/go/src/github.com/cockroachdb/cockroach/vendor/github.com/knz/go-libedit/unix/editline_unix.go:281 +0xe7 fp=0xc00201f3f0 sp=0xc00201f368 pc=0x4282047
github.com/knz/go-libedit/unix.EditLine.GetLine(0x0, 0x20, 0xb, 0x0, 0x0)
        /home/spill/go/src/github.com/cockroachdb/cockroach/vendor/github.com/knz/go-libedit/unix/editline_unix.go:281 +0xaa fp=0xc00201f468 sp=0xc00201f3f0 pc=0x42804aa
github.com/cockroachdb/cockroach/pkg/cli/clisqlshell.(*cliState).doReadLine(0xc0012367e0, 0x6, 0x5)
        /home/spill/go/src/github.com/cockroachdb/cockroach/pkg/cli/clisqlshell/sql.go:957 +0x60c fp=0xc00201f540 sp=0xc00201f468 pc=0x4287d0c
github.com/cockroachdb/cockroach/pkg/cli/clisqlshell.(*cliState).doRunShell(0xc0012367e0, 0x0, 0xc0001be000, 0xc0001be008, 0xc0001be398, 0x0, 0x0)
        /home/spill/go/src/github.com/cockroachdb/cockroach/pkg/cli/clisqlshell/sql.go:1714 +0xbe fp=0xc00201f5b8 sp=0xc00201f540 pc=0x428bffe
github.com/cockroachdb/cockroach/pkg/cli/clisqlshell.(*cliState).RunInteractive(0xc0012367e0, 0xc0001be000, 0xc0001be008, 0xc0001be398, 0x0, 0x0)
        /home/spill/go/src/github.com/cockroachdb/cockroach/pkg/cli/clisqlshell/sql.go:1677 +0x52 fp=0xc00201f600 sp=0xc00201f5b8 pc=0x428bf12
github.com/cockroachdb/cockroach/pkg/cli/clisqlcfg.(*Context).Run(0xc000ad7520, 0x6af9d28, 0xc001006c30, 0x6af9d28, 0xc001006c30)

The C stack trace, obtained by gdb:

#0  0x000000000468a3c7 in terminal_overwrite () at /home/spill/go/src/github.com/cockroachdb/cockroach/c-deps/libedit/src/terminal.c:643
#1  0x000000000468a246 in terminal_move_to_char () at /home/spill/go/src/github.com/cockroachdb/cockroach/c-deps/libedit/src/terminal.c:591
#2  0x0000000004686e4c in re_refresh_cursor () at /home/spill/go/src/github.com/cockroachdb/cockroach/c-deps/libedit/src/refresh.c:1082
#3  0x000000000468521d in el_wgets () at /home/spill/go/src/github.com/cockroachdb/cockroach/c-deps/libedit/src/read.c:546
#4  0x000000000452d315 in go_libedit_gets (el=0x7fffcaac2600, lprompt=0x7fffca80d000 "[email protected]:26257/defaultdb> ", rprompt=0x0, p_sigcfg=<optimized out>, count=0xc0027c89b8, interrupted=0xc0027c89bc,
    widechar=1) at c_editline.c:467
#5  0x000000000452be59 in _cgo_69d4664a3f57_C2func_go_libedit_gets (v=0xc0030b1368) at cgo-gcc-prolog:109
#6  0x00000000004b7c90 in runtime.asmcgocall () at /usr/lib/go-1.16/src/runtime/asm_amd64.s:667
#7  0x00000000004874fd in runtime.park_m (gp=0xc000102780) at /usr/lib/go-1.16/src/runtime/proc.go:3274

Environment:

crdb 21.2

Jira issue: CRDB-10440

Epic CRDB-22182
@knz knz added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Oct 6, 2021
@knz
Copy link
Contributor Author

knz commented Oct 6, 2021

One step would be to see if the issue repros with the latest libedit (our vendored copy is a few versions behind). Then we can debug it.

@knz knz added the A-cli-client CLI commands that pertain to using SQL features label Oct 6, 2021
@knz knz mentioned this issue Sep 26, 2022
Merged
craig bot pushed a commit that referenced this issue Dec 1, 2022
@knz @pav-kv
Merge #86457 #92335
f8bf34c 
86457: cli: replace libedit with bubbline r=ZhouXing19 a=knz

First commit from #88574.
Benefits from (but is not dependent on) #87606 server-side.

Fixes #21826
Fixes #71207
Fixes #71209
Fixes #57885

NB: this will benefit from upstream library releases based off the still-unmerged PRs listed in knz/bubbline#2.

Release justification: n/a will not merge before stability ends

Release note (cli change): The engine used as line editor in the
interactive shell (`cockroach sql`, `demo`) has been updated. It
includes numerous bug fixes and new features.
The previous engine can still be accessed via the env
var COCKROACH_SQL_FORCE_LIBEDIT. This support will be removed
in a later version.

92335: kvserver,logstore: introduce log StateLoader r=tbg a=pavelkalinnikov

Previously the `StateLoader` accessed both log and state machine state. This commit moves most of the log-specific accesstors to the `logstore` package.

Part of #91979
Release note: None

Co-authored-by: Raphael 'kena' Poss <[email protected]>
Co-authored-by: Pavel Kalinnikov <[email protected]>
@craig craig bot closed this as completed in #86457 Dec 1, 2022
@exalate-issue-sync exalate-issue-sync bot added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label Dec 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@knz knz

Labels
A-cli-client CLI commands that pertain to using SQL features C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

cli: replace libedit with bubbline knz/cockroach
1 participant
@knz