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 is the continuation of what Joel proposed on:
<https://sourceware.org/ml/gdb-patches/2013-12/msg00977.html>
Now that I have already submitted and pushed the patch to split
i386_stap_parse_special_token into two smaller functions, it is indeed
simpler to understand this patch.
It occurs because, on x86, triplet displacement operands are allowed
(like "-4+8-20(%rbp)"), and the current parser for this expression is
buggy. It does not correctly extract the register name from the
expression, which leads to incorrect evaluation. The parser was also
being very "generous" with the expression, so I included a few more
checks to ensure that we're indeed dealing with a triplet displacement
operand.
This patch also includes testcases for the two different kind of
expressions that can be encountered on x86: the triplet displacement
(explained above) and the three-argument displacement (as in
"(%rbx,%ebx,-8)"). The tests are obviously arch-dependent and are
placed under gdb.arch/.
Message-ID: <m3mwj1j12v.fsf@redhat.com>
URL: <https://sourceware.org/ml/gdb-patches/2014-01/msg00310.html>
gdb/
2014-02-20 Sergio Durigan Junior <sergiodj@redhat.com>
PR tdep/16397
* i386-tdep.c (i386_stap_parse_special_token_triplet): Check if a
number comes after the + or - signs. Adjust length of register
name to be extracted.
gdb/testsuite/
2014-02-20 Sergio Durigan Junior <sergiodj@redhat.com>
PR tdep/16397
* gdb.arch/amd64-stap-special-operands.exp: New file.
* gdb.arch/amd64-stap-three-arg-disp.S: Likewise.
* gdb.arch/amd64-stap-three-arg-disp.c: Likewise.
* gdb.arch/amd64-stap-triplet.S: Likewise.
* gdb.arch/amd64-stap-triplet.c: Likewise.
The AIX linker pointed out that gdb had multiple definitions of the
various *_varobj_ops objects. This patch fixes the problem by marking
the declarations as "extern" in varobj.h. Tested by rebuilding on
x86-64 Fedora 18 and on AIX.
2014-02-20 Tom Tromey <tromey@redhat.com>
* varobj.h (c_varobj_ops, cplus_varobj_ops, java_varobj_ops)
(ada_varobj_ops): Mark "extern".
I happened to notice that last_o_file_start is write-only in
read_dbx_symtab. This patch removes it. Tested by rebuilding.
2014-02-20 Tom Tromey <tromey@redhat.com>
* dbxread.c (read_dbx_symtab): Remove last_o_file_start.
* dwarf2read.c (struct die_info): New member in_process.
(reset_die_in_process): New function.
(process_die): Set it at the start, reset when returning.
(inherit_abstract_dies): Only call process_die if origin_child_die
not already being processed.
testsuite/
* gdb.dwarf2/dw2-icycle.S: New file.
* gdb.dwarf2/dw2-icycle.c: New file.
* gdb.dwarf2/dw2-icycle.exp: New file.
This patch brings back a comment that got stripped down a bit too much
during a recent change.
gdb/ChangeLog:
* windows-nat.c (handle_unload_dll): Add function documentation.
(do_initial_windows_stuff): Add comment explaining why we wait
until after inferior initialization has finished before
processing all DLLs.
Now that get_module_name is no longer called for handling DLL events,
we can simplify it a bit, knowing that the only use is to get the
executable's filename.
While doing so, we adjusted the implementation a bit to avoid
references to DLLs, renamed it to make its more-targeted usage
more explicit, moved it right before the only function that uses it.
We also remove the use of hard-coded length for the buffers being
used.
gdb/ChangeLog:
* windows-nat.c (get_module_name): Delete.
(windows_get_exec_module_filename): New function, mostly
inspired from get_module_name.
(windows_pid_to_exec_file): Replace call to get_module_name
by call to windows_get_exec_module_filename.
When a DLL gets loaded an the debugger gets a debug event about it,
the currently implementation in handle_load_dll currently tries to
fetch the DLL's name by first iterating over all DLLs known to
the system. It should be sufficient to rely on the name provided
with the event, however, especially in the situation we are now,
where we now know that we're past the statup phase for our inferior.
This patch therefore simplifies windows-nat.c::handle_load_dll to
only rely on the event's lpImageName.
It also updates the function's comment to document the assumption
regarding not being during the inferior's startup phase. And while
at it, it fixes the function documentation, which was probably
unintentionally inherited from another function (perhaps windows_wait).
gdb/ChangeLog:
* windows-nat.c (handle_load_dll): Rewrite this function's
introductory comment. Remove code using get_module_name
to get the DLL's name.
This patch aims at simplifying DLL handling during the inferior
initialization (process creation during the "run", or during an
"attach"). Instead of processing each DLL load event, which is
sometimes incomplete, we ignore these events until the inferior
has completed its startup phase, and then just iterate over all
DLLs via EnumProcessModules.
gdb/ChangeLog:
* windows-nat.c (get_windows_debug_event): Ignore
LOAD_DLL_DEBUG_EVENT and UNLOAD_DLL_DEBUG_EVENT
if windows_initialization_done == 0.
(windows_add_all_dlls): Renames windows_ensure_ntdll_loaded.
Adjust implementation to always load all DLLs.
(do_initial_windows_stuff): Replace call to
windows_ensure_ntdll_loaded by call to windows_add_all_dlls.
The "dll-symbols" command, specific to native Windows platforms,
gives the impression that the symbols were not loaded, first
because it completes silently, and second because the "info shared"
output does not get updated after the command completes:
(gdb) dll-symbols C:\WINDOWS\syswow64\rpcrt4.dll
(gdb) info shared
From To Syms Read Shared Object Library
[...]
0x77e51000 0x77ee2554 No C:\WINDOWS\system32\rpcrt4.dll
(we exected the "Syms Read" column to read "Yes").
As far as I can tell, the symbols actually do get loaded, but completely
independently from the solib framework, which explains the silent
loading and the fact that the "Syms Read" column does not get updated.
See windows-nat.c::safe_symbol_file_add_stub, which calls symbol_file_add
instead of calling solib_add.
But, aside from the fact that the "Syms Read" status does not get
updated, I also noticed that it does not take into account the DLL's
actual load address when loading its symbols. As a result, I believe
that we get it wrong if the DLL does not get loaded at the prefered
address.
Rather than trying to fix this command, there does not seem to be
a reason other than historical for having Windows-specific commands
which essentially re-implements the "sharedlibrary" command. The
command interface is slightly different (the latter takes a regexp
rather than a plain filename), but it should be just as easy to use
the "sharedlibrary" command, or its "share" alias, as usisng the
"dll-symbols" command. For instance:
(gdb) share rpcrt4.dll
Reading symbols from C:\WINDOWS\system32\rpcrt4.dll...(no debugging symbols found)...done.
Loaded symbols for C:\WINDOWS\system32\rpcrt4.dll
(gdb) info shared
From To Syms Read Shared Object Library
[...]
0x77e51000 0x77ee2554 Yes (*) C:\WINDOWS\system32\rpcrt4.dll
This patch therefore deprecates the "dll-symbols" command, as well
as its two aliases "add-shared-symbol-files" and "assf", with a view
of deleting them as soon as the 7.8 branch gets cut.
gdb/ChangeLog:
* windows-nat.c (_initialize_windows_nat): Deprecate the
"dll-symbols" command. Turn the "add-shared-symbol-files"
and "assf" aliases into commands, and deprecate them as well.
* NEWS: Add entry explaining that "dll-symbols" and its two
aliases are now deprecated.
gdb/doc/ChangeLog:
* gdb.texinfo (Files): Document "add-shared-symbol-files"
and "assf" as being deprecated.
(Cygwin Native): Likewise for "dll-symbols".
(Non-debug DLL Symbols): Remove reference to "dll-symbols"
as a way to force the loading of symbols from a DLL.
This patch fixes the following ARI warning:
gdb/dec-thread.c:695: regression: multi-line string: Multi-line string
must have the newline escaped
I think the new-line was unintentional, so I simply removed it,
and then reformatted the string to fit within our 70-80 max characters-
per-line rule.
gdb/ChangeLog:
* dec-thread.c (dec_thread_get_ada_task_ptid): Avoid unescaped
new-line in debug string. Remove trailing spaces.
* NEWS: Add entry for the new feature
* python/py-value.c (valpy_binop): Call value_x_binop for struct
and class values.
testsuite/
* gdb.python/py-value-cc.cc: Improve test case to enable testing
operations on gdb.Value objects.
* gdb.python/py-value-cc.exp: Add new test to test operations on
gdb.Value objects.
doc/
* python.texi (Values From Inferior): Add description about the
new feature.
It's best that we standardize on process_stratum targets using the
ptid.lwp field to store thread ids. The idea being leave the ptid.tid
field free for any thread_stratum target that might want to sit on
top. This patch adds a comment in that direction to struct ptid's
definition.
gdb/
2014-02-19 Pedro Alves <palves@redhat.com>
* common/ptid.h (struct ptid): Mention that process_stratum
targets should prefer ptid.lwp.
From GDB's perspective, independently of how the target really
implements threads, gdb/remote sees all threads as if kernel/system
threads. A rationale along theses lines led to gdbserver storing
thread ids in ptid.lwp in all ports.
Because remote.c is currently using ptid.tid, we can't make gdbserver
and gdb share bits of remote-specific code that manipulates ptids
(e.g., write_ptid/read_ptid).
This patch thus makes remote.c use ptid.lwp instead of ptid.tid.
I believe that on the GDB side too, it's best that we standardize on
process_stratum targets using the ptid.lwp field to store thread ids
anyway. The idea being leave the ptid.tid field free for any
thread_stratum target that might want to sit on top.
Tested on x86_64 Fedora 17, w/ local gdbserver.
gdb/
2014-02-19 Pedro Alves <palves@redhat.com>
* remote.c (remote_thread_alive, write_ptid, read_ptid)
(read_ptid, remote_newthread_step, remote_threads_extra_info)
(remote_get_ada_task_ptid, append_resumption, remote_stop_ns)
(threadalive_test, remote_pid_to_str): Use the ptid.lwp field to
store remote thread ids rather than ptid.tid.
(_initialize_remote): Adjust.
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.
I happened to notice that nto-procfs.c defines
procfs_remove_hw_breakpoint but never uses it. This caused it not to
be updated by my target-method-updating script. This patch fixes the
function and installs it properly. I have no way to test this,
however.
2014-02-19 Tom Tromey <tromey@redhat.com>
* nto-procfs.c (procfs_remove_hw_breakpoint): Add 'self'
argument.
(init_procfs_ops): Correctly set to_remove_hw_breakpoint.
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.
exec_set_find_memory_regions is used to modify the exec target.
However, it only has a single caller, and so it is much clearer to
simply set the appropriate field directly. It's also better for the
coming multi-target world to avoid this kind of global state change
anyway.
2014-02-19 Tom Tromey <tromey@redhat.com>
* gcore.h (objfile_find_memory_regions): Declare.
* gcore.c (objfile_find_memory_regions): No longer static. Add
"self" argument.
(_initialize_gcore): Don't call exec_set_find_memory_regions.
* exec.c: Include gcore.h.
(exec_set_find_memory_regions): Remove.
(exec_find_memory_regions): Remove.
(exec_do_find_memory_regions): Remove.
(init_exec_ops): Update.
* defs.h (exec_set_find_memory_regions): Remove.
This changes instances of TARGET_DEFAULT_RETURN(0) to
TARGET_DEFAULT_RETURN(NULL) when appropriate. The use of "0" was a
relic from an earlier implementation of make-target-delegates; and I
didn't want to go back through the long patch series, fixing up
conflicts, just to change this small detail.
2014-02-19 Tom Tromey <tromey@redhat.com>
* target-delegates.c: Rebuild.
* target.h (struct target_ops) <to_extra_thread_info,
to_thread_name, to_pid_to_exec_file, to_get_section_table,
to_memory_map, to_read_description, to_traceframe_info>: Use NULL,
not 0, in TARGET_DEFAULT_RETURN.
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.
