Use a standard vector instead of the home-made version. I used a vector
of plain pointers, because the cli_ui_out_data object doesn't own the
streams objects (i.e. they shouldn't be deleted when the vector is
deleted).
gdb/ChangeLog:
* cli-out.h (cli_ui_out_data) <streams>: Change type to
std::vector.
* cli-out.c: Remove vec.h include.
(cli_uiout_dtor): Update.
(cli_field_fmt): Update.
(cli_spaces): Update.
(cli_text): Update.
(cli_message): Update.
(cli_flush): Update.
(cli_redirect): Update.
(out_field_fmt): Update.
(field_separator): Update.
(cli_out_data_ctor): Update.
(cli_out_new): Update.
(cli_out_set_stream): Update.
Use a standard vector instead of the home-made version. I used a vector
of plain pointers, because the mi_ui_out_data object doesn't own the
streams objects (i.e. they shouldn't be deleted when the vector is
deleted).
gdb/ChangeLog:
* mi/mi-out.c: Remove vec.h include.
(mi_ui_out_data) <streams>: Change type to std::vector.
(mi_field_string): Update.
(mi_field_fmt): Update.
(mi_flush): Update.
(mi_redirect): Update.
(field_separator): Update.
(mi_open): Update.
(mi_close): Update.
(mi_out_buffered): Update.
(mi_out_rewind): Update.
(mi_out_put): Update.
(mi_out_data_ctor): Update.
(mi_out_data_dtor): Don't free streams.
Convert the levels field of struct ui_out to be a vector of unique_ptr
to ui_out_level. This way, the ownership of the ui_out_level objects by
the ui_out instance is clear.
gdb/ChangeLog:
* ui-out.c (ui_out_level_p): Remove typedef.
(DEF_VEC_P (ui_out_level_p)): Remove definition.
(struct ui_out) <levels>: Change type to vector of unique_ptr of
ui_out_level.
(current_level): Update.
(push_level): Update.
(pop_level): Update, don't manually delete the ui_out_level
instance.
(ui_out_new): Update.
The following patches introduce C++ vectors and strings as fields of the
various ui_out structures. We therefore need to use new/delete so that
their contructor/destructor is called. I find it simpler to change all
the allocations in a separate preliminary patch, rather than in each
individual patch.
gdb/ChangeLog:
* cli-out.c (cli_uiout_dtor): Use delete instead of xfree.
(cli_out_new): Use new instead of XNEW.
* mi/mi-out.c (mi_out_data_dtor): Use delete instead of xfree.
(mi_out_new): Use new instead of XNEW.
* tui/tui-out.c (tui_out_new): Likewise.
* ui-out.c (push_level): Likewise.
(pop_level): Use delete instead of xfree.
(clear_header_list): Use delete instead of xfree.
(append_header_to_list): Use new instead of XNEW.
(ui_out_new): Likewise.
Since we don't use suffix rules nor implicit rules in gdb, we can
disable them. The advantage is a slightly faster make [1].
Here are some numbers about the speedup. I ran this on my trusty old
Intel Q6600, so the time numbers are probably higher than what you'd get
on any recent hardware. I ran "make" in the gdb/ directory of an
already built repository (configured with --enable-targets=all). I
recorded the time of execution (average of 5). I then ran "make -d" and
recorded the number of printed lines, which gives a rough idea of the
number of operations done.
I compared the following configurations, to see the impact of both the
empty .SUFFIXES target and the empty pattern rules, as well as running
"make -r", which can be considered the "ideal" case.
A - baseline
B - baseline + .SUFFIXES
C - baseline + pattern rules
D - baseline + .SUFFIXES + pattern rules
E - baseline + make -r
config | time (s) | "make -d"
-----------------------------
A | 5.74 | 2396643
B | 1.19 | 298469
C | 2.81 | 1266573
D | 1.13 | 245489
E | 1.01 | 163914
We can see that the empty .SUFFIXES target has a bigger impact than the
empty pattern rules, but still it doesn't hurt to disable the implicit
pattern rules as well.
There are still some mentions of implicit rules I can't get rid of in
the "make -d" output. For example, it's trying to build .c files from
.w files:
Looking for an implicit rule for '/home/simark/src/binutils-gdb/gdb/infrun.c'.
Trying pattern rule with stem 'infrun'.
Trying implicit prerequisite '/home/simark/src/binutils-gdb/gdb/infrun.w'.
and trying to build Makefile.in from a bunch of extensions:
Looking for an implicit rule for 'Makefile.in'.
Trying pattern rule with stem 'Makefile.in'.
Trying implicit prerequisite 'Makefile.in.o'.
Trying pattern rule with stem 'Makefile.in'.
Trying implicit prerequisite 'Makefile.in.c'.
Trying pattern rule with stem 'Makefile.in'.
Trying implicit prerequisite 'Makefile.in.cc'.
... many more ...
If somebody knows how to disable them, we can do it, but at this point
the returns are minimal, so it is not that important.
I verified that both in-tree and out-of-tree builds work.
[1] Switching from explicit rules to pattern rules for files in
subdirectories actually made it slower, so this is kind of a way to
redeem myself. But it the end it's faster than it was previously,
so it was all worth it. :)
gdb/ChangeLog:
* disable-implicit-rules.mk: New file.
* Makefile.in: Include disable-implicit-rules.mk.
* data-directory/Makefile.in: Likewise.
* gnulib/Makefile.in: Likewise.
gdb/doc/ChangeLog:
* Makefile.in: Likewise.
gdb/gdbserver/ChangeLog:
* Makefile.in: Include disable-implicit-rules.mk.
gdb/testsuite/ChangeLog:
* Makefile.in: Include disable-implicit-rules.mk.
When GDB read inferior memory as an address or an instruction,
it should be unsigned.
gdb:
2016-11-30 Yao Qi <yao.qi@linaro.org>
* arm-tdep.c (arm_scan_prologue): Read memory as unsigned integer.
(arm_exidx_unwind_sniffer): Likewise.
With the previous change, value.location.address is only valid for
lval_memory. This patch restrict some checking on value.lval on
using address. Since we have a check on VALUE_VAL in
set_value_address, we need to set VALUE_VAL properly before
set_value_address too.
gdb:
2016-11-25 Yao Qi <yao.qi@linaro.org>
* ada-lang.c (ensure_lval): Call set_value_address after setting
VALUE_LVAL.
* elfread.c (elf_gnu_ifunc_resolve_addr): Set VALUE_LVAL to
lval_memory.
(elf_gnu_ifunc_resolver_return_stop): Likewise.
* value.c (value_fn_field): Likewise.
(value_from_contents_and_address_unresolved): Likewise.
(value_from_contents_and_address): Likewise.
(value_address): Check value->lval isn't
lval_memory.
(value_raw_address): Likewise.
(set_value_address): Assert value->lval is lval_memory.
value.regnum and value.next_frame_id are only used for lval_register,
so this patch moves them to union value.location. As a result, when
we copy value, only copy location, don't need to copy regnum and
next_frame_id.
This patch also changes regnum's type to int as there is no space
constraint, so update deprecated_value_regnum_hack return type too.
gdb:
2016-11-28 Yao Qi <yao.qi@linaro.org>
* valops.c (value_slice): Don't set frame id of slice.
* value.c (struct value) <regnum, next_frame_id>: Move them to...
(struct value) <location>: ... here. Update comments.
(allocate_value_lazy): Don't set frame id and regnum.
(deprecated_value_next_frame_id_hack): Adjust.
(deprecated_value_regnum_hack): Adjust.
(value_copy): Don't copy frame id and regnu.
(value_primitive_field): Likewise.
(value_from_component): Likewise.
(deprecated_value_regnum_hack): Return int *.
* value.h (deprecated_value_regnum_hack): Update declaration.
Nowadays, we set computed value's frame id, which is a misuse to me.
The computed value itself doesn't care about frame id, but function
value_computed_funcs (val)->read (or read_pieced_value) cares about
which frame the register is relative to, so 'struct piece_closure' is
a better place to fit frame id.
This patch adds a frame id in 'struct piece_closure', and use it
instead of using computed value's frame id.
gdb:
2016-11-28 Yao Qi <yao.qi@linaro.org>
* dwarf2loc.c (struct piece_closure) <frame_id>: New field.
(allocate_piece_closure): Add new parameter 'frame' and set
closure's frame_id field accordingly.
(read_pieced_value): Get frame from closure instead of value.
(dwarf2_evaluate_loc_desc_full): Remove code getting frame id.
Don't set value's frame id.
Constify the data path between ui_out_wrap_hint and the wrap_indent
global, because we can. It's clearer that the argument passed to
wrap_hint is not intended to be modified by the ui_out implementation.
gdb/ChangeLog:
* mi/mi-out.c (mi_wrap_hint): Constify argument.
* cli-out.c (cli_wrap_hint): Likewise.
* ui-out.c (ui_out_wrap_hint, uo_wrap_hint): Likewise.
* ui-out.h (ui_out_wrap_hint, wrap_hint_ftype): Likewise.
* utils.c (wrap_here): Likewise.
(wrap_indent): Constify.
* utils.h (wrap_here): Constify argument.
The wrapper uo_redirect seems like it should return the return value
from of implementation function, since callers rely on it, but it
doesn't.
gdb/ChangeLog:
* ui-out.c (uo_redirect): Return the return value from the
implementation function.
It's not actually used, and removing it simplifies the upcoming patches
a bit. After the whole series, destroying an ui_out object will be
simply "delete uiout", which will call the default destructor.
gdb/ChangeLog:
* ui-out.c (ui_out_destroy, uo_data_destroy): Remove.
* ui-out.h (ui_out_destroy): Remove.
Just a little cleanup, so the name is more consistent with the naming of
the equivalent structures of cli and tui. It goes away in subsequent
patches anyway, but it might help follow the changes in those patches...
gdb/ChangeLog:
* mi/mi-out.c (ui_out_data): Rename to ...
(mi_ui_out_data): ... this.
Using std::move forces an extra copy of the object. These changes fix
-Wpessimizing-move warnings from clang.
gdb/ChangeLog:
* ada-lang.c (create_excep_cond_exprs): Do not use 'std::move'.
* ax-gdb.c (agent_eval_command_one): Likewise.
(agent_eval_command_one): Likewise.
* breakpoint.c (parse_cond_to_aexpr): Likewise.
(parse_cmd_to_aexpr): Likewise.
* dtrace-probe.c (dtrace_process_dof_probe): Likewise.
* parse.c (parse_expression_for_completion): Likewise.
Both libc++ and libstdc++ declare non-throwing new operators as
noexcept and overloads must also be noexcept. This fixes a
-Wmissing-exception-spec warning with clang.
gdb/ChangeLog:
* common/new-op.c (operator new): Mark 'noexcept'.
(operator new[]): Likewise.
The function copy_bitwise used for copying DWARF pieces can potentially
be invoked for large chunks of data. For instance, consider a large
struct one of whose members is currently located in a register. In this
case copy_bitwise would still copy the data bitwise in a loop, which is
much slower than necessary.
This change uses memcpy for the large part instead, if possible.
gdb/ChangeLog:
* dwarf2loc.c (copy_bitwise): Use memcpy for the middle part, if
it is byte-aligned.
This adds a unit test for the copy_bitwise function in dwarf2loc.c.
With the old (broken) version of copy_bitwise this test would generate
the following failure message:
(gdb) maintenance selftest
Self test failed: copy_bitwise 11000000 != 10000000 (7+2 -> 0)
gdb/ChangeLog:
2016-11-24 Andreas Arnez <arnez@linux.vnet.ibm.com>
Pedro Alves <palves@redhat.com>
* dwarf2loc.c (bits_to_str, check_copy_bitwise)
(copy_bitwise_tests): New functions.
(_initialize_dwarf2loc): Register the new function
copy_bitwise_tests as a unit test.
* selftest.c (run_self_tests): Improve the failure message's
wording and formatting.
When the user writes or reads a variable whose location is described
with DWARF pieces (DW_OP_piece or DW_OP_bit_piece), GDB's helper
function copy_bitwise is invoked for each piece. The implementation of
this function has a bug that may result in a corrupted copy, depending
on alignment and bit size. (Full-byte copies are not affected.)
This rewrites copy_bitwise, replacing its algorithm by a fixed version,
and adding an appropriate test case. Without the fix the new test case
fails, e.g.:
print def_t
$2 = {a = 0, b = 4177919}
(gdb) FAIL: gdb.dwarf2/nonvar-access.exp: print def_t
Written in binary, the wrong result above looks like this:
01111111011111111111111
Which means that two zero bits have sneaked into the copy of the
original all-one bit pattern. The test uses this simple all-one value
in order to avoid another GDB bug that causes the DWARF piece of a
DW_OP_stack_value to be taken from the wrong end on big-endian
architectures.
gdb/ChangeLog:
* dwarf2loc.c (extract_bits_primitive): Remove.
(extract_bits): Remove.
(copy_bitwise): Rewrite. Fixes a possible corruption that may
occur for non-byte-aligned copies.
gdb/testsuite/ChangeLog:
* gdb.dwarf2/nonvar-access.exp: Add a test for accessing
non-byte-aligned bit fields.
The DW_AT_data_bit_offset attribute was introduced by DWARF V4 and
allows specifying the offset of a data member within its containing
entity. But although the new attribute was intended to replace
DW_AT_bit_offset for this purpose, GDB ignores it, and thus GCC still
emits DW_AT_bit_offset instead. See also
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71669.
This change fixes GDB's lack of support for DW_AT_data_bit_offset and
adds an appropriate test case.
gdb/ChangeLog:
PR gdb/12616
* dwarf2read.c (dwarf2_add_field): Handle the DWARF V4 attribute
DW_AT_data_bit_offset.
gdb/testsuite/ChangeLog:
PR gdb/12616
* gdb.dwarf2/nonvar-access.exp: New testcase. Check that GDB
respects the DW_AT_data_bit_offset attribute.
This patch fixes a few problems with GDB's time handling.
#1 - It avoids problems with gnulib's C++ namespace support
On MinGW, the struct timeval that should be passed to gnulib's
gettimeofday replacement is incompatible with libiberty's
timeval_sub/timeval_add. That's because gnulib also replaces "struct
timeval" with its own definition, while libiberty expects the
system's.
E.g., in code like this:
gettimeofday (&prompt_ended, NULL);
timeval_sub (&prompt_delta, &prompt_ended, &prompt_started);
timeval_add (&prompt_for_continue_wait_time,
&prompt_for_continue_wait_time, &prompt_delta);
That's currently handled in gdb by not using gnulib's gettimeofday at
all (see common/gdb_sys_time.h), but that #undef hack won't work with
if/when we enable gnulib's C++ namespace support, because that mode
adds compile time warnings for uses of ::gettimeofday, which are hard
errors with -Werror.
#2 - But there's an elephant in the room: gettimeofday is not monotonic...
We're using it to:
a) check how long functions take, for performance analysis
b) compute when in the future to fire events in the event-loop
c) print debug timestamps
But that's exactly what gettimeofday is NOT meant for. Straight from
the man page:
~~~
The time returned by gettimeofday() is affected by
discontinuous jumps in the system time (e.g., if the system
administrator manually changes the system time). If you need a
monotonically increasing clock, see clock_gettime(2).
~~~
std::chrono (part of the C++11 standard library) has a monotonic clock
exactly for such purposes (std::chrono::steady_clock). This commit
switches to use that instead of gettimeofday, fixing all the issues
mentioned above.
gdb/ChangeLog:
2016-11-23 Pedro Alves <palves@redhat.com>
* Makefile.in (SFILES): Add common/run-time-clock.c.
(HFILES_NO_SRCDIR): Add common/run-time-clock.h.
(COMMON_OBS): Add run-time-clock.o.
* common/run-time-clock.c, common/run-time-clock.h: New files.
* defs.h (struct timeval, print_transfer_performance): Delete
declarations.
* event-loop.c (struct gdb_timer) <when>: Now a
std::chrono::steady_clock::time_point.
(create_timer): use std::chrono::steady_clock instead of
gettimeofday. Use new instead of malloc.
(delete_timer): Use delete instead of xfree.
(duration_cast_timeval): New.
(update_wait_timeout): Use std::chrono::steady_clock instead of
gettimeofday.
* maint.c: Include <chrono> instead of "gdb_sys_time.h", <time.h>
and "timeval-utils.h".
(scoped_command_stats::~scoped_command_stats)
(scoped_command_stats::scoped_command_stats): Use
std::chrono::steady_clock instead of gettimeofday. Use
user_cpu_time_clock instead of get_run_time.
* maint.h: Include "run-time-clock.h" and <chrono>.
(scoped_command_stats): <m_start_cpu_time>: Now a
user_cpu_time_clock::time_point.
<m_start_wall_time>: Now a std::chrono::steady_clock::time_point.
* mi/mi-main.c: Include "run-time-clock.h" and <chrono> instead of
"gdb_sys_time.h" and <sys/resource.h>.
(rusage): Delete.
(mi_execute_command): Use new instead of XNEW.
(mi_load_progress): Use std::chrono::steady_clock instead of
gettimeofday.
(timestamp): Rewrite in terms of std::chrono::steady_clock,
user_cpu_time_clock and system_cpu_time_clock.
(timeval_diff): Delete.
(print_diff): Adjust to use std::chrono::steady_clock,
user_cpu_time_clock and system_cpu_time_clock.
* mi/mi-parse.h: Include "run-time-clock.h" and <chrono> instead
of "gdb_sys_time.h".
(struct mi_timestamp): Change fields types to
std::chrono::steady_clock::time_point, user_cpu_time_clock::time
and system_cpu_time_clock::time_point, instead of struct timeval.
* symfile.c: Include <chrono> instead of <time.h> and
"gdb_sys_time.h".
(struct time_range): New.
(generic_load): Use std::chrono::steady_clock instead of
gettimeofday.
(print_transfer_performance): Replace timeval parameters with a
std::chrono::steady_clock::duration parameter. Adjust.
* utils.c: Include <chrono> instead of "timeval-utils.h",
"gdb_sys_time.h", and <time.h>.
(prompt_for_continue_wait_time): Now a
std::chrono::steady_clock::duration.
(defaulted_query, prompt_for_continue): Use
std::chrono::steady_clock instead of
gettimeofday/timeval_sub/timeval_add.
(reset_prompt_for_continue_wait_time): Use
std::chrono::steady_clock::duration instead of struct timeval.
(get_prompt_for_continue_wait_time): Return a
std::chrono::steady_clock::duration instead of struct timeval.
(vfprintf_unfiltered): Use std::chrono::steady_clock instead of
gettimeofday. Use std::string. Use '.' instead of ':'.
* utils.h: Include <chrono>.
(get_prompt_for_continue_wait_time): Return a
std::chrono::steady_clock::duration instead of struct timeval.
gdb/gdbserver/ChangeLog:
2016-11-23 Pedro Alves <palves@redhat.com>
* debug.c: Include <chrono> instead of "gdb_sys_time.h".
(debug_vprintf): Use std::chrono::steady_clock instead of
gettimeofday. Use '.' instead of ':'.
* tracepoint.c: Include <chrono> instead of "gdb_sys_time.h".
(get_timestamp): Use std::chrono::steady_clock instead of
gettimeofday.
Mostly some whitespace changes to make things a bit more consistent.
gdb/ChangeLog:
* Makefile.in: Fix whitespace formatting.
gdb/gdbserver/ChangeLog:
* Makefile.in: Fix whitespace formatting.
I find the big file lists in the Makefiles a bit ugly and not very
practical. Since there are multiple filenames on each line (as much as
fits in 80 columns), it's not easy to add, remove or change a name in
the middle. As a result, we have a mix of long and short lines in no
particular order (ALL_TARGET_OBS is a good example).
I therefore suggest flattening the lists (one name per line) and keeping
them in alphabetical order. The diffs will be much clearer and merge
conflicts will be easier to resolve.
A nice (IMO) side-effect I observed is that the files are compiled
alphabetically by make, so it gives a rough idea of the progress of the
build.
I added a comment in gdb/Makefile.in to mention to keep the file lists
ordered, and gave the general guidelines on what order to respect. I
added a comment in other Makefiles which refers to gdb/Makefile.in, to
avoid duplication.
Running the patch through the buildbot found that gdb.base/default.exp
started to fail. The languages in the error message shown when typing
"set language" have changed order. We could probably improve gdb so
that it prints them in a stable order, regardless of the order of the
object list passed to the linked, but just fixing the test is easier for
now.
New in v2:
- Change ordering style, directories go at the end.
- Cleanup gdbserver's and data-directory's Makefile as well.
- Add comments at top of Makefiles about the ordering.
- Remove wrong trailing backslahes.
- Fix test gdb.base/default.exp.
gdb/ChangeLog:
* Makefile.in: Add comment about file lists ordering.
(SUBDIR_CLI_OBS, SUBDIR_CLI_SRCS, SUBDIR_MI_OBS, SUBDIR_MI_SRCS,
SUBDIR_TUI_OBS, SUBDIR_TUI_SRCS, SUBDIR_GCC_COMPILE_OBS,
SUBDIR_GCC_COMPILE_SRCS, SUBDIR_GUILE_OBS, SUBDIR_GUILE_SRCS,
SUBDIR_PYTHON_OBS, SUBDIR_PYTHON_SRCS, SUBDIR_GDBTK_OBS,
SUBDIR_GDBTK_SRCS, XMLFILES, REMOTE_OBS, ALL_64_TARGET_OBS,
ALL_TARGET_OBS, SFILES, HFILES_NO_SRCDIR, HFILES_WITH_SRCDIR,
COMMON_OBS, YYFILES, YYOBJ, generated_files, ALLDEPFILES):
Flatten list and order alphabetically.
* data-directory/Makefile.in: Add comment about file lists
ordering.
(GEN_SYSCALLS_FILES, PYTHON_FILE_LIST): Flatten list and order
alphabetically.
gdb/gdbserver/ChangeLog:
* Makefile.in (SFILES, OBS): Flatten list and order
alphabetically.
gdb/testsuite/ChangeLog:
* gdb.base/default.exp: Fix output of "set language".
Use regcache in software_single_step.
gdb:
2016-11-22 Yao Qi <yao.qi@linaro.org>
* aarch64-tdep.c (aarch64_software_single_step): Call
get_regcache_arch instead of get_frame_arch. Call
regcache_read_pc instead of get_frame_pc.
This patch adds a new regcache api regcache_raw_get_signed.
gdb:
2016-11-22 Yao Qi <yao.qi@linaro.org>
* regcache.c (regcache_raw_get_signed): New function.
* regcache.h (regcache_raw_get_signed): Declare.
We renamed VALUE_FRAME_ID to VALUE_NEXT_FRAME_ID recently,
https://sourceware.org/ml/gdb-patches/2016-11/msg00018.html
and we should use VALUE_NEXT_FRAME_ID in value_from_component
too.
gdb:
2016-11-22 Yao Qi <yao.qi@linaro.org>
* value.c (value_from_component): Use VALUE_NEXT_FRAME_ID
instead of VALUE_FROM_ID.
A little oversight from my part, it caused the Makefile not to track
the dependencies from mi/*.c files.
gdb/ChangeLog:
* Makefile.in (%o: $(srcdir)/mi/%.c): Add missing POSTCOMPILE
step.
