* [RFA 0/2] c++ patch follow-ups @ 2016-10-20 21:51 Tom Tromey 2016-10-20 21:51 ` [RFA 1/2] Change minimal_symbol_reader::record_full to take a bool Tom Tromey 2016-10-20 21:51 ` [RFA 2/2] Make some dwarf_expr_context methods pure virtual Tom Tromey 0 siblings, 2 replies; 14+ messages in thread From: Tom Tromey @ 2016-10-20 21:51 UTC (permalink / raw) To: gdb-patches These are follow-up patches that Pedro requested during review of my earlier C++ series. The first patch changes a type in minimal_symbol_reader. The second patch changes some dwarf_expr_context methods to be pure virtual. Built and regtested on x86-64 Fedora 24. Tom ^ permalink raw reply [flat|nested] 14+ messages in thread
* [RFA 1/2] Change minimal_symbol_reader::record_full to take a bool 2016-10-20 21:51 [RFA 0/2] c++ patch follow-ups Tom Tromey @ 2016-10-20 21:51 ` Tom Tromey 2016-10-20 22:14 ` Pedro Alves 2016-10-20 21:51 ` [RFA 2/2] Make some dwarf_expr_context methods pure virtual Tom Tromey 1 sibling, 1 reply; 14+ messages in thread From: Tom Tromey @ 2016-10-20 21:51 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey This changes an "int" to a "bool" in the signature for minimal_symbol_reader::record_full, and then fixes the callers. 2016-10-20 Tom Tromey <tom@tromey.com> * minsyms.h (minimal_symbol_reader::record_full): "copy_name" now a bool. (record, record_with_info): Update. * minsyms.c (record): Fix indentation. (record_full): Fix indentation. Update for type change. * elfread.c (record_minimal_symbol): "copy_name" now a bool. (elf_symtab_read): "copy_names" now a bool. (elf_rel_plt_read, elf_read_minimal_symbols): Update. --- gdb/ChangeLog | 11 +++++++++++ gdb/elfread.c | 19 ++++++++++--------- gdb/minsyms.c | 9 ++++----- gdb/minsyms.h | 14 +++++++------- 4 files changed, 32 insertions(+), 21 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index f3dd5d7..35df7f0 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,16 @@ 2016-10-20 Tom Tromey <tom@tromey.com> + * minsyms.h (minimal_symbol_reader::record_full): "copy_name" now + a bool. + (record, record_with_info): Update. + * minsyms.c (record): Fix indentation. + (record_full): Fix indentation. Update for type change. + * elfread.c (record_minimal_symbol): "copy_name" now a bool. + (elf_symtab_read): "copy_names" now a bool. + (elf_rel_plt_read, elf_read_minimal_symbols): Update. + +2016-10-20 Tom Tromey <tom@tromey.com> + * main.c: Include <vector>. (cmdarg_s): Remove typedef. Don't define VEC. (captured_main_1): Use vector, not VEC. Remove cleanups. diff --git a/gdb/elfread.c b/gdb/elfread.c index 485e55d..f5aa55e 100644 --- a/gdb/elfread.c +++ b/gdb/elfread.c @@ -192,7 +192,7 @@ elf_locate_sections (bfd *ignore_abfd, asection *sectp, void *eip) static struct minimal_symbol * record_minimal_symbol (minimal_symbol_reader &reader, - const char *name, int name_len, int copy_name, + const char *name, int name_len, bool copy_name, CORE_ADDR address, enum minimal_symbol_type ms_type, asection *bfd_section, struct objfile *objfile) @@ -229,7 +229,7 @@ static void elf_symtab_read (minimal_symbol_reader &reader, struct objfile *objfile, int type, long number_of_symbols, asymbol **symbol_table, - int copy_names) + bool copy_names) { struct gdbarch *gdbarch = get_objfile_arch (objfile); asymbol *sym; @@ -488,7 +488,7 @@ elf_symtab_read (minimal_symbol_reader &reader, { int len = atsign - sym->name; - record_minimal_symbol (reader, sym->name, len, 1, symaddr, + record_minimal_symbol (reader, sym->name, len, true, symaddr, ms_type, sym->section, objfile); } } @@ -505,8 +505,8 @@ elf_symtab_read (minimal_symbol_reader &reader, { struct minimal_symbol *mtramp; - mtramp = record_minimal_symbol (reader, sym->name, len - 4, 1, - symaddr, + mtramp = record_minimal_symbol (reader, sym->name, len - 4, + true, symaddr, mst_solib_trampoline, sym->section, objfile); if (mtramp) @@ -612,7 +612,7 @@ elf_rel_plt_read (minimal_symbol_reader &reader, msym = record_minimal_symbol (reader, string_buffer, name_len + got_suffix_len, - 1, address, mst_slot_got_plt, got_plt, + true, address, mst_slot_got_plt, got_plt, objfile); if (msym) SET_MSYMBOL_SIZE (msym, ptr_size); @@ -1078,7 +1078,8 @@ elf_read_minimal_symbols (struct objfile *objfile, int symfile_flags, bfd_get_filename (objfile->obfd), bfd_errmsg (bfd_get_error ())); - elf_symtab_read (reader, objfile, ST_REGULAR, symcount, symbol_table, 0); + elf_symtab_read (reader, objfile, ST_REGULAR, symcount, symbol_table, + false); } /* Add the dynamic symbols. */ @@ -1104,7 +1105,7 @@ elf_read_minimal_symbols (struct objfile *objfile, int symfile_flags, bfd_errmsg (bfd_get_error ())); elf_symtab_read (reader, objfile, ST_DYNAMIC, dynsymcount, - dyn_symbol_table, 0); + dyn_symbol_table, false); elf_rel_plt_read (reader, objfile, dyn_symbol_table); } @@ -1140,7 +1141,7 @@ elf_read_minimal_symbols (struct objfile *objfile, int symfile_flags, for (i = 0; i < synthcount; i++) synth_symbol_table[i] = synthsyms + i; elf_symtab_read (reader, objfile, ST_SYNTHETIC, synthcount, - synth_symbol_table.get (), 1); + synth_symbol_table.get (), true); } /* Install any minimal symbols that have been collected as the current diff --git a/gdb/minsyms.c b/gdb/minsyms.c index 5f6db60..d804c95 100644 --- a/gdb/minsyms.c +++ b/gdb/minsyms.c @@ -943,7 +943,7 @@ minimal_symbol_reader::~minimal_symbol_reader () void minimal_symbol_reader::record (const char *name, CORE_ADDR address, - enum minimal_symbol_type ms_type) + enum minimal_symbol_type ms_type) { int section; @@ -974,10 +974,9 @@ minimal_symbol_reader::record (const char *name, CORE_ADDR address, struct minimal_symbol * minimal_symbol_reader::record_full (const char *name, int name_len, - int copy_name, - CORE_ADDR address, - enum minimal_symbol_type ms_type, - int section) + bool copy_name, CORE_ADDR address, + enum minimal_symbol_type ms_type, + int section) { struct msym_bunch *newobj; struct minimal_symbol *msymbol; diff --git a/gdb/minsyms.h b/gdb/minsyms.h index b22920b..06b3b4e 100644 --- a/gdb/minsyms.h +++ b/gdb/minsyms.h @@ -93,35 +93,35 @@ class minimal_symbol_reader ADDRESS - the address of the symbol MS_TYPE - the type of the symbol SECTION - the symbol's section - appropriate obj_section for the minimal symbol. This can be NULL. - OBJFILE - the objfile associated with the minimal symbol. */ + */ struct minimal_symbol *record_full (const char *name, int name_len, - int copy_name, + bool copy_name, CORE_ADDR address, enum minimal_symbol_type ms_type, int section); /* Like record_full, but: - uses strlen to compute NAME_LEN, - - passes COPY_NAME = 1, + - passes COPY_NAME = true, - and passes a default SECTION, depending on the type This variant does not return the new symbol. */ - void record (const char *, CORE_ADDR, enum minimal_symbol_type); + void record (const char *name, CORE_ADDR address, + enum minimal_symbol_type ms_type); /* Like record_full, but: - uses strlen to compute NAME_LEN, - - passes COPY_NAME = 1. */ + - passes COPY_NAME = true. */ struct minimal_symbol *record_with_info (const char *name, CORE_ADDR address, enum minimal_symbol_type ms_type, int section) { - return record_full (name, strlen (name), 1, address, ms_type, section); + return record_full (name, strlen (name), true, address, ms_type, section); } private: -- 2.7.4 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA 1/2] Change minimal_symbol_reader::record_full to take a bool 2016-10-20 21:51 ` [RFA 1/2] Change minimal_symbol_reader::record_full to take a bool Tom Tromey @ 2016-10-20 22:14 ` Pedro Alves 0 siblings, 0 replies; 14+ messages in thread From: Pedro Alves @ 2016-10-20 22:14 UTC (permalink / raw) To: Tom Tromey, gdb-patches OK. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 14+ messages in thread
* [RFA 2/2] Make some dwarf_expr_context methods pure virtual 2016-10-20 21:51 [RFA 0/2] c++ patch follow-ups Tom Tromey 2016-10-20 21:51 ` [RFA 1/2] Change minimal_symbol_reader::record_full to take a bool Tom Tromey @ 2016-10-20 21:51 ` Tom Tromey 2016-10-20 22:21 ` Pedro Alves 1 sibling, 1 reply; 14+ messages in thread From: Tom Tromey @ 2016-10-20 21:51 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey This patch changes some dwarf_expr_context to be pure virtual, as mentioned during the discussion of an earlier patch in this series. 2016-10-20 Tom Tromey <tom@tromey.com> * dwarf2expr.h (class dwarf_expr_context) <get_frame_base, get_frame_cfa, get_tls_address, dwarf_call, push_dwarf_block_entry_value, get_addr_index, get_object_address>: Now pure virtual. * dwarf2-frame.c (class dwarf_expr_executor) <get_frame_base, get_frame_cfa, get_tls_address, dwarf_call, push_dwarf_block_entry_value, get_addr_index, get_object_address>: New methods. <invalid>: New method. --- gdb/ChangeLog | 12 ++++++++++++ gdb/dwarf2-frame.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ gdb/dwarf2expr.h | 38 +++++++------------------------------- 3 files changed, 63 insertions(+), 31 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 35df7f0..ad7da6b 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,17 @@ 2016-10-20 Tom Tromey <tom@tromey.com> + * dwarf2expr.h (class dwarf_expr_context) + <get_frame_base, get_frame_cfa, get_tls_address, dwarf_call, + push_dwarf_block_entry_value, get_addr_index, get_object_address>: + Now pure virtual. + * dwarf2-frame.c (class dwarf_expr_executor) + <get_frame_base, get_frame_cfa, get_tls_address, dwarf_call, + push_dwarf_block_entry_value, get_addr_index, get_object_address>: + New methods. + <invalid>: New method. + +2016-10-20 Tom Tromey <tom@tromey.com> + * minsyms.h (minimal_symbol_reader::record_full): "copy_name" now a bool. (record, record_with_info): Update. diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c index c9962e1..beab304 100644 --- a/gdb/dwarf2-frame.c +++ b/gdb/dwarf2-frame.c @@ -351,6 +351,50 @@ class dwarf_expr_executor : public dwarf_expr_context { read_memory (addr, buf, len); } + + void get_frame_base (const gdb_byte **start, size_t *length) OVERRIDE + { + invalid ("DW_OP_fbreg"); + } + + void push_dwarf_reg_entry_value (enum call_site_parameter_kind kind, + union call_site_parameter_u kind_u, + int deref_size) OVERRIDE + { + invalid ("DW_OP_GNU_entry_value"); + } + + CORE_ADDR get_object_address () OVERRIDE + { + invalid ("DW_OP_push_object_address"); + } + + CORE_ADDR get_frame_cfa () OVERRIDE + { + invalid ("DW_OP_call_frame_cfa"); + } + + CORE_ADDR get_tls_address (CORE_ADDR offset) OVERRIDE + { + invalid ("DW_OP_form_tls_address"); + } + + void dwarf_call (cu_offset die_offset) OVERRIDE + { + invalid ("DW_OP_call*"); + } + + CORE_ADDR get_addr_index (unsigned int index) + { + invalid ("DW_OP_GNU_addr_index"); + } + + private: + + void invalid (const char *op) ATTRIBUTE_NORETURN + { + error (_("%s is invalid in this context"), op); + } }; static CORE_ADDR diff --git a/gdb/dwarf2expr.h b/gdb/dwarf2expr.h index 63bdc6e..7bf194a 100644 --- a/gdb/dwarf2expr.h +++ b/gdb/dwarf2expr.h @@ -156,16 +156,10 @@ struct dwarf_expr_context /* Return the location expression for the frame base attribute, in START and LENGTH. The result must be live until the current expression evaluation is complete. */ - virtual void get_frame_base (const gdb_byte **start, size_t *length) - { - error (_("%s is invalid in this context"), "DW_OP_fbreg"); - } + virtual void get_frame_base (const gdb_byte **start, size_t *length) = 0; /* Return the CFA for the frame. */ - virtual CORE_ADDR get_frame_cfa () - { - error (_("%s is invalid in this context"), "DW_OP_call_frame_cfa"); - } + virtual CORE_ADDR get_frame_cfa () = 0; /* Return the PC for the frame. */ virtual CORE_ADDR get_frame_pc () @@ -175,19 +169,13 @@ struct dwarf_expr_context /* Return the thread-local storage address for DW_OP_GNU_push_tls_address or DW_OP_form_tls_address. */ - virtual CORE_ADDR get_tls_address (CORE_ADDR offset) - { - error (_("%s is invalid in this context"), "DW_OP_form_tls_address"); - } + virtual CORE_ADDR get_tls_address (CORE_ADDR offset) = 0; /* Execute DW_AT_location expression for the DWARF expression subroutine in the DIE at DIE_OFFSET in the CU. Do not touch STACK while it being passed to and returned from the called DWARF subroutine. */ - virtual void dwarf_call (cu_offset die_offset) - { - error (_("%s is invalid in this context"), "DW_OP_call*"); - } + virtual void dwarf_call (cu_offset die_offset) = 0; /* Return the base type given by the indicated DIE. This can throw an exception if the DIE is invalid or does not represent a base @@ -206,26 +194,14 @@ struct dwarf_expr_context DW_AT_GNU_call_site_value. */ virtual void push_dwarf_reg_entry_value (enum call_site_parameter_kind kind, union call_site_parameter_u kind_u, - int deref_size) - { - internal_error (__FILE__, __LINE__, - _("Support for DW_OP_GNU_entry_value is unimplemented")); - } + int deref_size) = 0; /* Return the address indexed by DW_OP_GNU_addr_index. This can throw an exception if the index is out of range. */ - virtual CORE_ADDR get_addr_index (unsigned int index) - { - error (_("%s is invalid in this context"), "DW_OP_GNU_addr_index"); - } + virtual CORE_ADDR get_addr_index (unsigned int index) = 0; /* Return the `object address' for DW_OP_push_object_address. */ - virtual CORE_ADDR get_object_address () - { - internal_error (__FILE__, __LINE__, - _("Support for DW_OP_push_object_address " - "is unimplemented")); - } + virtual CORE_ADDR get_object_address () = 0; private: -- 2.7.4 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA 2/2] Make some dwarf_expr_context methods pure virtual 2016-10-20 21:51 ` [RFA 2/2] Make some dwarf_expr_context methods pure virtual Tom Tromey @ 2016-10-20 22:21 ` Pedro Alves 2016-10-24 12:28 ` Ulrich Weigand 0 siblings, 1 reply; 14+ messages in thread From: Pedro Alves @ 2016-10-20 22:21 UTC (permalink / raw) To: Tom Tromey, gdb-patches On 10/20/2016 10:51 PM, Tom Tromey wrote: > This patch changes some dwarf_expr_context to be pure virtual, as > mentioned during the discussion of an earlier patch in this series. > > 2016-10-20 Tom Tromey <tom@tromey.com> > > * dwarf2expr.h (class dwarf_expr_context) > <get_frame_base, get_frame_cfa, get_tls_address, dwarf_call, > push_dwarf_block_entry_value, get_addr_index, get_object_address>: > Now pure virtual. > * dwarf2-frame.c (class dwarf_expr_executor) > <get_frame_base, get_frame_cfa, get_tls_address, dwarf_call, > push_dwarf_block_entry_value, get_addr_index, get_object_address>: > New methods. > <invalid>: New method. Nice, thanks. LGTM. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA 2/2] Make some dwarf_expr_context methods pure virtual 2016-10-20 22:21 ` Pedro Alves @ 2016-10-24 12:28 ` Ulrich Weigand 2016-10-24 13:36 ` Tom Tromey 0 siblings, 1 reply; 14+ messages in thread From: Ulrich Weigand @ 2016-10-24 12:28 UTC (permalink / raw) To: Pedro Alves; +Cc: Tom Tromey, gdb-patches Pedro Alves wrote: > On 10/20/2016 10:51 PM, Tom Tromey wrote: > > This patch changes some dwarf_expr_context to be pure virtual, as > > mentioned during the discussion of an earlier patch in this series. > > > > 2016-10-20 Tom Tromey <tom@tromey.com> > > > > * dwarf2expr.h (class dwarf_expr_context) > > <get_frame_base, get_frame_cfa, get_tls_address, dwarf_call, > > push_dwarf_block_entry_value, get_addr_index, get_object_address>: > > Now pure virtual. > > * dwarf2-frame.c (class dwarf_expr_executor) > > <get_frame_base, get_frame_cfa, get_tls_address, dwarf_call, > > push_dwarf_block_entry_value, get_addr_index, get_object_address>: > > New methods. > > <invalid>: New method. > > Nice, thanks. LGTM. This seems to have broken my SPU daily build (running on RHEL 5 with a GCC 4.1 system compiler): gdb/dwarf2expr.h:69: warning: 'struct dwarf_expr_context' has virtual functions but non-virtual destructor Is this a C++ version issue? Bye, Ulrich -- Dr. Ulrich Weigand GNU/Linux compilers and toolchain Ulrich.Weigand@de.ibm.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA 2/2] Make some dwarf_expr_context methods pure virtual 2016-10-24 12:28 ` Ulrich Weigand @ 2016-10-24 13:36 ` Tom Tromey 2016-10-25 13:22 ` Pedro Alves 0 siblings, 1 reply; 14+ messages in thread From: Tom Tromey @ 2016-10-24 13:36 UTC (permalink / raw) To: Ulrich Weigand; +Cc: Pedro Alves, Tom Tromey, gdb-patches >>>>> "Ulrich" == Ulrich Weigand <uweigand@de.ibm.com> writes: Ulrich> This seems to have broken my SPU daily build (running on RHEL 5 with a Ulrich> GCC 4.1 system compiler): Ulrich> gdb/dwarf2expr.h:69: warning: 'struct dwarf_expr_context' has Ulrich> virtual functions but non-virtual destructor Ulrich> Is this a C++ version issue? I don't know. Adding a "virtual" to the destructor is safe though. I would do it but I'm traveling this week. Tom ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA 2/2] Make some dwarf_expr_context methods pure virtual 2016-10-24 13:36 ` Tom Tromey @ 2016-10-25 13:22 ` Pedro Alves 2016-10-25 13:29 ` Ulrich Weigand 2016-10-25 13:47 ` [RFA 2/2] Make some dwarf_expr_context methods pure virtual Tom Tromey 0 siblings, 2 replies; 14+ messages in thread From: Pedro Alves @ 2016-10-25 13:22 UTC (permalink / raw) To: Tom Tromey, Ulrich Weigand; +Cc: gdb-patches On 10/24/2016 02:36 PM, Tom Tromey wrote: >>>>>> "Ulrich" == Ulrich Weigand <uweigand@de.ibm.com> writes: > > Ulrich> This seems to have broken my SPU daily build (running on RHEL 5 with a > Ulrich> GCC 4.1 system compiler): Any chance you could install the newer GCC from DTS on that machine? Otherwise, if/when we go C++11, that builder will stop working. > Ulrich> gdb/dwarf2expr.h:69: warning: 'struct dwarf_expr_context' has > Ulrich> virtual functions but non-virtual destructor > > Ulrich> Is this a C++ version issue? > > I don't know. Adding a "virtual" to the destructor is safe though. I > would do it but I'm traveling this week. Looks like -Wnon-virtual-dtor was removed from (guessing) -Wall at some point. With GCC 7 / trunk: $ make WERROR_CFLAGS="-Wnon-virtual-dtor" dwarf2expr.o ... In file included from .../src/gdb/dwarf2expr.c:28:0: .../src/gdb/dwarf2expr.h:68:8: warning: ‘struct dwarf_expr_context’ has virtual functions and accessible non-virtual destructor [-Wnon-virtual-dtor] struct dwarf_expr_context ^~~~~~~~~~~~~~~~~~ I'll add the "virtual" in a bit. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA 2/2] Make some dwarf_expr_context methods pure virtual 2016-10-25 13:22 ` Pedro Alves @ 2016-10-25 13:29 ` Ulrich Weigand 2016-10-25 13:59 ` [pushed] Make dwarf_expr_context's destructor virtual (Re: [RFA 2/2] Make some dwarf_expr_context methods pure virtual) Pedro Alves 2016-10-25 13:47 ` [RFA 2/2] Make some dwarf_expr_context methods pure virtual Tom Tromey 1 sibling, 1 reply; 14+ messages in thread From: Ulrich Weigand @ 2016-10-25 13:29 UTC (permalink / raw) To: Pedro Alves; +Cc: Tom Tromey, gdb-patches Pedro Alves wrote: > >>>>>> "Ulrich" == Ulrich Weigand <uweigand@de.ibm.com> writes: > > > > Ulrich> This seems to have broken my SPU daily build (running on RHEL 5 with a > > Ulrich> GCC 4.1 system compiler): > > Any chance you could install the newer GCC from DTS on that machine? I don't think there even is a DTS for RHEL 5 on Power, or has that changed? > Otherwise, if/when we go C++11, that builder will stop working. Well, once GDB officially no longer supports building with GCC 4.1, I'll have to come up with another solution; I'll probably just build my own compiler then. However, as long as GCC 4.1 *is* supported, I think it is a good to actually still have a system testing that. (In any case, I'm not sure how long it makes sense to keep the Cell SPU daily build up and running, given that RHEL 5 is about to go out of service anyway and more recent distros no longer support Cell ...) > In file included from .../src/gdb/dwarf2expr.c:28:0: > .../src/gdb/dwarf2expr.h:68:8: warning: ‘struct dwarf_expr_context’ has virtual functions and accessible non-virtual destructor [-Wnon-virtual-dtor] > struct dwarf_expr_context > ^~~~~~~~~~~~~~~~~~ > > I'll add the "virtual" in a bit. Thanks! Bye, Ulrich -- Dr. Ulrich Weigand GNU/Linux compilers and toolchain Ulrich.Weigand@de.ibm.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* [pushed] Make dwarf_expr_context's destructor virtual (Re: [RFA 2/2] Make some dwarf_expr_context methods pure virtual) 2016-10-25 13:29 ` Ulrich Weigand @ 2016-10-25 13:59 ` Pedro Alves 0 siblings, 0 replies; 14+ messages in thread From: Pedro Alves @ 2016-10-25 13:59 UTC (permalink / raw) To: Ulrich Weigand; +Cc: Tom Tromey, gdb-patches On 10/25/2016 02:29 PM, Ulrich Weigand wrote: > Pedro Alves wrote: >>>>>>>> "Ulrich" == Ulrich Weigand <uweigand@de.ibm.com> writes: >>> >>> Ulrich> This seems to have broken my SPU daily build (running on RHEL 5 with a >>> Ulrich> GCC 4.1 system compiler): >> >> Any chance you could install the newer GCC from DTS on that machine? > > I don't think there even is a DTS for RHEL 5 on Power, or has that changed? Whoops, you're right. Sorry, missed that. >> Otherwise, if/when we go C++11, that builder will stop working. > > Well, once GDB officially no longer supports building with GCC 4.1, I'll > have to come up with another solution; I'll probably just build my own > compiler then. However, as long as GCC 4.1 *is* supported, I think it > is a good to actually still have a system testing that. Agreed. I'd like to move forward with requiring C++11 soon though. I was mainly giving it some more time: https://sourceware.org/ml/gdb-patches/2016-10/msg00607.html > (In any case, I'm not sure how long it makes sense to keep the Cell SPU > daily build up and running, given that RHEL 5 is about to go out of > service anyway and more recent distros no longer support Cell ...) It's up to you of course. As we chatted on the Cauldron, it's unfortunate since that's the best way we have to avoid multi-arch support from bit rotting. I'd like to see gdb move in the direction of supporting seamless remote-procedure calls. E.g., following an RPC call across 32-bit client x 64-bit server and back, even on the same machine. Seamless CPU+GPU debugging support likely will take advantage of it as well in the future. So maybe we'll gain back testing some other way. > >> In file included from .../src/gdb/dwarf2expr.c:28:0: >> .../src/gdb/dwarf2expr.h:68:8: warning: ‘struct dwarf_expr_context’ has virtual functions and accessible non-virtual destructor [-Wnon-virtual-dtor] >> struct dwarf_expr_context >> ^~~~~~~~~~~~~~~~~~ >> >> I'll add the "virtual" in a bit. > > Thanks! Pushed now. From beb18c865c42ab57176099eecb65bb52e71def85 Mon Sep 17 00:00:00 2001 From: Pedro Alves <palves@redhat.com> Date: Tue, 25 Oct 2016 14:32:35 +0100 Subject: [PATCH] Make dwarf_expr_context's destructor virtual MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ref: https://sourceware.org/ml/gdb-patches/2016-10/msg00662.html $ make WERROR_CFLAGS="-Wnon-virtual-dtor" dwarf2expr.o ... In file included from .../src/gdb/dwarf2expr.c:28:0: .../src/gdb/dwarf2expr.h:68:8: warning: ‘struct dwarf_expr_context’ has virtual functions and accessible non-virtual destructor [-Wnon-virtual-dtor] struct dwarf_expr_context ^~~~~~~~~~~~~~~~~~ Happens to not be a problem in practice currently because concrete subclasses are allocated on the stack. I.e., we don't ever delete objects of types that derive from dwarf_expr_context through pointers to dwarf_expr_context. gdb/ChangeLog: 2016-10-25 Pedro Alves <palves@redhat.com> * dwarf2expr.h (struct dwarf_expr_context) <~dwarf_expr_context>: Make virtual. --- gdb/ChangeLog | 5 +++++ gdb/dwarf2expr.h | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 10d6866..91f36f9 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,8 @@ +2016-10-25 Pedro Alves <palves@redhat.com> + + * dwarf2expr.h (struct dwarf_expr_context) <~dwarf_expr_context>: + Make virtual. + 2016-10-25 Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> PR build/20712 diff --git a/gdb/dwarf2expr.h b/gdb/dwarf2expr.h index 7bf194a..3d08120 100644 --- a/gdb/dwarf2expr.h +++ b/gdb/dwarf2expr.h @@ -68,7 +68,7 @@ struct dwarf_stack_value struct dwarf_expr_context { dwarf_expr_context (); - ~dwarf_expr_context (); + virtual ~dwarf_expr_context (); void push_address (CORE_ADDR value, int in_stack_memory); void eval (const gdb_byte *addr, size_t len); -- 2.5.5 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA 2/2] Make some dwarf_expr_context methods pure virtual 2016-10-25 13:22 ` Pedro Alves 2016-10-25 13:29 ` Ulrich Weigand @ 2016-10-25 13:47 ` Tom Tromey 2016-10-25 14:05 ` Pedro Alves 1 sibling, 1 reply; 14+ messages in thread From: Tom Tromey @ 2016-10-25 13:47 UTC (permalink / raw) To: Pedro Alves; +Cc: Tom Tromey, Ulrich Weigand, gdb-patches >>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes: Pedro> Looks like -Wnon-virtual-dtor was removed from (guessing) -Wall Pedro> at some point. Perhaps it's worth adding it explicitly in configure.ac. (Though I don't know why it was removed from -Wall, so maybe it's not always appropriate, for reasons I don't know.) Tom ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA 2/2] Make some dwarf_expr_context methods pure virtual 2016-10-25 13:47 ` [RFA 2/2] Make some dwarf_expr_context methods pure virtual Tom Tromey @ 2016-10-25 14:05 ` Pedro Alves 2016-10-25 16:25 ` Trevor Saunders 0 siblings, 1 reply; 14+ messages in thread From: Pedro Alves @ 2016-10-25 14:05 UTC (permalink / raw) To: Tom Tromey; +Cc: Ulrich Weigand, gdb-patches On 10/25/2016 02:47 PM, Tom Tromey wrote: >>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes: > > Pedro> Looks like -Wnon-virtual-dtor was removed from (guessing) -Wall > Pedro> at some point. > > Perhaps it's worth adding it explicitly in configure.ac. > (Though I don't know why it was removed from -Wall, so maybe it's not > always appropriate, for reasons I don't know.) Yeah, cross my mind too. I'd like to understand why it was removed first. (I don't know whether it used to be in -Wall, or default, or enabled along some other -Wfoo option.) I found these: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=7302 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=15214 but those fixed false positives. I think that would have supported enabling by default. Sounds like something else might have been identified. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA 2/2] Make some dwarf_expr_context methods pure virtual 2016-10-25 14:05 ` Pedro Alves @ 2016-10-25 16:25 ` Trevor Saunders 2016-10-25 16:57 ` Pedro Alves 0 siblings, 1 reply; 14+ messages in thread From: Trevor Saunders @ 2016-10-25 16:25 UTC (permalink / raw) To: Pedro Alves; +Cc: Tom Tromey, Ulrich Weigand, gdb-patches On Tue, Oct 25, 2016 at 03:04:57PM +0100, Pedro Alves wrote: > On 10/25/2016 02:47 PM, Tom Tromey wrote: > >>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes: > > > > Pedro> Looks like -Wnon-virtual-dtor was removed from (guessing) -Wall > > Pedro> at some point. > > > > Perhaps it's worth adding it explicitly in configure.ac. > > (Though I don't know why it was removed from -Wall, so maybe it's not > > always appropriate, for reasons I don't know.) > > Yeah, cross my mind too. I'd like to understand why it was > removed first. (I don't know whether it used to be in -Wall, or > default, or enabled along some other -Wfoo option.) with g++ 6 I'm seeing -Wdelete-non-virtual-dtor enabled by -Wall, which seems to be warning about the same thing? oh, accept -wdelete-non-virtual-dtor looks like it wouldn't warn if the class is always on the stack like it is here. Trev ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA 2/2] Make some dwarf_expr_context methods pure virtual 2016-10-25 16:25 ` Trevor Saunders @ 2016-10-25 16:57 ` Pedro Alves 0 siblings, 0 replies; 14+ messages in thread From: Pedro Alves @ 2016-10-25 16:57 UTC (permalink / raw) To: Trevor Saunders; +Cc: Tom Tromey, Ulrich Weigand, gdb-patches On 10/25/2016 05:33 PM, Trevor Saunders wrote: > On Tue, Oct 25, 2016 at 03:04:57PM +0100, Pedro Alves wrote: >> On 10/25/2016 02:47 PM, Tom Tromey wrote: >>>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes: >>> >>> Pedro> Looks like -Wnon-virtual-dtor was removed from (guessing) -Wall >>> Pedro> at some point. >>> >>> Perhaps it's worth adding it explicitly in configure.ac. >>> (Though I don't know why it was removed from -Wall, so maybe it's not >>> always appropriate, for reasons I don't know.) >> >> Yeah, cross my mind too. I'd like to understand why it was >> removed first. (I don't know whether it used to be in -Wall, or >> default, or enabled along some other -Wfoo option.) > > with g++ 6 I'm seeing -Wdelete-non-virtual-dtor enabled by -Wall, which > seems to be warning about the same thing? > > oh, accept -wdelete-non-virtual-dtor looks like it wouldn't warn if the > class is always on the stack like it is here. Ah, nice. Sounds like we're already covered then. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2016-10-25 16:57 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-10-20 21:51 [RFA 0/2] c++ patch follow-ups Tom Tromey 2016-10-20 21:51 ` [RFA 1/2] Change minimal_symbol_reader::record_full to take a bool Tom Tromey 2016-10-20 22:14 ` Pedro Alves 2016-10-20 21:51 ` [RFA 2/2] Make some dwarf_expr_context methods pure virtual Tom Tromey 2016-10-20 22:21 ` Pedro Alves 2016-10-24 12:28 ` Ulrich Weigand 2016-10-24 13:36 ` Tom Tromey 2016-10-25 13:22 ` Pedro Alves 2016-10-25 13:29 ` Ulrich Weigand 2016-10-25 13:59 ` [pushed] Make dwarf_expr_context's destructor virtual (Re: [RFA 2/2] Make some dwarf_expr_context methods pure virtual) Pedro Alves 2016-10-25 13:47 ` [RFA 2/2] Make some dwarf_expr_context methods pure virtual Tom Tromey 2016-10-25 14:05 ` Pedro Alves 2016-10-25 16:25 ` Trevor Saunders 2016-10-25 16:57 ` Pedro Alves
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox