Alex Bennée <alex.ben...@linaro.org> writes: > It would be better to wrap the test in a function (static bool > is_connected()?) so the semantic meaning is clear in the code and we can > fix things in one place if needed.
That makes good sense to me. > How exactly did you create the segfault? Just starting with -s and > attaching to a running tasks works fine for me although I Can see > semihosting stuff would never get to gdb after connection. Making a semihosting call before GDB is connected results in dereferencing a NULL gdbserver_state.c_cpu pointer below gdb_do_syscallv. The sequence goes like this: 1. gdbserver_start is called during qemu startup, which calls init_gdbserver_state which sets gdbserver_state.init = true 2. application makes semihosting call (like putc) 3. semihosting code calls use_gdb_syscalls(), which returns true because gdbserver_state.init is true 4. gdb_do_syscallv checks gdbserver_state.init, which is true 5. gdb_do_syscallv uses gdbserver_state.c_cpu, which is still NULL and causes a segfault in qemu_cpu_kick > Hmm I don't see anything obviously wrong - although I note a bunch of > tests also check for ->fd which is probably a clearer indication of an > active connection. I'm sure this could be improved with a semantically > clearer code though. fd is < 0 only *after* a connection has failed, it is not set to -1 before a connection has started. I agree that using 'fd' is a good idea instead of c_cpu, but it would need to be combined with checking 'init' and initializing fd to -1 when init is set to true. In any case, hiding all of this behind a couple of functions seems like a good idea. For now, I'll continue to use c_cpu as that is independent of CONFIG_USER_ONLY *and* has the advantage of being initialized to NULL by default. It's marked with XXX in the patch as it seems like a bit of a kludge. > Yes it's a bit of a hack. I can imagine starting with a remote GDB > connection and then loosing it after opening a file descriptor would > result in Bad Things (tm). I'm not sure what the cleanest approach is to > handling the resulting mess. Hrm. use_gdb_syscalls caches the results of the first test, so we won't ever mix things, we'll just get some semihosting calls dropped when the gdb server is not connected. If use_gdb_syscalls checks for a valid connection, then gdb will never get semihosting calls if -S is not on the command line. If use_gdb_syscalls checks for gdbserver_state.init, then gdb will get semihosting calls whenever it is connected, otherwise those calls will be dropped. -- -keith
signature.asc
Description: PGP signature