|Summary:||[8.3, -m32] FAIL: gdb.base/interrupt.exp: continue (the program exited)|
|Product:||gdb||Reporter:||Tom de Vries <vries>|
|Component:||gdb||Assignee:||Not yet assigned to anyone <unassigned>|
Description Tom de Vries 2019-05-21 11:25:03 UTC
With target board unix/-m32, I get these fails: ... FAIL: gdb.base/interrupt.exp: continue (the program exited) FAIL: gdb.base/interrupt.exp: echo data FAIL: gdb.base/interrupt.exp: Send Control-C, second time FAIL: gdb.base/interrupt.exp: signal SIGINT (the program is no longer running) ... for origin/gdb-8.3-branch. AFAICT, that should be fixed by backporting master commit: ... commit 3f52fdbcb599f76b4838020721ca6c9f1cc28f84 Author: Kevin Buettner <email@example.com> Date: Sat Mar 16 12:40:01 2019 -0700 Fix amd64->i386 linux syscall restart problem ...
Comment 1 Tom de Vries 2019-05-21 13:02:41 UTC
(In reply to Tom de Vries from comment #0) > AFAICT, that should be fixed by backporting master commit: > ... > commit 3f52fdbcb599f76b4838020721ca6c9f1cc28f84 > Author: Kevin Buettner <firstname.lastname@example.org> > Date: Sat Mar 16 12:40:01 2019 -0700 > > Fix amd64->i386 linux syscall restart problem > ... Proposed backport here ( https://sourceware.org/ml/gdb-patches/2019-05/msg00470.html ).
Comment 2 Tom de Vries 2019-07-12 11:46:30 UTC
Posted for pre-commit review: https://sourceware.org/ml/gdb-patches/2019-07/msg00344.html
Comment 3 email@example.com 2019-07-12 12:34:23 UTC
The gdb-8.3-branch branch has been updated by Tom de Vries <firstname.lastname@example.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=a252e71900abcaf6c817f4f9a1daa6c29d140d58 commit a252e71900abcaf6c817f4f9a1daa6c29d140d58 Author: Kevin Buettner <email@example.com> Date: Fri Jul 12 14:33:40 2019 +0200 Fix amd64->i386 linux syscall restart problem [ Backport of master commit 3f52fdbcb5. ] This commit fixes some failures in gdb.base/interrupt.exp when debugging a 32-bit i386 linux inferior from an amd64 host. When running the following test... make check RUNTESTFLAGS="--target_board unix/-m32 interrupt.exp" ... without this commit, I see the following output: FAIL: gdb.base/interrupt.exp: continue (the program exited) FAIL: gdb.base/interrupt.exp: echo data FAIL: gdb.base/interrupt.exp: Send Control-C, second time FAIL: gdb.base/interrupt.exp: signal SIGINT (the program is no longer running) ERROR: Undefined command "". ERROR: GDB process no longer exists === gdb Summary === When the test is run with this commit in place, we see 12 passes instead. This is the desired behavior. Analysis: On Linux, when a syscall is interrupted by a signal, the syscall may return -ERESTARTSYS when a signal occurs. Doing so indicates that the syscall is restartable. Then, depending on settings associated with the signal handler, and after the signal handler is called, the kernel can then either return -EINTR or can cause the syscall to be restarted. In this discussion, we are concerned with the latter case. On i386, the kernel returns this status via the EAX register. When debugging a 32-bit (i386) process from a 64-bit (amd64) GDB, the debugger fetches 64-bit registers even though the process being debugged is 32-bit. Since we're debugging a 32-bit target, only 32 bits are being saved in the register cache. Now, ideally, GDB would save all 64-bits in the regcache and then would be able to restore those same values when it comes time to continue the target. I've looked into doing this, but it's not easy and I don't see many benefits to doing so. One benefit, however, would be that EAX would appear as a negative value for doing syscall restarts. At the moment, GDB is setting the high 32 bits of RAX (and other registers too) to 0. So, when GDB restores EAX just prior to a syscall restart, the high 32 bits of RAX are zeroed, thus making it look like a positive value. For this particular purpose, we need to sign extend EAX so that RAX will appear as a negative value when EAX is set to -ERESTARTSYS. This in turn will cause the signal handling code in the kernel to recognize -ERESTARTSYS which will in turn cause the syscall to be restarted. This commit is based on work by Jan Kratochvil from 2009: https://sourceware.org/ml/gdb-patches/2009-11/msg00592.html Jan's patch had the sign extension code in amd64-nat.c. Several other native targets make use of this code, so it seemed better to move the sign extension code to a linux specific file. I also added similar code to gdbserver. Another approach is to fix the problem in the kernel. Hui Zhu tried to get a fix into the kernel back in 2014, but it was not accepted. Discussion regarding this approach may be found here: https://lore.kernel.org/patchwork/patch/457841/ Even if a fix were to be put into the kernel, we'd still need some kind of fix in GDB in order to support older kernels. Finally, I'll note that Fedora has been carrying a similar patch for at least nine years. Other distributions, including RHEL and CentOS have picked up this change and have been using it too. gdb/ChangeLog: PR gdb/24592 * amd64-linux-nat.c (amd64_linux_collect_native_gregset): New function. (fill_gregset): Call amd64_linux_collect_native_gregset instead of amd64_collect_native_gregset. (amd64_linux_nat_target::store_registers): Likewise. gdb/gdbserver/ChangeLog: PR gdb/24592 * linux-x86-low.c (x86_fill_gregset): Sign extend EAX value when using a 64-bit gdbserver.
Comment 4 firstname.lastname@example.org 2019-07-12 12:34:27 UTC
The gdb-8.3-branch branch has been updated by Tom de Vries <email@example.com>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=1e31ba653644faf289e8210ef25c7ab9a65fa865 commit 1e31ba653644faf289e8210ef25c7ab9a65fa865 Author: Kevin Buettner <firstname.lastname@example.org> Date: Fri Jul 12 14:33:40 2019 +0200 Fix regression caused by recently added syscall restart code [ Backport of master commit e90a813d96. ] This line of code... *(int64_t *) ptr = *(int32_t *) ptr; ...in linux-x86-low.c is not needed (and does not work correctly) within a 32-bit executable. I added an __x86_64__ ifdef (which is used extensively elsewhere in the file for like purposes) to prevent this code from being included in 32-bit builds. It fixes the following regressions when running on native i686-pc-linux-gnu: FAIL: gdb.server/abspath.exp: continue to main FAIL: gdb.server/connect-without-multi-process.exp: multiprocess=auto: continue to main FAIL: gdb.server/connect-without-multi-process.exp: multiprocess=off: continue to main FAIL: gdb.server/ext-restart.exp: restart: run to main FAIL: gdb.server/ext-restart.exp: run to main FAIL: gdb.server/ext-run.exp: continue to main FAIL: gdb.server/ext-wrapper.exp: print d FAIL: gdb.server/ext-wrapper.exp: restart: print d FAIL: gdb.server/ext-wrapper.exp: restart: run to marker FAIL: gdb.server/ext-wrapper.exp: run to marker FAIL: gdb.server/no-thread-db.exp: continue to breakpoint: after tls assignment FAIL: gdb.server/reconnect-ctrl-c.exp: first: stop with control-c FAIL: gdb.server/reconnect-ctrl-c.exp: second: stop with control-c FAIL: gdb.server/run-without-local-binary.exp: run test program until the end FAIL: gdb.server/server-kill.exp: continue to breakpoint: after server_pid assignment FAIL: gdb.server/server-kill.exp: tstatus FAIL: gdb.server/server-run.exp: continue to main gdb/gdbserver/ChangeLog: PR gdb/24592 * linux-x86-low.c (x86_fill_gregset): Don't compile 64-bit sign extension code on 32-bit builds.
Comment 5 Tom de Vries 2019-07-12 12:40:46 UTC
Patches backported, marking resolved-fixed.