I found a place that should be giving back the terminal to the target,
but only if the target was already owning it. So I need to add a
getter for who owns the terminal.
The trouble is that several places/target have their own globals to
track this state:
- inflow.c:terminal_is_ours
- remote.c:remote_async_terminal_ours_p
- linux-nat.c:async_terminal_is_ours
- go32-nat.c:terminal_is_ours
While one might think of adding a new target_ops method to query this,
conceptually, this state isn't really part of a particular target_ops.
Considering multi-target, the core shouldn't have to ask all targets
to know whether it's GDB that owns the terminal. There's only one GDB
(or rather, only one top level interpreter).
So what this comment does is add a new global that is tracked by the
core instead. A subsequent pass may later remove the other globals.
Tested on x86_64 Fedora 20, native and gdbserver.
gdb/
2014-10-17 Pedro Alves <palves@redhat.com>
* target.c (enum terminal_state): New enum.
(terminal_state): New global.
(target_terminal_init): New function.
(target_terminal_inferior): Skip if inferior already owns the
terminal.
(target_terminal_ours, target_terminal_ours_for_output): New
functions.
* target.h (target_terminal_init): Convert to function prototype.
(target_terminal_ours_for_output): Convert to function prototype
and tweak comment.
(target_terminal_ours): Convert to function prototype and tweak
comment.
* windows-nat.c (do_initial_windows_stuff): Call
target_terminal_init instead of child_terminal_init_with_pgrp.
When GDB wants to sync the thread list with the target's (e.g., due to
"info threads"), it calls update_thread_list:
update_thread_list (void)
{
prune_threads ();
target_find_new_threads ();
update_threads_executing ();
}
And then prune_threads does:
prune_threads (void)
{
struct thread_info *tp, *next;
for (tp = thread_list; tp; tp = next)
{
next = tp->next;
if (!thread_alive (tp))
delete_thread (tp->ptid);
}
}
Calling thread_live on each thread one by one is expensive.
E.g., on Linux, it ends up doing kill(SIG0) once for each thread. Not
a big deal, but still a bunch of syscalls...
With the remote target, it's cumbersome. That thread_alive call ends
up generating one T packet per thread:
Sending packet: $Tp2141.2150#82...Packet received: OK
Sending packet: $Tp2141.214f#b7...Packet received: OK
Sending packet: $Tp2141.2141#82...Packet received: OK
Sending packet: $qXfer:threads:read::0,fff#03...Packet received: l<threads>\n<thread id="p2141.2141" core="2"/>\n<thread id="p2141.214f" core="1"/>\n<thread id="p2141.2150" core="2"/>\n</threads>\n
That seems a bit silly when target_find_new_threads method
implementations will always fetch the whole current set of target
threads, and then add those that are not in GDB's thread list, to
GDB's thread list.
This patch thus pushes down the responsibility of pruning dead threads
to the target_find_new_threads method instead, so a target may
implement pruning dead threads however it wants.
Once we do that, target_find_new_threads becomes a misnomer, so the
patch renames it to target_update_thread_list.
The patch doesn't attempt to do any optimization to any target yet.
It simply exports prune_threads, and makes all implementations of
target_update_thread_list call that. It's meant to be a no-op.
gdb/
2014-10-15 Pedro Alves <palves@redhat.com>
* ada-tasks.c (print_ada_task_info, task_command_1): Adjust.
* bsd-uthread.c (bsd_uthread_find_new_threads): Rename to ...
(bsd_uthread_update_thread_list): ... this. Call prune_threads.
(bsd_uthread_target): Adjust.
* corelow.c (core_open): Adjust.
* dec-thread.c (dec_thread_find_new_threads): Update comment.
(dec_thread_update_thread_list): New function.
(init_dec_thread_ops): Adjust.
* gdbthread.h (prune_threads): New declaration.
* linux-thread-db.c (thread_db_find_new_threads): Rename to ...
(thread_db_update_thread_list): ... this. Call prune_threads.
(init_thread_db_ops): Adjust.
* nto-procfs.c (procfs_find_new_threads): Rename to ...
(procfs_update_thread_list): ... this. Call prune_threads.
(procfs_attach, procfs_create_inferior, init_procfs_targets):
Adjust.
* obsd-nat.c (obsd_find_new_threads): Rename to ...
(obsd_update_thread_list): ... this. Call prune_threads.
(obsd_add_target): Adjust.
* procfs.c (procfs_target): Adjust.
(procfs_notice_thread): Update comment.
(procfs_find_new_threads): Rename to ...
(procfs_update_thread_list): ... this. Call prune_threads.
* ravenscar-thread.c (ravenscar_update_inferior_ptid): Update
comment.
(ravenscar_wait): Adjust.
(ravenscar_find_new_threads): Rename to ...
(ravenscar_update_thread_list): ... this. Call prune_threads.
(init_ravenscar_thread_ops): Adjust.
* record-btrace.c (record_btrace_find_new_threads): Rename to ...
(record_btrace_update_thread_list): ... this. Adjust comment.
(init_record_btrace_ops): Adjust.
* remote.c (remote_threads_info): Rename to ...
(remote_update_thread_list): ... this. Call prune_threads.
(remote_start_remote, extended_remote_attach_1, init_remote_ops):
Adjust.
* sol-thread.c (check_for_thread_db): Adjust.
(sol_find_new_threads_callback): Rename to ...
(sol_update_thread_list_callback): ... this.
(sol_find_new_threads): Rename to ...
(sol_update_thread_list): ... this. Call prune_threads. Adjust.
(sol_get_ada_task_ptid, init_sol_thread_ops): Adjust.
* target-delegates.c: Regenerate.
* target.c (target_find_new_threads): Rename to ...
(target_update_thread_list): ... this.
* target.h (struct target_ops): Rename to_find_new_threads field
to to_update_thread_list.
(target_find_new_threads): Rename to ...
(target_update_thread_list): ... this.
* thread.c (prune_threads): Make extern.
(update_thread_list): Adjust.
Currently, with "set breakpoint auto-hw off", we'll still try to
insert a software breakpoint at addresses covered by supposedly
read-only or inacessible regions:
(top-gdb) mem 0x443000 0x450000 ro
(top-gdb) set mem inaccessible-by-default off
(top-gdb) disassemble
Dump of assembler code for function main:
0x0000000000443956 <+34>: movq $0x0,0x10(%rax)
=> 0x000000000044395e <+42>: movq $0x0,0x18(%rax)
0x0000000000443966 <+50>: mov -0x24(%rbp),%eax
0x0000000000443969 <+53>: mov %eax,-0x20(%rbp)
End of assembler dump.
(top-gdb) b *0x0000000000443969
Breakpoint 5 at 0x443969: file ../../src/gdb/gdb.c, line 29.
(top-gdb) c
Continuing.
warning: cannot set software breakpoint at readonly address 0x443969
Breakpoint 5, 0x0000000000443969 in main (argc=1, argv=0x7fffffffd918) at ../../src/gdb/gdb.c:29
29 args.argc = argc;
(top-gdb)
We warn, saying that the insertion can't be done, but then proceed
attempting the insertion anyway, and in case of manually added
regions, the insert actually succeeds.
This is a regression; GDB used to fail inserting the breakpoint. More
below.
I stumbled on this as I wrote a test that manually sets up a read-only
memory region with the "mem" command, in order to test GDB's behavior
with breakpoints set on read-only regions, even when the real memory
the breakpoints are set at isn't really read-only. I wanted that in
order to add a test that exercises software single-stepping through
read-only regions.
Note that the memory regions that target_memory_map returns aren't
like e.g., what would expect to see in /proc/PID/maps on Linux.
Instead, they're the physical memory map from the _debuggers_
perspective. E.g., a read-only region would be real ROM or flash
memory, while a read-only+execute mapping in /proc/PID/maps is still
read-write to the debugger (otherwise the debugger wouldn't be able to
set software breakpoints in the code segment).
If one tries to manually write to memory that falls within a memory
region that is known to be read-only, with e.g., "p foo = 1", then we
hit a check in memory_xfer_partial_1 before the write mananges to make
it to the target side.
But writing a software/memory breakpoint nowadays goes through
target_write_raw_memory, and unlike when writing memory with
TARGET_OBJECT_MEMORY, nothing on the TARGET_OBJECT_RAW_MEMORY path
checks whether we're trying to write to a read-only region.
At the time "breakpoint auto-hw" was added, we didn't have the
TARGET_OBJECT_MEMORY vs TARGET_OBJECT_RAW_MEMORY target object
distinction yet, and the code path in memory_xfer_partial that blocks
writes to read-only memory was hit for memory breakpoints too. With
GDB 6.8 we had:
warning: cannot set software breakpoint at readonly address 0000000000443943
Warning:
Cannot insert breakpoint 1.
Error accessing memory address 0x443943: Input/output error.
So I started out by fixing this by adding the memory region validation
to TARGET_OBJECT_RAW_MEMORY too.
But later, when testing against GDBserver, I realized that that would
only block software/memory breakpoints GDB itself inserts with
gdb/mem-break.c. If a target has a to_insert_breakpoint method, the
insertion request will still pass through to the target. So I ended
up converting the "cannot set breakpoint" warning in breakpoint.c to a
real error return, thus blocking the insertion sooner.
With that, we'll end up no longer needing the TARGET_OBJECT_RAW_MEMORY
changes once software single-step breakpoints are converted to real
breakpoints. We need them today as software single-step breakpoints
bypass insert_bp_location. But, it'll be best to leave that in as
safeguard anyway, for other direct uses of TARGET_OBJECT_RAW_MEMORY.
Tested on x86_64 Fedora 20, native and gdbserver.
gdb/
2014-10-01 Pedro Alves <palves@redhat.com>
* breakpoint.c (insert_bp_location): Error out if inserting a
software breakpoint at a read-only address.
* target.c (memory_xfer_check_region): New function, factored out
from ...
(memory_xfer_partial_1): ... this. Make the 'reg_len' local a
ULONGEST.
(target_xfer_partial) <TARGET_OBJECT_RAW_MEMORY>: Check the access
against the memory region attributes.
gdb/testsuite/
2014-10-01 Pedro Alves <palves@redhat.com>
* gdb.base/breakpoint-in-ro-region.c: New file.
* gdb.base/breakpoint-in-ro-region.exp: New file.
This commit renames target_stop_ptid as target_stop_and_wait and
target_continue_ptid as target_continue_no_signal. Comments are
updated to more fully describe the functions' behaviour.
gdb/ChangeLog:
* target/target.h (target_stop_ptid): Renamed as...
(target_stop_and_wait): New function. Updated comment.
All uses updated.
(target_continue_ptid): Renamed as...
(target_continue_no_signal): New function. Updated comment.
All uses updated.
This commit introduces two new functions to stop and restart target
processes that shared code can use and that clients must implement.
It also changes some shared code to use these functions.
gdb/ChangeLog:
* target/target.h (target_stop_ptid, target_continue_ptid):
Declare.
* target.c (target_stop_ptid, target_continue_ptid): New
functions.
* common/agent.c [!GDBSERVER]: Don't include infrun.h.
(agent_run_command): Always use target_stop_ptid and
target_continue_ptid.
gdb/gdbserver/ChangeLog:
* target.c (target_stop_ptid, target_continue_ptid): New
functions.
This introduces target/target.h. This file declares some functions
that the shared code can use and that clients must implement. It also
changes some shared code to use these functions.
gdb/ChangeLog:
* target/target.h: New file.
* Makefile.in (HFILES_NO_SRCDIR): Add target/target.h.
* target.h: Include target/target.h.
(target_read_memory, target_write_memory): Don't declare.
* target.c (target_read_uint32): New function.
* common/agent.c: Include target/target.h.
[!GDBSERVER]: Don't include target.h.
(helper_thread_id): Type changed to uint32_t.
(agent_get_helper_thread_id): Use target_read_uint32.
(agent_run_command): Always use target_read_memory and
target_write_memory.
(agent_capability): Type changed to uint32_t.
(agent_capability_check): Use target_read_uint32.
gdb/gdbserver/ChangeLog:
* target.h: Include target/target.h.
* target.c (target_read_memory, target_read_uint32)
(target_write_memory): New functions.
I thought that this home made implementation of a vector could be
replaced by the more standard VEC. The implementation seems to predate
the introduction of vec.h, so that would explain why it exists.
Ran make check before and after, no new failures.
gdb/ChangeLog:
2014-08-19 Simon Marchi <simon.marchi@ericsson.com>
* target.c (target_struct_size): Remove.
(target_struct_allocsize): Remove.
(DEFAULT_ALLOCSIZE): Remove.
(target_ops_p): New typedef.
(DEF_VEC_P (target_ops_p)): New vector type.
(target_structs): Change type to VEC (target_ops_p).
(add_target_with_completer): Replace "push" code by VEC_safe_push.
(find_default_run_target): Rewrite for loop following changes to
target_structs.
This commit moves the inclusion of errno.h to common-defs.h and
removes all other inclusions. Note that prior to this commit
server.h included errno.h protected by "#ifdef HAVE_ERRNO_H".
This protection was added with the Windows CE port, which is
currently broken. Since no other platform needs this, I have
removed the protection and the configury to support it.
gdb/
2014-08-07 Gary Benson <gbenson@redhat.com>
* common/common-defs.h: Include errno.h.
* defs.h: Do not include errno.h.
* ada-typeprint.c: Likewise.
* c-typeprint.c: Likewise.
* core-regset.c: Likewise.
* corefile.c: Likewise.
* corelow.c: Likewise.
* event-loop.c: Likewise.
* f-typeprint.c: Likewise.
* gnu-nat.c: Likewise.
* go32-nat.c: Likewise.
* i386gnu-nat.c: Likewise.
* m2-typeprint.c: Likewise.
* nat/linux-btrace.c: Likewise.
* p-typeprint.c: Likewise.
* procfs.c: Likewise.
* remote-sim.c: Likewise.
* rs6000-nat.c: Likewise.
* target.c: Likewise.
* typeprint.c: Likewise.
* ui-file.c: Likewise.
* valops.c: Likewise.
* valprint.c: Likewise.
gdb/gdbserver/
2014-08-07 Gary Benson <gbenson@redhat.com>
* configure.ac (AC_CHECK_HEADERS): Remove errno.h.
* configure: Regenerate.
* config.in: Likewise.
* server.h: Do not include errno.h.
* event-loop.c: Likewise.
* hostio-errno.c: Likewise.
* linux-low.c: Likewise.
* remote-utils.c: Likewise.
* spu-low.c: Likewise.
* utils.c: Likewise.
* gdbreplay.c: Unconditionally include errno.h.
Right now, "set debug target" acts a bit strangely.
Most target APIs only notice that it has changed when the target stack
is changed in some way. This is because many methods implement the
setting using the special debug target. However, a few spots do
change their behavior immediately -- any place explicitly checking
"targetdebug".
Some of this peculiar behavior is documented. However, I think that
it just isn't very useful for it to work this way. So, this patch
changes "set debug target" to take effect immediately in all cases.
This is done by simply calling update_current_target when the setting
is changed.
This required one small change in the test suite. Here a test was
expecting the current behavior.
Built and regtested on x86-64 Fedora 20.
2014-08-04 Tom Tromey <tromey@redhat.com>
* target.c (set_targetdebug): New function.
(initialize_targets): Pass set_targetdebug when creating "set
debug target".
2014-08-04 Tom Tromey <tromey@redhat.com>
* gdb.texinfo (Debugging Output): Update for change to "set debug
target".
2014-08-04 Tom Tromey <tromey@redhat.com>
* gdb.base/sss-bp-on-user-bp-2.exp: Expect output from "set debug
target 0".
This is a follow-on to the patch to auto-generate target debug methods.
While working on that patch I noticed that the to_open debug setting
will never work. There is no path by which debug_to_open can be
called.
This patch fixes the problem by using a generic function as the
implementation of the various "target" subcommands, and then putting
the debug printing there.
This is also a tiny step toward fixing PR 7250 (and apparently why
command contexts were introduced).
Built and regtested on x86-64 Fedora 20.
2014-07-30 Tom Tromey <tromey@redhat.com>
* target.c (open_target): New function.
(add_target_with_completer, add_deprecated_target_alias): Use
set_cmd_sfunc, set_cmd_context.
(debug_to_open): Remove.
(setup_target_debug): Update.
While working on target_is_pushed, I noticed that it is written in a
strange way. The code currently keeps an extra indirection, where a
simple linked list traversal is all that is needed. It seems likely
this was done by copying and pasting other code. However, there is no
reason to do this and the more obvious code is simpler to reason
about. So, this patch change the implementation.
2014-07-29 Tom Tromey <tromey@redhat.com>
* target.c (target_is_pushed): Simplify.
The target debug methods are inconsistently maintained. Most to_*
methods have some kind of targetdebug awareness, but not all of them
do. The ones that do vary in the quantity and quality of output they
generate.
This patch changes most of the target debug methods to be
automatically generated. All the arguments are printed, and separate
lines are printed for entering and existing the outermost call to the
target stack.
For example now you'd see:
-> multi-thread->to_terminal_ours (...)
-> multi-thread->to_is_async_p (...)
<- multi-thread->to_is_async_p (0x1ebb580) = 1
<- multi-thread->to_terminal_ours (0x1ebb580)
-> multi-thread->to_thread_address_space (...)
<- multi-thread->to_thread_address_space (0x1ebb580, 26802) = 1
In this case you can see nested calls. The "multi-thread" on the left
hand side is the topmost target's shortname.
There are some oddities with this patch. I'm on the fence about it
all, I really just wrote it on a whim.
It's not simple to convert every possible method, since a few don't
participate in target delegation.
Printing is done by type, so I introduced some new
debug-printing-specific typedefs to handle cases where it is nicer to
do something else.
On the plus side, this lays the groundwork for making targetdebug
affect every layer of the target stack. The idea would be to wrap
each target_ops in the stack with its own debug_target, and then you
could see calls propagate down the stack and back up; I suppose with
indentation to make it prettier. (That said there are some gotchas
lurking in this idea due to target stack introspection.)
Regtested on x86-64 Fedora 20.
2014-07-24 Tom Tromey <tromey@redhat.com>
* make-target-delegates (munge_type, write_debugmethod): New
functions.
(debug_names): New global.
($TARGET_DEBUG_PRINTER): New global.
(write_function_header): Strip TARGET_DEBUG_PRINTER from the type
name.
Write debug methods. Generate init_debug_target.
* target-debug.h: New file.
* target-delegates.c: Rebuild.
* target.c: Include target-debug.h.
(debug_target): Hoist definition.
(target_kill, target_get_section_table, target_memory_map)
(target_flash_erase, target_flash_done, target_detach)
(target_disconnect, target_wait, target_resume)
(target_pass_signals, target_program_signals, target_follow_fork)
(target_mourn_inferior, target_search_memory)
(target_thread_address_space, target_close)
(target_find_new_threads, target_core_of_thread)
(target_verify_memory, target_insert_mask_watchpoint)
(target_remove_mask_watchpoint): Remove targetdebug code.
(debug_to_post_attach, debug_to_prepare_to_store)
(debug_to_files_info, debug_to_insert_breakpoint)
(debug_to_remove_breakpoint, debug_to_can_use_hw_breakpoint)
(debug_to_region_ok_for_hw_watchpoint)
(debug_to_can_accel_watchpoint_condition)
(debug_to_stopped_by_watchpoint, debug_to_stopped_data_address)
(debug_to_watchpoint_addr_within_range)
(debug_to_insert_hw_breakpoint, debug_to_remove_hw_breakpoint)
(debug_to_insert_watchpoint, debug_to_remove_watchpoint)
(debug_to_terminal_init, debug_to_terminal_inferior)
(debug_to_terminal_ours_for_output, debug_to_terminal_ours)
(debug_to_terminal_save_ours, debug_to_terminal_info)
(debug_to_load, debug_to_post_startup_inferior)
(debug_to_insert_fork_catchpoint)
(debug_to_remove_fork_catchpoint)
(debug_to_insert_vfork_catchpoint)
(debug_to_remove_vfork_catchpoint)
(debug_to_insert_exec_catchpoint)
(debug_to_remove_exec_catchpoint, debug_to_has_exited)
(debug_to_can_run, debug_to_thread_architecture, debug_to_stop)
(debug_to_rcmd, debug_to_pid_to_exec_file): Remove.
(setup_target_debug): Call init_debug_target.
* target.h (TARGET_DEBUG_PRINTER): New macro.
(struct target_ops) <to_resume, to_wait, to_pass_signals,
to_program_signals>: Use TARGET_DEBUG_PRINTER.
This fixes PR gdb/17130.
The bug is that some code in utils.c was not updated during the target
delegation change:
if (job_control
/* If there is no terminal switching for this target, then we can't
possibly get screwed by the lack of job control. */
|| current_target.to_terminal_ours == NULL)
fatal ("Quit");
else
fatal ("Quit (expect signal SIGINT when the program is resumed)");
After the delegation change, to_terminal_ours will never be NULL.
I think this bug can be seen before the target delegation change by
enabling target debugging -- this would also cause to_terminal_ours to
be non-NULL.
The fix is to introduce a new target_supports_terminal_ours function,
that properly checks the target stack. This is not perhaps ideal, but
I think is a reasonable-enough approach, and in keeping with some
other existing code of the same form.
This patch also fixes a similar bug in target_supports_delete_record.
2014-07-18 Tom Tromey <tromey@redhat.com>
PR gdb/17130:
* utils.c (quit): Use target_supports_terminal_ours.
* target.h (target_supports_terminal_ours): Declare.
* target.c (target_supports_delete_record): Don't check
to_delete_record against NULL.
(target_supports_terminal_ours): New function.
This fixes the record "run" regression pointed out by Marc Khouzam:
https://sourceware.org/ml/gdb/2014-06/msg00096.html
The bug is that target_require_runnable must agree with the handling
of the "run" target, but currently it is out of sync. This patch
fixes the problem by changing target_require_runnable to also ignore
the record_stratum.
Built and regtested on x86-64 Fedora 20.
New test case included.
2014-07-14 Tom Tromey <tromey@redhat.com>
* target.c (target_require_runnable): Also check record_stratum.
Update comment.
2014-07-14 Tom Tromey <tromey@redhat.com>
* gdb.reverse/rerun-prec.c: New file.
* gdb.reverse/rerun-prec.exp: New file.
This changes to_info_record to use target delegation.
Also, target_info_record was unused, so this patch removes it.
2014-07-07 Tom Tromey <tromey@redhat.com>
* target-delegates.c: Rebuild.
* target.c (target_info_record): Remove.
* record.c (info_record_command): Unconditionally call
to_info_record.
* target.h (struct target_ops) <to_info_record>: Use
TARGET_DEFAULT_IGNORE.
(target_info_record): Remove.
This converts to_get_thread_local_address to use
TARGET_DEFAULT_NORETURN. One possible oddity is that this changes the
text of the kind of exception thrown in some cases. This doesn't seem
to be a problem; in fact perhaps the final call to 'error' in
target_translate_tls_address should be changed to call
generic_tls_error.
2014-07-07 Tom Tromey <tromey@redhat.com>
* target.h (struct target_ops) <to_get_thread_local_address>: Use
TARGET_DEFAULT_NORETURN.
* target.c (generic_tls_error): New function.
(target_translate_tls_address): Don't search target stack.
* target-delegates.c: Rebuild.
* ppc-linux-tdep.c (ppc_linux_spe_context): Don't search target
stack.
* linux-thread-db.c (thread_db_get_thread_local_address):
Unconditionally call beneath target.
This makes the argument to the target_ops to_load method "const", and
fixes up the fallout. Tested by rebuilding all the affected files.
2014-06-26 Tom Tromey <tromey@redhat.com>
* defs.h (generic_load): Update.
* m32r-rom.c (m32r_load_gen): Make "filename" const.
* monitor.c (monitor_load): Make "args" const.
* remote-m32r-sdi.c (m32r_load): Make "args" const.
* remote-mips.c (mips_load_srec, pmon_load_fast): Make "args"
const.
(mips_load): Make "file" const.
* remote-sim.c (gdbsim_load): Make "args" const.
* remote.c (remote_load): Make "name" const.
* symfile.c (generic_load): Make "args" const.
* target-delegates.c: Rebuild.
* target.c (target_load): Make "arg" const.
(debug_to_load): Make "args" const.
* target.h (struct target_ops) <to_load>: Make parameter const.
(target_load): Update.
Add new target functions to_prepare_to_generate_core and
to_done_generating_core that are called before and after generating a core
file, respectively.
This allows targets to prepare for core file generation and to clean up
afterwards.
gdb/
* target.h (target_ops) <to_prepare_to_generate_core>
<to_done_generating_core>: New.
(target_prepare_to_generate_core, target_done_generating_core): New.
* target.c (target_prepare_to_generate_core)
(target_done_generating_core): New.
* target-delegates.c: Regenerate.
* gcore.c: (write_gcore_file): Rename to ...
(write_gcore_file_1): ...this.
(write_gcore_file): Call target_prepare_to_generate_core
and target_done_generating_core.
This makes a parameter of to_info_proc const and then fixes up some
fallout, including parameters in a couple of gdbarch methods.
I could not test the procfs.c change. I verified it by inspection.
If this causes an error here, it will be trivial to fix.
2014-06-16 Tom Tromey <tromey@redhat.com>
* target.h (struct target_ops) <to_info_proc>: Make parameter
const.
(target_info_proc): Update.
* target.c (target_info_proc): Make "args" const.
* procfs.c (procfs_info_proc): Update.
* linux-tdep.c (linux_info_proc): Update.
(linux_core_info_proc_mappings): Make "args" const.
(linux_core_info_proc): Update.
* gdbarch.sh (info_proc, core_info_proc): Make "args" const.
* gdbarch.c: Rebuild.
* gdbarch.h: Rebuild.
* corelow.c (core_info_proc): Update.
This converts to_thread_address_space to use TARGET_DEFAULT_FUNC.
This method was one of a handful not using the normal target
delegation approach. The only rationale here is consistency in the
target vector.
Built and regtested on x86-64 Fedora 20.
2014-06-04 Tom Tromey <tromey@redhat.com>
* target-delegates.c: Rebuild.
* target.c (default_thread_address_space): New function.
(target_thread_address_space): Simplify.
* target.h (struct target_ops) <to_thread_address_space>: Add
TARGET_DEFAULT_FUNC.
This finally makes background execution commands possible by default.
However, in order to do that, there's one last thing we need to do --
we need to separate the MI and target notions of "async". Unlike the
CLI, where the user explicitly requests foreground vs background
execution in the execution command itself (c vs c&), MI chose to treat
"set target-async" specially -- setting it changes the default
behavior of execution commands.
So, we can't simply "set target-async" default to on, as that would
affect MI frontends. Instead we have to make the setting MI-specific,
and teach MI about sync commands on top of an async target.
Because the "target" word in "set target-async" ends up as a potential
source of confusion, the patch adds a "set mi-async" option, and makes
"set target-async" a deprecated alias.
Rather than make the targets always async, this patch introduces a new
"maint set target-async" option so that the GDB developer can control
whether the target is async. This makes it simpler to debug issues
arising only in the synchronous mode; important because sync mode
seems unlikely to go away.
Unlike in previous revisions, "set target-async" does not affect this
new maint parameter. The rationale for this is that then one can
easily run the test suite in the "maint set target-async off" mode and
have tests that enable mi-async fail just like they fail on
non-async-capable targets. This emulation is exactly the point of the
maint option.
I had asked Tom in a previous iteration to split the actual change of
the target async default to a separate patch, but it turns out that
that is quite awkward in this version of the patch, because with MI
async and target async decoupled (unlike in previous versions), if we
don't flip the default at the same time, then just "set target-async
on" alone never actually manages to do anything. It's best to not
have that transitory state in the tree.
Given "set target-async on" now only has effect for MI, the patch goes
through the testsuite removing it from non-MI tests. MI tests are
adjusted to use the new and less confusing "mi-async" spelling.
2014-05-29 Pedro Alves <palves@redhat.com>
Tom Tromey <tromey@redhat.com>
* NEWS: Mention "maint set target-async", "set mi-async", and that
background execution commands are now always available.
* target.h (target_async_permitted): Update comment.
* target.c (target_async_permitted, target_async_permitted_1):
Default to 1.
(set_target_async_command): Rename to ...
(maint_set_target_async_command): ... this.
(show_target_async_command): Rename to ...
(maint_show_target_async_command): ... this.
(_initialize_target): Adjust.
* infcmd.c (prepare_execution_command): Make extern.
* inferior.h (prepare_execution_command): Declare.
* infrun.c (set_observer_mode): Leave target async alone.
* mi/mi-interp.c (mi_interpreter_init): Install
mi_on_sync_execution_done as sync_execution_done observer.
(mi_on_sync_execution_done): New function.
(mi_execute_command_input_handler): Don't print the prompt if we
just started a synchronous command with an async target.
(mi_on_resume): Check sync_execution before printing prompt.
* mi/mi-main.h (mi_async_p): Declare.
* mi/mi-main.c: Include gdbcmd.h.
(mi_async_p): New function.
(mi_async, mi_async_1): New globals.
(set_mi_async_command, show_mi_async_command, mi_async): New
functions.
(exec_continue): Call prepare_execution_command.
(run_one_inferior, mi_cmd_exec_run, mi_cmd_list_target_features)
(mi_execute_async_cli_command): Use mi_async_p.
(_initialize_mi_main): Install "set mi-async". Make
"target-async" a deprecated alias.
2014-05-29 Pedro Alves <palves@redhat.com>
Tom Tromey <tromey@redhat.com>
* gdb.texinfo (Non-Stop Mode): Remove "set target-async 1"
from example.
(Asynchronous and non-stop modes): Document '-gdb-set mi-async'.
Mention that target-async is now deprecated.
(Maintenance Commands): Document maint set/show target-async.
2014-05-29 Pedro Alves <palves@redhat.com>
Tom Tromey <tromey@redhat.com>
* gdb.base/async-shell.exp: Don't enable target-async.
* gdb.base/async.exp
* gdb.base/corefile.exp (corefile_test_attach): Remove 'async'
parameter. Adjust.
(top level): Don't test with "target-async".
* gdb.base/dprintf-non-stop.exp: Don't enable target-async.
* gdb.base/gdb-sigterm.exp: Don't test with "target-async".
* gdb.base/inferior-died.exp: Don't enable target-async.
* gdb.base/interrupt-noterm.exp: Likewise.
* gdb.mi/mi-async.exp: Use "mi-async" instead of "target-async".
* gdb.mi/mi-nonstop-exit.exp: Likewise.
* gdb.mi/mi-nonstop.exp: Likewise.
* gdb.mi/mi-ns-stale-regcache.exp: Likewise.
* gdb.mi/mi-nsintrall.exp: Likewise.
* gdb.mi/mi-nsmoribund.exp: Likewise.
* gdb.mi/mi-nsthrexec.exp: Likewise.
* gdb.mi/mi-watch-nonstop.exp: Likewise.
* gdb.multi/watchpoint-multi.exp: Adjust comment.
* gdb.python/py-evsignal.exp: Don't enable target-async.
* gdb.python/py-evthreads.exp: Likewise.
* gdb.python/py-prompt.exp: Likewise.
* gdb.reverse/break-precsave.exp: Don't test with "target-async".
* gdb.server/solib-list.exp: Don't enable target-async.
* gdb.threads/thread-specific-bp.exp: Likewise.
* lib/mi-support.exp: Adjust to use mi-async.
If one sets a breakpoint with a condition that involves calling a
function in the inferior, and then the condition evaluates false, GDB
outputs one *running event for each time the program hits the
breakpoint. E.g.,
$ gdb return-false -i=mi
(gdb)
start
...
(gdb)
b 14 if return_false ()
&"b 14 if return_false ()\n"
~"Breakpoint 2 at 0x4004eb: file return-false.c, line 14.\n"
...
^done
(gdb)
c
&"c\n"
~"Continuing.\n"
^running
*running,thread-id=(...)
(gdb)
*running,thread-id=(...)
*running,thread-id=(...)
*running,thread-id=(...)
*running,thread-id=(...)
*running,thread-id=(...)
... repeat forever ...
An easy way a user can trip on this is with a dprintf with "set
dprintf-style call". In that case, a dprintf is just a breakpoint
that when hit GDB calls the printf function in the inferior, and then
resumes it, just like the case above.
If the breakpoint/dprintf is set in a loop, then these spurious events
can potentially slow down a frontend much, if it decides to refresh
its GUI whenever it sees this event (Eclipse is one such case).
When we run an infcall, we pretend we don't actually run the inferior.
This is already handled for the usual case of calling a function
directly from the CLI:
(gdb)
p return_false ()
&"p return_false ()\n"
~"$1 = 0"
~"\n"
^done
(gdb)
Note no *running, nor *stopped events. That's handled by:
static void
mi_on_resume (ptid_t ptid)
{
...
/* Suppress output while calling an inferior function. */
if (tp->control.in_infcall)
return;
and equivalent code on normal_stop.
However, in the cases of the PR, after finishing the infcall there's
one more resume, and mi_on_resume doesn't know that it should suppress
output then too, somehow.
The "running/stopped" state is a high level user/frontend state.
Internal stops are invisible to the frontend. If follows from that
that we should be setting the thread to running at a higher level
where we still know the set of threads the user _intends_ to resume.
Currently we mark a thread as running from within target_resume, a low
level target operation. As consequence, today, if we resume a
multi-threaded program while stopped at a breakpoint, we see this:
-exec-continue
^running
*running,thread-id="1"
(gdb)
*running,thread-id="all"
The first *running was GDB stepping over the breakpoint, and the
second is GDB finally resuming everything.
Between those two *running's, threads other than "1" still have their
state set to stopped. That's bogus -- in async mode, this opens a
tiny window between both resumes where the user might try to run
another execution command to threads other than thread 1, and very
much confuse GDB.
That is, the "step" below should fail the "step", complaining that the
thread is running:
(gdb) c -a &
(gdb) thread 2
(gdb) step
IOW, threads that GDB happens to not resume immediately (say, because
it needs to step over a breakpoint) shall still be marked as running.
Then, if we move marking threads as running to a higher layer,
decoupled from target_resume, plus skip marking threads as running
when running an infcall, the spurious *running events disappear,
because there will be no state transitions at all.
I think we might end up adding a new thread state -- THREAD_INFCALL or
some such, however since infcalls are always synchronous today, I
didn't find a need. There's no way to execute a CLI/MI command
directly from the prompt if some thread is running an infcall.
Tested on x86_64 Fedora 20.
gdb/
2014-05-29 Pedro Alves <palves@redhat.com>
PR PR15693
* infrun.c (resume): Determine how much to resume depending on
whether the caller wanted a step, not whether we can hardware step
the target. Mark all threads that we intend to run as running,
unless we're calling an inferior function.
(normal_stop): If the thread is running an infcall, don't finish
thread state.
* target.c (target_resume): Don't mark threads as running here.
gdb/testsuite/
2014-05-29 Pedro Alves <palves@redhat.com>
Hui Zhu <hui@codesourcery.com>
PR PR15693
* gdb.mi/mi-condbreak-call-thr-state-mt.c: New file.
* gdb.mi/mi-condbreak-call-thr-state-st.c: New file.
* gdb.mi/mi-condbreak-call-thr-state.c: New file.
* gdb.mi/mi-condbreak-call-thr-state.exp: New file.
Move infrun.c declarations out of inferior.h to a new infrun.h file.
Tested by building on:
i686-w64-mingw32, enable-targets=all
x86_64-linux, enable-targets=all
i586-pc-msdosdjgpp
And also grepped the whole tree for each symbol moved to find where
infrun.h might be necessary.
gdb/
2014-05-22 Pedro Alves <palves@redhat.com>
* inferior.h (debug_infrun, debug_displaced, stop_on_solib_events)
(sync_execution, sched_multi, step_stop_if_no_debug, non_stop)
(disable_randomization, enum exec_direction_kind)
(execution_direction, stop_registers, start_remote)
(clear_proceed_status, proceed, resume, user_visible_resume_ptid)
(wait_for_inferior, normal_stop, get_last_target_status)
(prepare_for_detach, fetch_inferior_event, init_wait_for_inferior)
(insert_step_resume_breakpoint_at_sal)
(follow_inferior_reset_breakpoints, stepping_past_instruction_at)
(set_step_info, print_stop_event, signal_stop_state)
(signal_print_state, signal_pass_state, signal_stop_update)
(signal_print_update, signal_pass_update)
(update_signals_program_target, clear_exit_convenience_vars)
(displaced_step_dump_bytes, update_observer_mode)
(signal_catch_update, gdb_signal_from_command): Move
declarations ...
* infrun.h: ... to this new file.
* amd64-tdep.c: Include infrun.h.
* annotate.c: Include infrun.h.
* arch-utils.c: Include infrun.h.
* arm-linux-tdep.c: Include infrun.h.
* arm-tdep.c: Include infrun.h.
* break-catch-sig.c: Include infrun.h.
* breakpoint.c: Include infrun.h.
* common/agent.c: Include infrun.h instead of inferior.h.
* corelow.c: Include infrun.h.
* event-top.c: Include infrun.h.
* go32-nat.c: Include infrun.h.
* i386-tdep.c: Include infrun.h.
* inf-loop.c: Include infrun.h.
* infcall.c: Include infrun.h.
* infcmd.c: Include infrun.h.
* infrun.c: Include infrun.h.
* linux-fork.c: Include infrun.h.
* linux-nat.c: Include infrun.h.
* linux-thread-db.c: Include infrun.h.
* monitor.c: Include infrun.h.
* nto-tdep.c: Include infrun.h.
* procfs.c: Include infrun.h.
* record-btrace.c: Include infrun.h.
* record-full.c: Include infrun.h.
* remote-m32r-sdi.c: Include infrun.h.
* remote-mips.c: Include infrun.h.
* remote-notif.c: Include infrun.h.
* remote-sim.c: Include infrun.h.
* remote.c: Include infrun.h.
* reverse.c: Include infrun.h.
* rs6000-tdep.c: Include infrun.h.
* s390-linux-tdep.c: Include infrun.h.
* solib-irix.c: Include infrun.h.
* solib-osf.c: Include infrun.h.
* solib-svr4.c: Include infrun.h.
* target.c: Include infrun.h.
* top.c: Include infrun.h.
* windows-nat.c: Include infrun.h.
* mi/mi-interp.c: Include infrun.h.
* mi/mi-main.c: Include infrun.h.
* python/py-threadevent.c: Include infrun.h.
Sometimes it's useful to be able to disable the automatic connection
to the native target. E.g., sometimes GDB disconnects from the
extended-remote target I was debugging, without me noticing it, and
then I do "run". That starts the program locally, and only after a
little head scratch session do I figure out the program is running
locally instead of remotely as intended. Same thing with "attach",
"info os", etc.
With the patch, we now can have this instead:
(gdb) set auto-connect-native-target off
(gdb) target extended-remote :9999
...
*gdb disconnects*
(gdb) run
Don't know how to run. Try "help target".
To still be able to connect to the native target with
auto-connect-native-target set to off, I've made "target native" work
instead of erroring out as today.
Before:
(gdb) target native
Use the "run" command to start a native process.
After:
(gdb) target native
Done. Use the "run" command to start a process.
(gdb) maint print target-stack
The current target stack is:
- native (Native process)
- exec (Local exec file)
- None (None)
(gdb) run
Starting program: ./a.out
...
I've also wanted this for the testsuite, when running against the
native-extended-gdbserver.exp board (runs against gdbserver in
extended-remote mode). With a non-native-target board, it's always a
bug to launch a program with the native target. Turns out we still
have one such case this patch catches:
(gdb) break main
Breakpoint 1 at 0x4009e5: file ../../../src/gdb/testsuite/gdb.base/coremaker.c, line 138.
(gdb) run
Don't know how to run. Try "help target".
(gdb) FAIL: gdb.base/corefile.exp: run: with core
On the patch itself, probably the least obvious bit is the need to go
through all targets, and move the unpush_target call to after the
generic_mourn_inferior call instead of before. This is what
inf-ptrace.c does too, ever since multi-process support was added.
The reason inf-ptrace.c does things in that order is that in the
current multi-process/single-target model, we shouldn't unpush the
target if there are still other live inferiors being debugged. The
check for that is "have_inferiors ()" (a misnomer nowadays...), which
does:
have_inferiors (void)
{
for (inf = inferior_list; inf; inf = inf->next)
if (inf->pid != 0)
return 1;
It's generic_mourn_inferior that ends up clearing inf->pid, so we need
to call it before the have_inferiors check. To make all native
targets behave the same WRT to explicit "target native", I've added an
inf_child_maybe_unpush_target function that targets call instead of
calling unpush_target directly, and as that includes the
have_inferiors check, I needed to adjust the targets.
Tested on x86_64 Fedora 20, native, and also with the
extended-gdbserver board.
Confirmed a cross build of djgpp gdb still builds.
Smoke tested a cross build of Windows gdb under Wine.
Untested otherwise.
gdb/
2014-05-21 Pedro Alves <palves@redhat.com>
* inf-child.c (inf_child_ops, inf_child_explicitly_opened): New
globals.
(inf_child_open_target): New function.
(inf_child_open): Use inf_child_open_target to push the target
instead of erroring out.
(inf_child_disconnect, inf_child_close)
(inf_child_maybe_unpush_target): New functions.
(inf_child_target): Install inf_child_disconnect and
inf_child_close. Store a pointer to the returned object.
* inf-child.h (inf_child_open_target, inf_child_maybe_unpush): New
declarations.
* target.c (auto_connect_native_target): New global.
(show_default_run_target): New function.
(find_default_run_target): Return NULL if automatically connecting
to the native target is disabled.
(_initialize_target): Install set/show auto-connect-native-target.
* NEWS: Mention "set auto-connect-native-target", and "target
native".
* linux-nat.c (super_close): New global.
(linux_nat_close): Call super_close.
(linux_nat_add_target): Store a pointer to the base class's
to_close method.
* inf-ptrace.c (inf_ptrace_mourn_inferior, inf_ptrace_detach): Use
inf_child_maybe_unpush.
* inf-ttrace.c (inf_ttrace_him): Don't push the target if it is
already pushed.
(inf_ttrace_mourn_inferior): Only unpush the target after mourning
the inferior. Use inf_child_maybe_unpush_target.
(inf_ttrace_attach): Don't push the target if it is already
pushed.
(inf_ttrace_detach): Use inf_child_maybe_unpush_target.
* darwin-nat.c (darwin_mourn_inferior): Only unpush the target
after mourning the inferior. Use inf_child_maybe_unpush_target.
(darwin_attach_pid): Don't push the target if it is already
pushed.
* gnu-nat.c (gnu_mourn_inferior): Only unpush the target after
mourning the inferior. Use inf_child_maybe_unpush_target.
(gnu_detach): Use inf_child_maybe_unpush_target.
* go32-nat.c (go32_create_inferior): Don't push the target if it
is already pushed.
(go32_mourn_inferior): Use inf_child_maybe_unpush_target.
* nto-procfs.c (procfs_is_nto_target): Adjust comment.
(procfs_open): Rename to ...
(procfs_open_1): ... this. Add target_ops parameter. Adjust
comments. Can target_preopen before changing node. Call
inf_child_open_target to push the target explicitly.
(procfs_attach): Don't push the target if it is already pushed.
(procfs_detach): Use inf_child_maybe_unpush_target.
(procfs_create_inferior): Don't push the target if it is already
pushed.
(nto_native_ops): New global.
(procfs_open): Reimplement.
(procfs_native_open): New function.
(init_procfs_targets): Install procfs_native_open as to_open of
"target native". Store a pointer to the "native" target in
nto_native_ops.
* procfs.c (procfs_attach): Don't push the target if it is already
pushed.
(procfs_detach): Use inf_child_maybe_unpush_target.
(procfs_mourn_inferior): Only unpush the target after mourning the
inferior. Use inf_child_maybe_unpush_target.
(procfs_init_inferior): Don't push the target if it is already
pushed.
* windows-nat.c (do_initial_windows_stuff): Don't push the target
if it is already pushed.
(windows_detach): Use inf_child_maybe_unpush_target.
(windows_mourn_inferior): Only unpush the target after mourning
the inferior. Use inf_child_maybe_unpush_target.
gdb/doc/
2014-05-21 Pedro Alves <palves@redhat.com>
* gdb.texinfo (Starting): Document "set/show
auto-connect-native-target".
(Target Commands): Document "target native".
gdb/testsuite/
2014-05-21 Pedro Alves <palves@redhat.com>
* boards/gdbserver-base.exp (GDBFLAGS): Set to "set
auto-connect-native-target off".
* gdb.base/auto-connect-native-target.c: New file.
* gdb.base/auto-connect-native-target.exp: New file.
I have posted:
TLS variables access for -static -lpthread executables
https://sourceware.org/ml/libc-help/2014-03/msg00024.html
and the GDB patch below has been confirmed as OK for current glibcs.
Further work should be done for newer glibcs:
Improve TLS variables glibc compatibility
https://sourceware.org/bugzilla/show_bug.cgi?id=16954
Still the patch below implements the feature in a fully functional way backward
compatible with current glibcs, it depends on the following glibc source line:
csu/libc-tls.c
main_map->l_tls_modid = 1;
gdb/
2014-05-21 Jan Kratochvil <jan.kratochvil@redhat.com>
Fix TLS access for -static -pthread.
* linux-thread-db.c (struct thread_db_info): Add td_thr_tlsbase_p.
(try_thread_db_load_1): Initialize it.
(thread_db_get_thread_local_address): Call it if LM is zero.
* target.c (target_translate_tls_address): Remove LM_ADDR zero check.
* target.h (struct target_ops) (to_get_thread_local_address): Add
load_module_addr comment.
gdb/gdbserver/
2014-05-21 Jan Kratochvil <jan.kratochvil@redhat.com>
Fix TLS access for -static -pthread.
* gdbserver/thread-db.c (struct thread_db): Add td_thr_tlsbase_p.
(thread_db_get_tls_address): Call it if LOAD_MODULE is zero.
(thread_db_load_search, try_thread_db_load_1): Initialize it.
gdb/testsuite/
2014-05-21 Jan Kratochvil <jan.kratochvil@redhat.com>
Fix TLS access for -static -pthread.
* gdb.threads/staticthreads.c <HAVE_TLS> (tlsvar): New.
<HAVE_TLS> (thread_function, main): Initialize it.
* gdb.threads/staticthreads.exp: Try gdb_compile_pthreads for $have_tls.
Add clean_restart.
<$have_tls != "">: Check TLSVAR.
Message-ID: <20140410115204.GB16411@host2.jankratochvil.net>
This does two things:
1. Adds a test.
Recently compare-sections got a new "-r" switch, but given no test
existed for compare-sections, the patch was allowed in with no
testsuite addition. This now adds a test for both compare-sections
and compare-sections -r.
2. Makes the compare-sections command work against all targets.
Currently, compare-sections only works with remote targets, and only
those that support the qCRC packet. The patch makes it so that if the
target doesn't support accelerating memory verification, then GDB
falls back to comparing memory itself. This is of course slower, but
it's better than nothing, IMO. While testing against extended-remote
GDBserver I noticed that we send the qCRC request to the target if
we're connected, but not yet running a program. That can't work of
course -- the patch fixes that. This all also goes in the direction
of bridging the local/remote parity gap.
I didn't decouple 1. from 2., because that would mean that the test
would need to handle the case of the target not supporting the
command.
Tested on x86_64 Fedora 17, native, remote GDBserver, and
extended-remote GDBserver. I also hack-disabled qCRC support to make
sure the fallback paths in remote.c work.
gdb/doc/
2014-05-20 Pedro Alves <palves@redhat.com>
* gdb.texinfo (Memory) <compare-sections>: Generalize comments to
not be remote specific. Add cross reference to the qCRC packet.
(Separate Debug Files): Update cross reference to the qCRC packet.
(General Query Packets) <qCRC packet>: Add anchor.
gdb/
2014-05-20 Pedro Alves <palves@redhat.com>
* NEWS: Mention that compare-sections now works with all targets.
* remote.c (PACKET_qCRC): New enum value.
(remote_verify_memory): Don't send qCRC if the target has no
execution. Use packet_support/packet_ok. If the target doesn't
support the qCRC packet, fallback to a deep memory copy.
(compare_sections_command): Say "target image" instead of "remote
executable".
(_initialize_remote): Add PACKET_qCRC to the list of config
packets that have no associated command. Extend comment.
* target.c (simple_verify_memory, default_verify_memory): New
function.
* target.h (struct target_ops) <to_verify_memory>: Default to
default_verify_memory.
(simple_verify_memory): New declaration.
* target-delegates.c: Regenerate.
gdb/testsuite/
2014-05-20 Pedro Alves <palves@redhat.com>
* gdb.base/compare-sections.c: New file.
* gdb.base/compare-sections.exp: New file.
As a follow-up to
[PATCH 7/8] Adjust read_value_memory to use to_xfer_partial
https://sourceware.org/ml/gdb-patches/2014-02/msg00384.html
this patch moves traceframe_available_memory down to the target side.
After this patch, the gdb core code is cleaner, and code on handling
unavailable memory is moved to remote/tfile/ctf targets.
In details, this patch moves traceframe_available_memory code from
memory_xfer_partial_1 to remote target only, so remote target still
uses traceframe_info mechanism to check unavailable memory, and use
remote_ops to read them from read-only sections. We don't use
traceframe_info mechanism for tfile and ctf target, because it is
fast to iterate all traceframes from trace file, so the summary
information got from traceframe_info is not necessary.
This patch also moves two functions to remote.c from target.c,
because they are only used in remote.c. I'll clean them up in another
patch.
gdb:
2014-03-22 Yao Qi <yao@codesourcery.com>
* ctf.c (ctf_xfer_partial): Check the return value of
exec_read_partial_read_only, if it is not TARGET_XFER_OK,
return TARGET_XFER_UNAVAILABLE.
* tracefile-tfile.c (tfile_xfer_partial): Likewise.
* target.c (target_read_live_memory): Move it to remote.c.
(memory_xfer_live_readonly_partial): Likewise.
(memory_xfer_partial_1): Move some code to remote_read_bytes.
* remote.c (target_read_live_memory): Moved from target.c.
(memory_xfer_live_readonly_partial): Likewise.
(remote_read_bytes): New, factored out from
memory_xfer_partial_1.
A patch in the target cleanup series caused a regression when using
record with target-async. Version 4 of the patch is here:
https://sourceware.org/ml/gdb-patches/2014-03/msg00159.html
The immediate problem is that record supplies to_can_async_p and
to_is_async_p methods, but does not supply a to_async method. So,
when target-async is set, record claims to support async -- but if the
underlying target does not support async, then the to_async method
call will end up in that method's default implementation, namely
tcomplain.
This worked previously because the record target used to provide a
to_async method; one that (erroneously, only at push time) checked the
other members of the target stack, and then simply dropped to_async
calls in the "does not implement async" case.
My first thought was to simply drop tcomplain as the default for
to_async. This works, but Pedro pointed out that the only reason
record has to supply to_can_async_p and to_is_async_p is that these
default to using the find_default_run_target machinery -- and these
defaults are only needed by "run" and "attach".
So, a nicer solution presents itself: change run and attach to
explicitly call into the default run target when needed; and change
to_is_async_p and to_can_async_p to default to "return 0". This makes
the target stack simpler to use and lets us remove the method
implementations from record. This is also in harmony with other plans
for the target stack; namely trying to reduce the impact of
find_default_run_target. This approach makes it clear that
find_default_is_async_p is not needed -- it is asking whether a target
that may not even be pushed is actually async, which seems like a
nonsensical question.
While an improvement, this approach proved to introduce the same bug
when using the core target. Looking a bit deeper, the issue is that
code in "attach" and "run" may need to use either the current target
stack or the default run target -- but different calls into the target
API in those functions could wind up querying different targets.
This new patch makes the target to use more explicit in "run" and
"attach". Then these commands explicitly make the needed calls
against that target. This ensures that a single target is used for
all relevant operations. This lets us remove a couple find_default_*
functions from various targets, including the dummy target. I think
this is a decent understandability improvement.
One issue I see with this patch is that the new calls in "run" and
"attach" are not very much like the rest of the target API. I think
fundamentally this is due to bad factoring in the target API, which
may need to be fixed for multi-target. Tackling that seemed ambitious
for a regression fix.
While working on this I noticed that there don't seem to be any test
cases that involve both target-async and record, so this patch changes
break-precsave.exp to add some. It also changes corefile.exp to add
some target-async tests; these pass with current trunk and with this
patch applied, but fail with the v1 patch.
This patch differs from v4 in that it moves initialization of
to_can_async_p and to_supports_non_stop into inf-child, adds some
assertions to complete_target_initialization, and adds some comments
to target.h.
Built and regtested on x86-64 Fedora 20.
2014-03-12 Tom Tromey <tromey@redhat.com>
* inf-child.c (return_zero): New function.
(inf_child_target): Set to_can_async_p, to_supports_non_stop.
* aix-thread.c (aix_thread_inferior_created): New function.
(aix_thread_attach): Remove.
(init_aix_thread_ops): Don't set to_attach.
(_initialize_aix_thread): Register inferior_created observer.
* corelow.c (init_core_ops): Don't set to_attach or
to_create_inferior.
* exec.c (init_exec_ops): Don't set to_attach or
to_create_inferior.
* infcmd.c (run_command_1): Use find_run_target. Make direct
target calls.
(attach_command): Use find_attach_target. Make direct target
calls.
* record-btrace.c (init_record_btrace_ops): Don't set
to_create_inferior.
* record-full.c (record_full_can_async_p, record_full_is_async_p):
Remove.
(init_record_full_ops, init_record_full_core_ops): Update. Don't
set to_create_inferior.
* target.c (complete_target_initialization): Add assertion.
(target_create_inferior): Remove.
(find_default_attach, find_default_create_inferior): Remove.
(find_attach_target, find_run_target): New functions.
(find_default_is_async_p, find_default_can_async_p)
(target_supports_non_stop, target_attach): Remove.
(init_dummy_target): Don't set to_create_inferior or
to_supports_non_stop.
* target.h (struct target_ops) <to_attach>: Add comment. Remove
TARGET_DEFAULT_FUNC.
<to_create_inferior>: Add comment.
<to_can_async_p, to_is_async_p, to_supports_non_stop>: Use
TARGET_DEFAULT_RETURN.
<to_can_async_p, to_supports_non_stop, to_can_run>: Add comments.
(find_attach_target, find_run_target): Declare.
(target_create_inferior): Remove.
(target_has_execution_1): Update comment.
(target_supports_non_stop): Remove.
* target-delegates.c: Rebuild.
2014-03-12 Tom Tromey <tromey@redhat.com>
* gdb.base/corefile.exp (corefile_test_run, corefile_test_attach):
New procs. Add target-async tests.
* gdb.reverse/break-precsave.exp (precsave_tests): New proc.
Add target-async tests.
In non-stop mode, or rather, breakpoints always-inserted mode, the
code cache can easily end up with stale breakpoint instructions:
All it takes is filling a cache line when breakpoints already exist in
that memory region, and then delete the breakpoint.
Vis. (from the new test):
(gdb) set breakpoint always-inserted on
(gdb) b 23
Breakpoint 2 at 0x400540: file ../../../src/gdb/testsuite/gdb.base/breakpoint-shadow.c, line 23.
(gdb) b 24
Breakpoint 3 at 0x400547: file ../../../src/gdb/testsuite/gdb.base/breakpoint-shadow.c, line 24.
disass main
Dump of assembler code for function main:
0x000000000040053c <+0>: push %rbp
0x000000000040053d <+1>: mov %rsp,%rbp
=> 0x0000000000400540 <+4>: movl $0x1,-0x4(%rbp)
0x0000000000400547 <+11>: movl $0x2,-0x4(%rbp)
0x000000000040054e <+18>: mov $0x0,%eax
0x0000000000400553 <+23>: pop %rbp
0x0000000000400554 <+24>: retq
End of assembler dump.
So far so good. Now flush the code cache:
(gdb) set code-cache off
(gdb) set code-cache on
Requesting a disassembly works as expected, breakpoint shadowing is
applied:
(gdb) disass main
Dump of assembler code for function main:
0x000000000040053c <+0>: push %rbp
0x000000000040053d <+1>: mov %rsp,%rbp
=> 0x0000000000400540 <+4>: movl $0x1,-0x4(%rbp)
0x0000000000400547 <+11>: movl $0x2,-0x4(%rbp)
0x000000000040054e <+18>: mov $0x0,%eax
0x0000000000400553 <+23>: pop %rbp
0x0000000000400554 <+24>: retq
End of assembler dump.
However, now delete the breakpoints:
(gdb) delete
Delete all breakpoints? (y or n) y
And disassembly shows the old breakpoint instructions:
(gdb) disass main
Dump of assembler code for function main:
0x000000000040053c <+0>: push %rbp
0x000000000040053d <+1>: mov %rsp,%rbp
=> 0x0000000000400540 <+4>: int3
0x0000000000400541 <+5>: rex.RB cld
0x0000000000400543 <+7>: add %eax,(%rax)
0x0000000000400545 <+9>: add %al,(%rax)
0x0000000000400547 <+11>: int3
0x0000000000400548 <+12>: rex.RB cld
0x000000000040054a <+14>: add (%rax),%al
0x000000000040054c <+16>: add %al,(%rax)
0x000000000040054e <+18>: mov $0x0,%eax
0x0000000000400553 <+23>: pop %rbp
0x0000000000400554 <+24>: retq
End of assembler dump.
Those breakpoint instructions are no longer installed in target memory
they're stale in the code cache. Easily confirmed by just disabling
the code cache:
(gdb) set code-cache off
(gdb) disass main
Dump of assembler code for function main:
0x000000000040053c <+0>: push %rbp
0x000000000040053d <+1>: mov %rsp,%rbp
=> 0x0000000000400540 <+4>: movl $0x1,-0x4(%rbp)
0x0000000000400547 <+11>: movl $0x2,-0x4(%rbp)
0x000000000040054e <+18>: mov $0x0,%eax
0x0000000000400553 <+23>: pop %rbp
0x0000000000400554 <+24>: retq
End of assembler dump.
I stumbled upon this when writing a patch to infrun.c, that made
handle_inferior_event & co fill in the cache before breakpoints were
removed from the target. Recall that wait_for_inferior flushes the
dcache for every event. So in that case, always-inserted mode was not
necessary to trigger this. It's just a convenient way to expose the
issue.
The dcache works at the raw memory level. We need to update it
whenever memory is written, no matter what kind of target memory
object was originally passed down by the caller. The issue is that
the dcache update code isn't reached when a caller explicitly writes
raw memory. Breakpoint insertion/removal is one such case --
mem-break.c uses target_write_read_memory/target_write_raw_memory.
The fix is to move the dcache update code from memory_xfer_partial_1
to raw_memory_xfer_partial so that it's always reachable.
When we do that, we can actually simplify a series of things.
memory_xfer_partial_1 no longer needs to handle writes for any kind of
memory object, and therefore dcache_xfer_memory no longer needs to
handle writes either. So the latter (dcache_xfer_memory) and its
callees can be simplified to only care about reads. While we're
touching dcache_xfer_memory's prototype, might as well rename it to
reflect that fact that it only handles reads, and make it follow the
new target_xfer_status/xfered_len style. This made me notice that
dcache_xfer_memory loses the real error status if a memory read fails:
we could have failed to read due to TARGET_XFER_E_UNAVAILABLE, for
instance, but we always return TARGET_XFER_E_IO, hence the FIXME note.
I felt that fixing that fell out of the scope of this patch.
Currently dcache_xfer_memory handles the case of a write failing. The
whole cache line is invalidated when that happens. However,
dcache_update, the sole mechanism for handling writes that will remain
after the patch, does not presently handle that scenario. That's a
bug. The patch makes it handle that, by passing down the
target_xfer_status status from the caller, so that it can better
decide what to do itself. While I was changing the function's
prototype, I constified the myaddr parameter, getting rid of the need
for the cast as seen in its existing caller.
Tested on x86_64 Fedora 17, native and gdbserver.
gdb/
2014-03-05 Pedro Alves <palves@redhat.com>
PR gdb/16575
* dcache.c (dcache_poke_byte): Constify ptr parameter. Return
void. Update comment.
(dcache_xfer_memory): Delete.
(dcache_read_memory_partial): New, based on the read bits of
dcache_xfer_memory.
(dcache_update): Add status parameter. Use ULONGEST for len, and
adjust. Discard cache lines if the reason for the update was
error.
* dcache.h (dcache_xfer_memory): Delete declaration.
(dcache_read_memory_partial): New declaration.
(dcache_update): Update prototype.
* target.c (raw_memory_xfer_partial): Update the dcache here.
(memory_xfer_partial_1): Don't handle dcache writes here.
gdb/testsuite/
2014-03-05 Pedro Alves <palves@redhat.com>
PR gdb/16575
* gdb.base/breakpoint-shadow.exp (compare_disassembly): New
procedure.
(top level): Adjust to use it. Add tests that exercise breakpoint
interaction with the code-cache.
As no target uses it anymore, it can finally go away.
After removing the deprecated_xfer_memory handling from
default_xfer_partial, we can delete the latter, because the only thing
it does is delegate to the target beneath unconditionally, which is
what the delegator installed by target-delegates.c will do for us if
no to_xfer_partial method is installed.
This was the last user of de_fault, so that goes away too.
Tested on x86_64 Fedora 17.
gdb/
2014-02-26 Pedro Alves <palves@redhat.com>
* target.c (complete_target_initialization): Don't install
default_xfer_partial as to_xfer_partial hook.
(nomemory): Delete.
(update_current_target): Don't INHERIT nor de_fault
deprecated_xfer_memory. Delete de_fault macro.
(default_xfer_partial, deprecated_debug_xfer_memory): Delete.
(setup_target_debug): Don't install a deprecated_xfer_memory hook.
* target.h (struct target_ops) <deprecated_xfer_memory>: Delete
field.
This removes target_ignore, which isn't used any more.
2014-02-25 Tom Tromey <tromey@redhat.com>
* target.h (target_ignore): Don't declare.
* target.c (target_ignore): Remove.
Nowadays, TARGET_XFER_E_UNAVAILABLE isn't regarded as an error in
to_xfer_partial interface, so _E_ looks odd. This patch is to
replace TARGET_XFER_E_UNAVAILABLE with TARGET_XFER_UNAVAILABLE,
and change its value from -2 to 2. Since there is no comparison
on the value of 'enum target_xfer_status', so it should be safe.
gdb:
2014-02-24 Yao Qi <yao@codesourcery.com>
* target.h (enum target_xfer_status)
<TARGET_XFER_E_UNAVAILABLE>: Rename it to ...
<TARGET_XFER_UNAVAILABLE>: ... it with setting value 2
explicitly. New.
* corefile.c (memory_error_message): User updated.
* exec.c (section_table_read_available_memory): Likewise.
* record-btrace.c (record_btrace_xfer_partial): Likewise.
* target.c (target_xfer_status_to_string): Likewise.
(raw_memory_xfer_partial): Likewise.
(memory_xfer_partial_1, target_xfer_partial): Likewise.
* valops.c (read_value_memory): Likewise.
* exec.h: Update comments.
This patch tweaks target_xfer_status_to_string on comments and argument
name.
gdb:
2014-02-24 Yao Qi <yao@codesourcery.com>
* target.c (target_xfer_status_to_string): Rename argument err
to status.
* target.h (target_xfer_status_to_string): Update declaration.
Replace target_xfer_error_to_string with
target_xfer_status_to_string in comment.
This patch removes macro TARGET_XFER_STATUS_ERROR_P, as Pedro pointed
out during patches review that TARGET_XFER_STATUS_ERROR_P tends to
be unnecessary.
gdb:
2014-02-24 Yao Qi <yao@codesourcery.com>
* target.h (TARGET_XFER_STATUS_ERROR_P): Remove.
* corefile.c (read_memory): Adjusted.
* target.c (target_write_with_progress): Adjusted.
The patch "return target_xfer_status in to_xfer_partial" caused a
regression in various s390(x) test cases, because memory_xfer_partial
filled only the first byte of the read buffer from a breakpoint shadow:
https://sourceware.org/ml/gdb-patches/2014-01/msg01071.html
This patch fixes the regression.
This converts to_get_unwinder and to_get_tailcall_unwinder to methods
and arranges for them to use the new delegation scheme.
This just lets us avoid having a differing style (neither new-style
nor INHERIT) of delegation in the tree.
2014-02-19 Tom Tromey <tromey@redhat.com>
* target.c (target_get_unwinder): Rewrite.
(target_get_tailcall_unwinder): Rewrite.
* record-btrace.c (record_btrace_to_get_unwinder): New function.
(record_btrace_to_get_tailcall_unwinder): New function.
(init_record_btrace_ops): Update.
* target.h (struct target_ops) <to_get_unwinder,
to_get_tailcall_unwinder>: Now function pointers. Use
TARGET_DEFAULT_RETURN.
This converts to_decr_pc_after_break to the new style of delegation,
removing forward_target_decr_pc_after_break.
2014-02-19 Tom Tromey <tromey@redhat.com>
* record-btrace.c (record_btrace_decr_pc_after_break): Delegate
directly.
* target-delegates.c: Rebuild.
* target.h (struct target_ops) <to_decr_pc_after_break>: Use
TARGET_DEFAULT_FUNC.
* target.c (default_target_decr_pc_after_break): Rename from
forward_target_decr_pc_after_break. Simplify.
(target_decr_pc_after_break): Rely on delegation.
This removes a few unnecessary calls to INHERIT and de_fault:
* to_doc is only used when a target is registered
* to_magic is only used when a target is pushed and not useful for
current_target.
* to_open and to_close are only ever called using a specific
target_ops object; there is no need to de_fault them.
2014-02-19 Tom Tromey <tromey@redhat.com>
* target.c (update_current_target): Do not INHERIT to_doc or
to_magic. Do not de_fault to_open or to_close.
This cleans up target.c to avoid function casts.
2014-02-19 Tom Tromey <tromey@redhat.com>
* target.c (complete_target_initialization): Remove casts. Use
return_zero_has_execution.
(return_zero): Add "ignore" argument.
(return_zero_has_execution): New function.
(init_dummy_target): Remove casts. Use
return_zero_has_execution.
During the conversion I kept all the "do not inherit" comments in
update_current_target. However, now they are not needed. This patch
updates the comments for INHERIT and de_fault, and removes the
somewhat odd INHERIT of to_stratum.
2014-02-19 Tom Tromey <tromey@redhat.com>
* target.c (update_current_target): Update comments. Do not
INHERIT to_stratum.
This switches to_read_description to the "new normal" delegation
scheme. This one was a bit trickier than the other changes due to the
way that target_read_description handled delegation. I examined all
the target implementations of to_read_description and changed the ones
returning NULL to instead delegate.
2014-02-19 Tom Tromey <tromey@redhat.com>
* arm-linux-nat.c (arm_linux_read_description): Delegate when
needed.
* corelow.c (core_read_description): Delegate when needed.
* remote.c (remote_read_description): Delegate when needed.
* target-delegates.c: Rebuild.
* target.c (target_read_description): Rewrite.
* target.h (struct target_ops) <to_read_description>: Update
comment. Use TARGET_DEFAULT_RETURN.
Note that this patch reformats the
to_static_tracepoint_markers_by_strid field declaration in struct
target_ops. This was needed because make-target-delegates requires
the opening paren for the parameters to be on the same line as the
method name, and I didn't see an easy way to fix this.
2014-02-19 Tom Tromey <tromey@redhat.com>
* target-delegates.c: Rebuild.
* target.c (update_current_target): Don't inherit or default
to_static_tracepoint_markers_by_strid.
* target.h (struct target_ops)
<to_static_tracepoint_markers_by_strid>: Use
TARGET_DEFAULT_NORETURN.
Pedro Alves