* Re: [ANNOUNCEMENT] GDB 8.1 release branch created! [not found] <announce.20180105041805.3FC35808E9@joel.gnat.com> @ 2018-01-16 17:31 ` Eli Zaretskii 2018-01-16 19:02 ` Sergio Durigan Junior 2018-01-18 15:53 ` Eli Zaretskii 0 siblings, 2 replies; 40+ messages in thread From: Eli Zaretskii @ 2018-01-16 17:31 UTC (permalink / raw) To: gdb-patches > From: Joel Brobecker <brobecker@adacore.com> > Date: Fri, 5 Jan 2018 08:18:05 +0400 (+04) > > A quick message to announce that the GDB 8.1 branch has just been created. > > The prerelease snapshots will be available at: > > ftp://sourceware.org/pub/gdb/snapshots/branch/gdb.tar.xz > ftp://sourceware.org/pub/gdb/snapshots/branch/gdb.tar.gz I've built this pre-release with mingw.org's MinGW, and found a few problems. The most serious one is the following compilation error: g++ -x c++ -O2 -gdwarf-4 -g3 -I. -I. -I./common -I./config -DLOCALEDIR="\"d:/usr/share/locale\"" -DHAVE_CONFIG_H -I./../include/opcode -I./../opcodes/.. -I./../readline/.. -I../bfd -I./../bfd -I./../include -I../libdecnumber -I./../libdecnumber -I./gnulib/import -Ibuild-gnulib/import -DTUI=1 -Id:/usr/include -Id:/usr/include/guile/2.0 -Id:/usr/include -Id:/usr/Python26/include -Id:/usr/Python26/include -Wall -Wpointer-arith -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable -Wno-sign-compare -Wno-narrowing -Wno-error=maybe-uninitialized -Wno-format -fno-strict-aliasing -DNDEBUG -fwrapv -c -o python/py-arch.o -MT python/py-arch.o -MMD -MP -MF python/.deps/py-arch.Tpo python/py-arch.c In file included from d:\usr\lib\gcc\mingw32\6.3.0\include\c++\math.h:36:0, from build-gnulib/import/math.h:27, from d:/usr/Python26/include/pyport.h:235, from d:/usr/Python26/include/Python.h:58, from python/python-internal.h:94, from python/py-arch.c:24: d:\usr\lib\gcc\mingw32\6.3.0\include\c++\cmath:1157:11: error: '::hypot' has not been declared using ::hypot; ^~~~~ Makefile:1618: recipe for target `python/py-arch.o' failed Googling suggests the following solution; is it okay to push this (with the necessary logs and after doing the "paperwork" required for branch changes)? Or does someone have better ideas? (Does this work in MinGW64?) --- gdb/python/python-internal.h~0 2018-01-12 05:31:04.000000000 +0200 +++ gdb/python/python-internal.h 2018-01-16 08:56:10.717759900 +0200 @@ -85,6 +85,12 @@ #define HAVE_SNPRINTF 1 #endif +/* Another kludge to avoid compilation errors because MinGW defines + 'hypot' to '_hypot', but the C++ headers says "using ::hypot". */ +#if defined(__MINGW32__) && defined(__cplusplus) +# define _hypot hypot +#endif + /* Request clean size types from Python. */ #define PY_SSIZE_T_CLEAN The other problems I saw are compilation warnings, see below. Some of them sound like GCC doesn't understand our TRY/CATCH blocks, but others seem like real problems, at least from the POV of a naïve reader such as myself. I wonder how come others don't see these warnings, and why weren't they fixed during development, when we use -Werror. Here are the warnings I saw: g++ -x c++ -O2 -gdwarf-4 -g3 -I. -I. -I./common -I./config -DLOCALEDIR="\"d:/usr/share/locale\"" -DHAVE_CONFIG_H -I./../include/opcode -I./../opcodes/.. -I./../readline/.. -I../bfd -I./../bfd -I./../include -I../libdecnumber -I./../libdecnumber -I./gnulib/import -Ibuild-gnulib/import -DTUI=1 -Id:/usr/include -Id:/usr/include/guile/2.0 -Id:/usr/include -Id:/usr/Python26/include -Id:/usr/Python26/include -Wall -Wpointer-arith -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable -Wno-sign-compare -Wno-narrowing -Wno-error=maybe-uninitialized -Wno-format -c -o cli/cli-cmds.o -MT cli/cli-cmds.o -MMD -MP -MF cli/.deps/cli-cmds.Tpo cli/cli-cmds.c cli/cli-cmds.c: In function 'void complete_command(const char*, int)': cli/cli-cmds.c:277:71: warning: 'word' may be used uninitialized in this function [-Wmaybe-uninitialized] = tracker->build_completion_result (word, word - arg, strlen (arg)); ^ cli/cli-cmds.c:277:71: warning: 'tracker' may be used uninitialized in this function [-Wmaybe-uninitialized] g++ -x c++ -O2 -gdwarf-4 -g3 -I. -I. -I./common -I./config -DLOCALEDIR="\"d:/usr/share/locale\"" -DHAVE_CONFIG_H -I./../include/opcode -I./../opcodes/.. -I./../readline/.. -I../bfd -I./../bfd -I./../include -I../libdecnumber -I./../libdecnumber -I./gnulib/import -Ibuild-gnulib/import -DTUI=1 -Id:/usr/include -Id:/usr/include/guile/2.0 -Id:/usr/include -Id:/usr/Python26/include -Id:/usr/Python26/include -Wall -Wpointer-arith -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable -Wno-sign-compare -Wno-narrowing -Wno-error=maybe-uninitialized -Wno-format -c -o breakpoint.o -MT breakpoint.o -MMD -MP -MF ./.deps/breakpoint.Tpo breakpoint.c breakpoint.c: In function 'void check_status_watchpoint(bpstat)': breakpoint.c:5079:4: warning: 'e' may be used uninitialized in this function [-Wmaybe-uninitialized] switch (e) ^~~~~~ breakpoint.c:5056:20: note: 'e' was declared here wp_check_result e; ^ g++ -x c++ -O2 -gdwarf-4 -g3 -I. -I. -I./common -I./config -DLOCALEDIR="\"d:/usr/share/locale\"" -DHAVE_CONFIG_H -I./../include/opcode -I./../opcodes/.. -I./../readline/.. -I../bfd -I./../bfd -I./../include -I../libdecnumber -I./../libdecnumber -I./gnulib/import -Ibuild-gnulib/import -DTUI=1 -Id:/usr/include -Id:/usr/include/guile/2.0 -Id:/usr/include -Id:/usr/Python26/include -Id:/usr/Python26/include -Wall -Wpointer-arith -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable -Wno-sign-compare -Wno-narrowing -Wno-error=maybe-uninitialized -Wno-format -c -o infrun.o -MT infrun.o -MMD -MP -MF ./.deps/infrun.Tpo infrun.c In file included from infrun.c:26:0: inferior.h: In function 'void handle_vfork_child_exec_or_exit(int)': inferior.h:537:39: warning: '*((void*)(& maybe_restore_inferior)+20).scoped_restore_current_inferior::m_saved_inf' may be used uninitialized in this function [-Wmaybe-uninitialized] { set_current_inferior (m_saved_inf); } ^ infrun.c:927:6: note: '*((void*)(& maybe_restore_inferior)+20).scoped_restore_current_inferior::m_saved_inf' was declared here maybe_restore_inferior; ^~~~~~~~~~~~~~~~~~~~~~ In file included from inferior.h:48:0, from infrun.c:26: progspace.h:285:47: warning: '*((void*)(& maybe_restore_inferior)+16).scoped_restore_current_program_space::m_saved_pspace' may be used uninitialized in this function [-Wmaybe-uninitialized] { set_current_program_space (m_saved_pspace); } ^ infrun.c:927:6: note: '*((void*)(& maybe_restore_inferior)+16).scoped_restore_current_program_space::m_saved_pspace' was declared here maybe_restore_inferior; ^~~~~~~~~~~~~~~~~~~~~~ g++ -x c++ -O2 -gdwarf-4 -g3 -I. -I. -I./common -I./config -DLOCALEDIR="\"d:/usr/share/locale\"" -DHAVE_CONFIG_H -I./../include/opcode -I./../opcodes/.. -I./../readline/.. -I../bfd -I./../bfd -I./../include -I../libdecnumber -I./../libdecnumber -I./gnulib/import -Ibuild-gnulib/import -DTUI=1 -Id:/usr/include -Id:/usr/include/guile/2.0 -Id:/usr/include -Id:/usr/Python26/include -Id:/usr/Python26/include -Wall -Wpointer-arith -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable -Wno-sign-compare -Wno-narrowing -Wno-error=maybe-uninitialized -Wno-format -c -o typeprint.o -MT typeprint.o -MMD -MP -MF ./.deps/ typeprint.Tpo typeprint.c typeprint.c: In function 'void whatis_exp(const char*, int)': typeprint.c:515:60: warning: 'val' may be used uninitialized in this function [-Wmaybe-uninitialized] real_type = value_rtti_type (val, &full, &top, &using_enc); ^ g++ -x c++ -O2 -gdwarf-4 -g3 -I. -I. -I./common -I./config -DLOCALEDIR="\"d:/usr/share/locale\"" -DHAVE_CONFIG_H -I./../include/opcode -I./../opcodes/.. -I./../readline/.. -I../bfd -I./../bfd -I./../include -I../libdecnumber -I./../libdecnumber -I./gnulib/import -Ibuild-gnulib/import -DTUI=1 -Id:/usr/include -Id:/usr/include/guile/2.0 -Id:/usr/include -Id:/usr/Python26/include -Id:/usr/Python26/include -Wall -Wpointer-arith -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable -Wno-sign-compare -Wno-narrowing -Wno-error=maybe-uninitialized -Wno-format -c -o compile/compile.o -MT compile/compile.o -MMD -MP -MF compile/.deps/compile.Tpo compile/compile.c compile/compile.c: In function 'void eval_compile_command(command_line*, const char*, compile_i_scope_types, void*)': compile/compile.c:549:20: warning: 'triplet_rx' may be used uninitialized in this function [-Wmaybe-uninitialized] argc, argv); ^ compile/compile.c:466:9: note: 'triplet_rx' was declared here char *triplet_rx; ^~~~~~~~~~ ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [ANNOUNCEMENT] GDB 8.1 release branch created! 2018-01-16 17:31 ` [ANNOUNCEMENT] GDB 8.1 release branch created! Eli Zaretskii @ 2018-01-16 19:02 ` Sergio Durigan Junior 2018-01-16 19:46 ` [PATCH] Fix warning on gdb/compile/compile.c (C++-ify "triplet_rx") Sergio Durigan Junior ` (2 more replies) 2018-01-18 15:53 ` Eli Zaretskii 1 sibling, 3 replies; 40+ messages in thread From: Sergio Durigan Junior @ 2018-01-16 19:02 UTC (permalink / raw) To: Eli Zaretskii; +Cc: gdb-patches On Tuesday, January 16 2018, Eli Zaretskii wrote: > I've built this pre-release with mingw.org's MinGW, and found a few > problems. [...] > The other problems I saw are compilation warnings, see below. Some of > them sound like GCC doesn't understand our TRY/CATCH blocks, but others > seem like real problems, at least from the POV of a naïve reader such > as myself. I wonder how come others don't see these warnings, and why > weren't they fixed during development, when we use -Werror. Here are > the warnings I saw: > [...] > g++ -x c++ -O2 -gdwarf-4 -g3 -I. -I. -I./common -I./config -DLOCALEDIR="\"d:/usr/share/locale\"" -DHAVE_CONFIG_H -I./../include/opcode -I./../opcodes/.. -I./../readline/.. -I../bfd -I./../bfd -I./../include -I../libdecnumber -I./../libdecnumber -I./gnulib/import -Ibuild-gnulib/import -DTUI=1 -Id:/usr/include -Id:/usr/include/guile/2.0 -Id:/usr/include -Id:/usr/Python26/include -Id:/usr/Python26/include -Wall -Wpointer-arith -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable -Wno-sign-compare -Wno-narrowing -Wno-error=maybe-uninitialized -Wno-format -c -o infrun.o -MT infrun.o -MMD -MP -MF ./.deps/infrun.Tpo infrun.c > In file included from infrun.c:26:0: > inferior.h: In function 'void handle_vfork_child_exec_or_exit(int)': > inferior.h:537:39: warning: '*((void*)(& maybe_restore_inferior)+20).scoped_restore_current_inferior::m_saved_inf' may be used uninitialized in this function [-Wmaybe-uninitialized] > { set_current_inferior (m_saved_inf); } > ^ > infrun.c:927:6: note: '*((void*)(& maybe_restore_inferior)+20).scoped_restore_current_inferior::m_saved_inf' was declared here > maybe_restore_inferior; > ^~~~~~~~~~~~~~~~~~~~~~ > In file included from inferior.h:48:0, > from infrun.c:26: > progspace.h:285:47: warning: '*((void*)(& maybe_restore_inferior)+16).scoped_restore_current_program_space::m_saved_pspace' may be used uninitialized in this function [-Wmaybe-uninitialized] > { set_current_program_space (m_saved_pspace); } > ^ > infrun.c:927:6: note: '*((void*)(& maybe_restore_inferior)+16).scoped_restore_current_program_space::m_saved_pspace' was declared here > maybe_restore_inferior; > ^~~~~~~~~~~~~~~~~~~~~~ > > g++ -x c++ -O2 -gdwarf-4 -g3 -I. -I. -I./common -I./config -DLOCALEDIR="\"d:/usr/share/locale\"" -DHAVE_CONFIG_H -I./../include/opcode -I./../opcodes/.. -I./../readline/.. -I../bfd -I./../bfd -I./../include -I../libdecnumber -I./../libdecnumber -I./gnulib/import -Ibuild-gnulib/import -DTUI=1 -Id:/usr/include -Id:/usr/include/guile/2.0 -Id:/usr/include -Id:/usr/Python26/include -Id:/usr/Python26/include -Wall -Wpointer-arith -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable -Wno-sign-compare -Wno-narrowing -Wno-error=maybe-uninitialized -Wno-format -c -o typeprint.o -MT typeprint.o -MMD -MP -MF ./.deps/ > typeprint.Tpo typeprint.c > typeprint.c: In function 'void whatis_exp(const char*, int)': > typeprint.c:515:60: warning: 'val' may be used uninitialized in this function [-Wmaybe-uninitialized] > real_type = value_rtti_type (val, &full, &top, &using_enc); > ^ > > g++ -x c++ -O2 -gdwarf-4 -g3 -I. -I. -I./common -I./config -DLOCALEDIR="\"d:/usr/share/locale\"" -DHAVE_CONFIG_H -I./../include/opcode -I./../opcodes/.. -I./../readline/.. -I../bfd -I./../bfd -I./../include -I../libdecnumber -I./../libdecnumber -I./gnulib/import -Ibuild-gnulib/import -DTUI=1 -Id:/usr/include -Id:/usr/include/guile/2.0 -Id:/usr/include -Id:/usr/Python26/include -Id:/usr/Python26/include -Wall -Wpointer-arith -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable -Wno-sign-compare -Wno-narrowing -Wno-error=maybe-uninitialized -Wno-format -c -o compile/compile.o -MT compile/compile.o -MMD -MP -MF compile/.deps/compile.Tpo compile/compile.c > compile/compile.c: In function 'void eval_compile_command(command_line*, const char*, compile_i_scope_types, void*)': > compile/compile.c:549:20: warning: 'triplet_rx' may be used uninitialized in this function [-Wmaybe-uninitialized] > argc, argv); > ^ > compile/compile.c:466:9: note: 'triplet_rx' was declared here > char *triplet_rx; > ^~~~~~~~~~ FWIW, I've seen these warnings when preparing a Fedora GDB release. I'll take a look at them. -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/ ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH] Fix warning on gdb/compile/compile.c (C++-ify "triplet_rx") 2018-01-16 19:02 ` Sergio Durigan Junior @ 2018-01-16 19:46 ` Sergio Durigan Junior 2018-01-17 15:33 ` Eli Zaretskii 2018-01-17 17:17 ` Simon Marchi 2018-01-16 20:32 ` [PATCH] Fix unitialized warning on gdb/typeprint.c:whatis_exp Sergio Durigan Junior 2018-01-16 20:36 ` [ANNOUNCEMENT] GDB 8.1 release branch created! Sergio Durigan Junior 2 siblings, 2 replies; 40+ messages in thread From: Sergio Durigan Junior @ 2018-01-16 19:46 UTC (permalink / raw) To: GDB Patches; +Cc: Eli Zaretskii, Sergio Durigan Junior This fixes a GCC warning that happens when compiling gdb/compile/compile.c on some GCC versions (e.g., "gcc (GCC) 7.2.1 20180104 (Red Hat 7.2.1-6)"). It's a simple patch that converts "triplet_rx" from "char *" to "std::string", thus guaranteeing that it will be always initialized. I've regtested this patch and did not find any regressions. OK to apply on both master and 8.1 (after creating a bug for it)? gdb/ChangeLog: 2018-01-16 Sergio Durigan Junior <sergiodj@redhat.com> * compile/compile.c (compile_to_object): Convert "triplet_rx" to "std::string". --- gdb/compile/compile.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c index 2ee75930ac..47646169c8 100644 --- a/gdb/compile/compile.c +++ b/gdb/compile/compile.c @@ -463,7 +463,7 @@ compile_to_object (struct command_line *cmd, const char *cmd_string, char **argv; int ok; struct gdbarch *gdbarch = get_current_arch (); - char *triplet_rx; + std::string triplet_rx; char *error_message; if (!target_has_execution) @@ -527,15 +527,15 @@ compile_to_object (struct command_line *cmd, const char *cmd_string, } else { - const char *os_rx = osabi_triplet_regexp (gdbarch_osabi (gdbarch)); - const char *arch_rx = gdbarch_gnu_triplet_regexp (gdbarch); + std::string os_rx = osabi_triplet_regexp (gdbarch_osabi (gdbarch)); + std::string arch_rx = gdbarch_gnu_triplet_regexp (gdbarch); /* Allow triplets with or without vendor set. */ - triplet_rx = concat (arch_rx, "(-[^-]*)?-", os_rx, (char *) NULL); - make_cleanup (xfree, triplet_rx); + triplet_rx = arch_rx + "(-[^-]*)?-" + os_rx; if (compiler->fe->ops->version >= GCC_FE_VERSION_1) - compiler->fe->ops->set_triplet_regexp (compiler->fe, triplet_rx); + compiler->fe->ops->set_triplet_regexp (compiler->fe, + triplet_rx.c_str ()); } /* Set compiler command-line arguments. */ @@ -545,7 +545,8 @@ compile_to_object (struct command_line *cmd, const char *cmd_string, if (compiler->fe->ops->version >= GCC_FE_VERSION_1) error_message = compiler->fe->ops->set_arguments (compiler->fe, argc, argv); else - error_message = compiler->fe->ops->set_arguments_v0 (compiler->fe, triplet_rx, + error_message = compiler->fe->ops->set_arguments_v0 (compiler->fe, + triplet_rx.c_str (), argc, argv); if (error_message != NULL) { -- 2.14.3 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH] Fix warning on gdb/compile/compile.c (C++-ify "triplet_rx") 2018-01-16 19:46 ` [PATCH] Fix warning on gdb/compile/compile.c (C++-ify "triplet_rx") Sergio Durigan Junior @ 2018-01-17 15:33 ` Eli Zaretskii 2018-01-17 17:17 ` Simon Marchi 1 sibling, 0 replies; 40+ messages in thread From: Eli Zaretskii @ 2018-01-17 15:33 UTC (permalink / raw) To: Sergio Durigan Junior; +Cc: gdb-patches > From: Sergio Durigan Junior <sergiodj@redhat.com> > Cc: Eli Zaretskii <eliz@gnu.org>, Sergio Durigan Junior <sergiodj@redhat.com> > Date: Tue, 16 Jan 2018 14:46:41 -0500 > > This fixes a GCC warning that happens when compiling > gdb/compile/compile.c on some GCC versions (e.g., "gcc (GCC) 7.2.1 > 20180104 (Red Hat 7.2.1-6)"). > > It's a simple patch that converts "triplet_rx" from "char *" to > "std::string", thus guaranteeing that it will be always initialized. > > I've regtested this patch and did not find any regressions. OK to > apply on both master and 8.1 (after creating a bug for it)? > > gdb/ChangeLog: > 2018-01-16 Sergio Durigan Junior <sergiodj@redhat.com> > > * compile/compile.c (compile_to_object): Convert "triplet_rx" > to "std::string". Thanks, this fixes the warning for me. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH] Fix warning on gdb/compile/compile.c (C++-ify "triplet_rx") 2018-01-16 19:46 ` [PATCH] Fix warning on gdb/compile/compile.c (C++-ify "triplet_rx") Sergio Durigan Junior 2018-01-17 15:33 ` Eli Zaretskii @ 2018-01-17 17:17 ` Simon Marchi 2018-01-17 23:07 ` Sergio Durigan Junior 1 sibling, 1 reply; 40+ messages in thread From: Simon Marchi @ 2018-01-17 17:17 UTC (permalink / raw) To: Sergio Durigan Junior; +Cc: GDB Patches, Eli Zaretskii On 2018-01-16 14:46, Sergio Durigan Junior wrote: > This fixes a GCC warning that happens when compiling > gdb/compile/compile.c on some GCC versions (e.g., "gcc (GCC) 7.2.1 > 20180104 (Red Hat 7.2.1-6)"). > > It's a simple patch that converts "triplet_rx" from "char *" to > "std::string", thus guaranteeing that it will be always initialized. > > I've regtested this patch and did not find any regressions. OK to > apply on both master and 8.1 (after creating a bug for it)? > > gdb/ChangeLog: > 2018-01-16 Sergio Durigan Junior <sergiodj@redhat.com> > > * compile/compile.c (compile_to_object): Convert "triplet_rx" > to "std::string". > --- > gdb/compile/compile.c | 15 ++++++++------- > 1 file changed, 8 insertions(+), 7 deletions(-) > > diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c > index 2ee75930ac..47646169c8 100644 > --- a/gdb/compile/compile.c > +++ b/gdb/compile/compile.c > @@ -463,7 +463,7 @@ compile_to_object (struct command_line *cmd, const > char *cmd_string, > char **argv; > int ok; > struct gdbarch *gdbarch = get_current_arch (); > - char *triplet_rx; > + std::string triplet_rx; > char *error_message; > > if (!target_has_execution) > @@ -527,15 +527,15 @@ compile_to_object (struct command_line *cmd, > const char *cmd_string, > } > else > { > - const char *os_rx = osabi_triplet_regexp (gdbarch_osabi > (gdbarch)); > - const char *arch_rx = gdbarch_gnu_triplet_regexp (gdbarch); > + std::string os_rx = osabi_triplet_regexp (gdbarch_osabi > (gdbarch)); > + std::string arch_rx = gdbarch_gnu_triplet_regexp (gdbarch); Making these std::string makes unnecessary copies. You can write the line below like this: triplet_rx = std::string (arch_rx) + "(-[^-]*)?-" + os_rx; Otherwise, LGTM. Simon ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH] Fix warning on gdb/compile/compile.c (C++-ify "triplet_rx") 2018-01-17 17:17 ` Simon Marchi @ 2018-01-17 23:07 ` Sergio Durigan Junior 2018-01-17 23:42 ` Simon Marchi 0 siblings, 1 reply; 40+ messages in thread From: Sergio Durigan Junior @ 2018-01-17 23:07 UTC (permalink / raw) To: Simon Marchi; +Cc: GDB Patches, Eli Zaretskii On Wednesday, January 17 2018, Simon Marchi wrote: > On 2018-01-16 14:46, Sergio Durigan Junior wrote: >> This fixes a GCC warning that happens when compiling >> gdb/compile/compile.c on some GCC versions (e.g., "gcc (GCC) 7.2.1 >> 20180104 (Red Hat 7.2.1-6)"). >> >> It's a simple patch that converts "triplet_rx" from "char *" to >> "std::string", thus guaranteeing that it will be always initialized. >> >> I've regtested this patch and did not find any regressions. OK to >> apply on both master and 8.1 (after creating a bug for it)? >> >> gdb/ChangeLog: >> 2018-01-16 Sergio Durigan Junior <sergiodj@redhat.com> >> >> * compile/compile.c (compile_to_object): Convert "triplet_rx" >> to "std::string". >> --- >> gdb/compile/compile.c | 15 ++++++++------- >> 1 file changed, 8 insertions(+), 7 deletions(-) >> >> diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c >> index 2ee75930ac..47646169c8 100644 >> --- a/gdb/compile/compile.c >> +++ b/gdb/compile/compile.c >> @@ -463,7 +463,7 @@ compile_to_object (struct command_line *cmd, const >> char *cmd_string, >> char **argv; >> int ok; >> struct gdbarch *gdbarch = get_current_arch (); >> - char *triplet_rx; >> + std::string triplet_rx; >> char *error_message; >> >> if (!target_has_execution) >> @@ -527,15 +527,15 @@ compile_to_object (struct command_line *cmd, >> const char *cmd_string, >> } >> else >> { >> - const char *os_rx = osabi_triplet_regexp (gdbarch_osabi >> (gdbarch)); >> - const char *arch_rx = gdbarch_gnu_triplet_regexp (gdbarch); >> + std::string os_rx = osabi_triplet_regexp (gdbarch_osabi >> (gdbarch)); >> + std::string arch_rx = gdbarch_gnu_triplet_regexp (gdbarch); > > Making these std::string makes unnecessary copies. > > You can write the line below like this: > > triplet_rx = std::string (arch_rx) + "(-[^-]*)?-" + os_rx; > > Otherwise, LGTM. Thanks, Simon. Pushed to master: 7d937cad0acdccd0ff485435fbe16f005e994c66 Is it OK to push this to the 8.1 branch as well? If so, I'm not sure I should create a bug for this, or just go ahead and push it. Thanks, -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/ ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH] Fix warning on gdb/compile/compile.c (C++-ify "triplet_rx") 2018-01-17 23:07 ` Sergio Durigan Junior @ 2018-01-17 23:42 ` Simon Marchi 2018-01-17 23:48 ` Sergio Durigan Junior 0 siblings, 1 reply; 40+ messages in thread From: Simon Marchi @ 2018-01-17 23:42 UTC (permalink / raw) To: Sergio Durigan Junior; +Cc: GDB Patches, Eli Zaretskii On 2018-01-17 18:07, Sergio Durigan Junior wrote: > On Wednesday, January 17 2018, Simon Marchi wrote: > >> On 2018-01-16 14:46, Sergio Durigan Junior wrote: >>> This fixes a GCC warning that happens when compiling >>> gdb/compile/compile.c on some GCC versions (e.g., "gcc (GCC) 7.2.1 >>> 20180104 (Red Hat 7.2.1-6)"). >>> >>> It's a simple patch that converts "triplet_rx" from "char *" to >>> "std::string", thus guaranteeing that it will be always initialized. >>> >>> I've regtested this patch and did not find any regressions. OK to >>> apply on both master and 8.1 (after creating a bug for it)? >>> >>> gdb/ChangeLog: >>> 2018-01-16 Sergio Durigan Junior <sergiodj@redhat.com> >>> >>> * compile/compile.c (compile_to_object): Convert "triplet_rx" >>> to "std::string". >>> --- >>> gdb/compile/compile.c | 15 ++++++++------- >>> 1 file changed, 8 insertions(+), 7 deletions(-) >>> >>> diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c >>> index 2ee75930ac..47646169c8 100644 >>> --- a/gdb/compile/compile.c >>> +++ b/gdb/compile/compile.c >>> @@ -463,7 +463,7 @@ compile_to_object (struct command_line *cmd, >>> const >>> char *cmd_string, >>> char **argv; >>> int ok; >>> struct gdbarch *gdbarch = get_current_arch (); >>> - char *triplet_rx; >>> + std::string triplet_rx; >>> char *error_message; >>> >>> if (!target_has_execution) >>> @@ -527,15 +527,15 @@ compile_to_object (struct command_line *cmd, >>> const char *cmd_string, >>> } >>> else >>> { >>> - const char *os_rx = osabi_triplet_regexp (gdbarch_osabi >>> (gdbarch)); >>> - const char *arch_rx = gdbarch_gnu_triplet_regexp (gdbarch); >>> + std::string os_rx = osabi_triplet_regexp (gdbarch_osabi >>> (gdbarch)); >>> + std::string arch_rx = gdbarch_gnu_triplet_regexp (gdbarch); >> >> Making these std::string makes unnecessary copies. >> >> You can write the line below like this: >> >> triplet_rx = std::string (arch_rx) + "(-[^-]*)?-" + os_rx; >> >> Otherwise, LGTM. > > Thanks, Simon. > > Pushed to master: > > 7d937cad0acdccd0ff485435fbe16f005e994c66 > > Is it OK to push this to the 8.1 branch as well? If so, I'm not sure I > should create a bug for this, or just go ahead and push it. Yes, it is fine to go to the 8.1 branch. A PR is not necessary right now. They are only necessary after there was an initial release on the branch, to track what fixes were made on top of that initial version. Simon ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH] Fix warning on gdb/compile/compile.c (C++-ify "triplet_rx") 2018-01-17 23:42 ` Simon Marchi @ 2018-01-17 23:48 ` Sergio Durigan Junior 0 siblings, 0 replies; 40+ messages in thread From: Sergio Durigan Junior @ 2018-01-17 23:48 UTC (permalink / raw) To: Simon Marchi; +Cc: GDB Patches, Eli Zaretskii On Wednesday, January 17 2018, Simon Marchi wrote: > On 2018-01-17 18:07, Sergio Durigan Junior wrote: >> On Wednesday, January 17 2018, Simon Marchi wrote: >> >>> On 2018-01-16 14:46, Sergio Durigan Junior wrote: >>>> This fixes a GCC warning that happens when compiling >>>> gdb/compile/compile.c on some GCC versions (e.g., "gcc (GCC) 7.2.1 >>>> 20180104 (Red Hat 7.2.1-6)"). >>>> >>>> It's a simple patch that converts "triplet_rx" from "char *" to >>>> "std::string", thus guaranteeing that it will be always initialized. >>>> >>>> I've regtested this patch and did not find any regressions. OK to >>>> apply on both master and 8.1 (after creating a bug for it)? >>>> >>>> gdb/ChangeLog: >>>> 2018-01-16 Sergio Durigan Junior <sergiodj@redhat.com> >>>> >>>> * compile/compile.c (compile_to_object): Convert "triplet_rx" >>>> to "std::string". >>>> --- >>>> gdb/compile/compile.c | 15 ++++++++------- >>>> 1 file changed, 8 insertions(+), 7 deletions(-) >>>> >>>> diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c >>>> index 2ee75930ac..47646169c8 100644 >>>> --- a/gdb/compile/compile.c >>>> +++ b/gdb/compile/compile.c >>>> @@ -463,7 +463,7 @@ compile_to_object (struct command_line *cmd, >>>> const >>>> char *cmd_string, >>>> char **argv; >>>> int ok; >>>> struct gdbarch *gdbarch = get_current_arch (); >>>> - char *triplet_rx; >>>> + std::string triplet_rx; >>>> char *error_message; >>>> >>>> if (!target_has_execution) >>>> @@ -527,15 +527,15 @@ compile_to_object (struct command_line *cmd, >>>> const char *cmd_string, >>>> } >>>> else >>>> { >>>> - const char *os_rx = osabi_triplet_regexp (gdbarch_osabi >>>> (gdbarch)); >>>> - const char *arch_rx = gdbarch_gnu_triplet_regexp (gdbarch); >>>> + std::string os_rx = osabi_triplet_regexp (gdbarch_osabi >>>> (gdbarch)); >>>> + std::string arch_rx = gdbarch_gnu_triplet_regexp (gdbarch); >>> >>> Making these std::string makes unnecessary copies. >>> >>> You can write the line below like this: >>> >>> triplet_rx = std::string (arch_rx) + "(-[^-]*)?-" + os_rx; >>> >>> Otherwise, LGTM. >> >> Thanks, Simon. >> >> Pushed to master: >> >> 7d937cad0acdccd0ff485435fbe16f005e994c66 >> >> Is it OK to push this to the 8.1 branch as well? If so, I'm not sure I >> should create a bug for this, or just go ahead and push it. > > Yes, it is fine to go to the 8.1 branch. A PR is not necessary right > now. They are only necessary after there was an initial release on > the branch, to track what fixes were made on top of that initial > version. Awesome. In this case, pushed to the 8.1 branch: 2a54d2158beeb2833cb3fb4da68e7c55e341159a Thanks, -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/ ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH] Fix unitialized warning on gdb/typeprint.c:whatis_exp 2018-01-16 19:02 ` Sergio Durigan Junior 2018-01-16 19:46 ` [PATCH] Fix warning on gdb/compile/compile.c (C++-ify "triplet_rx") Sergio Durigan Junior @ 2018-01-16 20:32 ` Sergio Durigan Junior 2018-01-17 15:34 ` Eli Zaretskii ` (2 more replies) 2018-01-16 20:36 ` [ANNOUNCEMENT] GDB 8.1 release branch created! Sergio Durigan Junior 2 siblings, 3 replies; 40+ messages in thread From: Sergio Durigan Junior @ 2018-01-16 20:32 UTC (permalink / raw) To: GDB Patches; +Cc: Eli Zaretskii, Sergio Durigan Junior This simple patch initializes "struct value *val" to NULL, which silences a when compiling GDB with certain GCC versions. This warning is technically incorrect, because there is now way that "val" will be used unitialized if you look at the code flow, but it's a simple "fix" and doesn't do any harm. Is it OK to push this to master and 8.1? I believe I will still need to create a bug with the 8.1 milestone set, even for this simple patch. gdb/ChangeLog: 2017-01-16 Sergio Durigan Junior <sergiodj@redhat.com> * typeprint.c (whatis_exp): Initialize "val" to NULL. --- gdb/typeprint.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gdb/typeprint.c b/gdb/typeprint.c index 9a125076a1..bf9aec5436 100644 --- a/gdb/typeprint.c +++ b/gdb/typeprint.c @@ -405,7 +405,7 @@ error_unknown_type (const char *sym_print_name) static void whatis_exp (const char *exp, int show) { - struct value *val; + struct value *val = NULL; struct cleanup *old_chain; struct type *real_type = NULL; struct type *type; -- 2.14.3 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH] Fix unitialized warning on gdb/typeprint.c:whatis_exp 2018-01-16 20:32 ` [PATCH] Fix unitialized warning on gdb/typeprint.c:whatis_exp Sergio Durigan Junior @ 2018-01-17 15:34 ` Eli Zaretskii 2018-01-17 16:48 ` Pedro Alves 2018-01-20 1:03 ` [PATCH] Fix segfault when using 'set print object on' + whatis <struct> Sergio Durigan Junior 2 siblings, 0 replies; 40+ messages in thread From: Eli Zaretskii @ 2018-01-17 15:34 UTC (permalink / raw) To: Sergio Durigan Junior; +Cc: gdb-patches > From: Sergio Durigan Junior <sergiodj@redhat.com> > Cc: Eli Zaretskii <eliz@gnu.org>, > Sergio Durigan Junior <sergiodj@redhat.com> > Date: Tue, 16 Jan 2018 15:32:39 -0500 > > This simple patch initializes "struct value *val" to NULL, which > silences a when compiling GDB with certain GCC versions. > > This warning is technically incorrect, because there is now way that > "val" will be used unitialized if you look at the code flow, but it's > a simple "fix" and doesn't do any harm. This fixes the warning for me (of course). Thanks. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH] Fix unitialized warning on gdb/typeprint.c:whatis_exp 2018-01-16 20:32 ` [PATCH] Fix unitialized warning on gdb/typeprint.c:whatis_exp Sergio Durigan Junior 2018-01-17 15:34 ` Eli Zaretskii @ 2018-01-17 16:48 ` Pedro Alves 2018-01-17 18:03 ` Sergio Durigan Junior 2018-01-20 1:03 ` [PATCH] Fix segfault when using 'set print object on' + whatis <struct> Sergio Durigan Junior 2 siblings, 1 reply; 40+ messages in thread From: Pedro Alves @ 2018-01-17 16:48 UTC (permalink / raw) To: Sergio Durigan Junior, GDB Patches; +Cc: Eli Zaretskii On 01/16/2018 08:32 PM, Sergio Durigan Junior wrote: > This simple patch initializes "struct value *val" to NULL, which > silences a when compiling GDB with certain GCC versions. Please include a representative paste of warnings in git logs. It makes it easier to see what a patch/commit is about. > This warning is technically incorrect, because there is now way that > "val" will be used unitialized if you look at the code flow, but it's > a simple "fix" and doesn't do any harm. No, it's not incorrect. It's showing a real bug. Try, e.g.: (gdb) set print object on (gdb) whatis some_structure_type Thread 1 "gdb" received signal SIGSEGV, Segmentation fault. 0x00000000005dda90 in check_typedef (type=0x6120736573756170) at src/gdb/gdbtypes.c:2388 2388 int instance_flags = TYPE_INSTANCE_FLAGS (type); (top-gdb) bt #0 0x00000000005dda90 in check_typedef(type*) (type=0x6120736573756170) at src/gdb/gdbtypes.c:2388 #1 0x00000000005e63fb in gnuv3_rtti_type(value*, int*, LONGEST*, int*) (value=0xdf21b7, full_p=0x7fffffffd180, top_p=0x7fffffffd188, using_enc_p=0x7fffffffd184) at src/gdb/gnu-v3-abi.c:293 #2 0x000000000055f7d8 in value_rtti_type(value*, int*, long*, int*) (v=0xdf21b7, full=0x7fffffffd180, top=0x7fffffffd188, using_enc=0x7fffffffd184) at src/gdb/cp-abi.c:117 #3 0x00000000006cc61f in whatis_exp(char const*, int) (exp=<optimized out>, show=-1) at src/gdb/typeprint.c:515 #4 0x0000000000472462 in cmd_func(cmd_list_element*, char const*, int) (cmd=<optimized out>, args=<optimized out>, from_tty=<optimized out>) at src/gdb/cli/cli-decode.c:1886 #5 0x00000000006b833a in execute_command(char const*, int) (p=<optimized out>, from_tty=1) at src/gdb/top.c:630 #6 0x00000000005c0f8c in command_handler(char const*) (command=0xdf21b0 "whatis siginfo_t") at src/gdb/event-top.c:583 #7 0x00000000005c12d8 in command_line_handler(char*) (rl=<optimized out>) at /home/pedro/gdb/mygit/src/gdb/event-top.c:774 ... Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH] Fix unitialized warning on gdb/typeprint.c:whatis_exp 2018-01-17 16:48 ` Pedro Alves @ 2018-01-17 18:03 ` Sergio Durigan Junior 0 siblings, 0 replies; 40+ messages in thread From: Sergio Durigan Junior @ 2018-01-17 18:03 UTC (permalink / raw) To: Pedro Alves; +Cc: GDB Patches, Eli Zaretskii On Wednesday, January 17 2018, Pedro Alves wrote: > On 01/16/2018 08:32 PM, Sergio Durigan Junior wrote: >> This simple patch initializes "struct value *val" to NULL, which >> silences a when compiling GDB with certain GCC versions. > > Please include a representative paste of warnings in git logs. > It makes it easier to see what a patch/commit is about. Will do. >> This warning is technically incorrect, because there is now way that >> "val" will be used unitialized if you look at the code flow, but it's >> a simple "fix" and doesn't do any harm. > > No, it's not incorrect. It's showing a real bug. Try, e.g.: > > (gdb) set print object on > (gdb) whatis some_structure_type > > Thread 1 "gdb" received signal SIGSEGV, Segmentation fault. > 0x00000000005dda90 in check_typedef (type=0x6120736573756170) at src/gdb/gdbtypes.c:2388 > 2388 int instance_flags = TYPE_INSTANCE_FLAGS (type); > (top-gdb) bt > #0 0x00000000005dda90 in check_typedef(type*) (type=0x6120736573756170) at src/gdb/gdbtypes.c:2388 > #1 0x00000000005e63fb in gnuv3_rtti_type(value*, int*, LONGEST*, int*) > (value=0xdf21b7, full_p=0x7fffffffd180, top_p=0x7fffffffd188, > using_enc_p=0x7fffffffd184) at src/gdb/gnu-v3-abi.c:293 > #2 0x000000000055f7d8 in value_rtti_type(value*, int*, long*, int*) (v=0xdf21b7, full=0x7fffffffd180, top=0x7fffffffd188, using_enc=0x7fffffffd184) at src/gdb/cp-abi.c:117 > #3 0x00000000006cc61f in whatis_exp(char const*, int) (exp=<optimized out>, show=-1) at src/gdb/typeprint.c:515 > #4 0x0000000000472462 in cmd_func(cmd_list_element*, char const*, int) (cmd=<optimized out>, args=<optimized out>, from_tty=<optimized out>) > at src/gdb/cli/cli-decode.c:1886 > #5 0x00000000006b833a in execute_command(char const*, int) (p=<optimized out>, from_tty=1) at src/gdb/top.c:630 > #6 0x00000000005c0f8c in command_handler(char const*) (command=0xdf21b0 "whatis siginfo_t") at src/gdb/event-top.c:583 > #7 0x00000000005c12d8 in command_line_handler(char*) (rl=<optimized out>) at /home/pedro/gdb/mygit/src/gdb/event-top.c:774 > ... Ah, I totally missed that. Sorry about jumping into the conclusion that it was incorrect. I'll try to investigate what's happening here. -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/ ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH] Fix segfault when using 'set print object on' + whatis <struct> 2018-01-16 20:32 ` [PATCH] Fix unitialized warning on gdb/typeprint.c:whatis_exp Sergio Durigan Junior 2018-01-17 15:34 ` Eli Zaretskii 2018-01-17 16:48 ` Pedro Alves @ 2018-01-20 1:03 ` Sergio Durigan Junior 2018-01-22 17:42 ` [PATCH v2] Fix segfault when using 'set print object on' + whatis <struct> (Re: [PATCH] Fix segfault when using 'set print object on' + whatis <struct>) Pedro Alves 2 siblings, 1 reply; 40+ messages in thread From: Sergio Durigan Junior @ 2018-01-20 1:03 UTC (permalink / raw) To: GDB Patches; +Cc: Eli Zaretskii, Pedro Alves, Sergio Durigan Junior This problem was hidden behind a "maybe-uninitialized" warning generated when compiling GDB with a recent GCC. The warning is: ../../gdb/typeprint.c: In function 'void whatis_exp(const char*, int)': ../../gdb/typeprint.c:515:12: warning: 'val' may be used uninitialized in this function [-Wmaybe-uninitialized] real_type = value_rtti_type (val, &full, &top, &using_enc); ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ I submitted a patch fixing this by initializing "val" to NULL, but it was the wrong fix, as Pedro pointed out on <https://sourceware.org/ml/gdb-patches/2018-01/msg00346.html>: (gdb) set print object on (gdb) whatis some_structure_type Thread 1 "gdb" received signal SIGSEGV, Segmentation fault. 0x00000000005dda90 in check_typedef (type=0x6120736573756170) at src/gdb/gdbtypes.c:2388 2388 int instance_flags = TYPE_INSTANCE_FLAGS (type); ... So I set off to find the cause of the problem. It turns out that a recent-ish refactoring of the code on 'whatis_exp', introduced by: commit c973d0aa4a2c737ab527ae44a617f1c357e07364 Date: Mon Aug 21 11:34:32 2017 +0100 Fix type casts losing typedefs and reimplement "whatis" typedef stripping was the reason of the failure. After investigating what 'set print object on' was supposed to do to the output of 'whatis', if made sense initialize "val = evaluate_type (expr.get ());" all the time, not only when we're dealing with the 'ptype' command. I've regtested this on the BuildBot, without seeing any regressions. I've also extended 'gdb.base/whatis.exp' to check if the segfault is not there anymore. gdb/ChangeLog: yyyy-mm-dd Sergio Durigan Junior <sergiodj@redhat.com> * typeprint.c (whatis_exp): Move initialization of "val" outside of "if". gdb/testsuite/ChangeLog: yyyy-mm-dd Sergio Durigan Junior <sergiodj@redhat.com> * gdb.base/whatis.exp: Test that 'set print object on' + 'whatis <struct>' doesn't segfault. --- gdb/testsuite/gdb.base/whatis.exp | 7 +++++++ gdb/typeprint.c | 3 ++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/gdb/testsuite/gdb.base/whatis.exp b/gdb/testsuite/gdb.base/whatis.exp index dd6aeb02f9..8207dab59c 100644 --- a/gdb/testsuite/gdb.base/whatis.exp +++ b/gdb/testsuite/gdb.base/whatis.exp @@ -566,3 +566,10 @@ gdb_test "whatis int (*)(void, int, int)" \ gdb_test "whatis int (*)(int, void, int)" \ "'void' invalid as parameter type" \ "whatis applied to function with 'void' parameter type" + +# Test that 'set print object on' + whatis doesn't segfault. +clean_restart $binfile +gdb_test_no_output "set print object on" +gdb_test "whatis v_struct1" \ + "type = struct t_struct" \ + "whatis + set print object on doesn't segfault" diff --git a/gdb/typeprint.c b/gdb/typeprint.c index 9a125076a1..dd6e75bd4f 100644 --- a/gdb/typeprint.c +++ b/gdb/typeprint.c @@ -471,6 +471,8 @@ whatis_exp (const char *exp, int show) expression_up expr = parse_expression (exp); + val = evaluate_type (expr.get ()); + /* The behavior of "whatis" depends on whether the user expression names a type directly, or a language expression (including variable names). If the former, then "whatis" @@ -495,7 +497,6 @@ whatis_exp (const char *exp, int show) /* The user expression names a type indirectly by naming an object or expression of that type. Find that indirectly-named type. */ - val = evaluate_type (expr.get ()); type = value_type (val); } } -- 2.14.3 ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v2] Fix segfault when using 'set print object on' + whatis <struct> (Re: [PATCH] Fix segfault when using 'set print object on' + whatis <struct>) 2018-01-20 1:03 ` [PATCH] Fix segfault when using 'set print object on' + whatis <struct> Sergio Durigan Junior @ 2018-01-22 17:42 ` Pedro Alves 2018-01-22 18:04 ` Sergio Durigan Junior 0 siblings, 1 reply; 40+ messages in thread From: Pedro Alves @ 2018-01-22 17:42 UTC (permalink / raw) To: Sergio Durigan Junior, GDB Patches; +Cc: Eli Zaretskii Hi Sergio, On 01/20/2018 01:03 AM, Sergio Durigan Junior wrote: > This problem was hidden behind a "maybe-uninitialized" warning > generated when compiling GDB with a recent GCC. The warning is: > > ../../gdb/typeprint.c: In function 'void whatis_exp(const char*, int)': > ../../gdb/typeprint.c:515:12: warning: 'val' may be used uninitialized in this function [-Wmaybe-uninitialized] > real_type = value_rtti_type (val, &full, &top, &using_enc); > ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > I submitted a patch fixing this by initializing "val" to NULL, but it > was the wrong fix, as Pedro pointed out on > <https://sourceware.org/ml/gdb-patches/2018-01/msg00346.html>: IMHO, adding such history to the commit log directly doesn't really add much here. It's better IMO to leave that to "what changed in v2" comments (that don't go in the commit log), and instead update the commit log to go straight to the point. (please keep reading.) > > (gdb) set print object on > (gdb) whatis some_structure_type > > Thread 1 "gdb" received signal SIGSEGV, Segmentation fault. > 0x00000000005dda90 in check_typedef (type=0x6120736573756170) at src/gdb/gdbtypes.c:2388 > 2388 int instance_flags = TYPE_INSTANCE_FLAGS (type); > ... > > So I set off to find the cause of the problem. It turns out that a > recent-ish refactoring of the code on 'whatis_exp', introduced by: > > commit c973d0aa4a2c737ab527ae44a617f1c357e07364 > Date: Mon Aug 21 11:34:32 2017 +0100 > > Fix type casts losing typedefs and reimplement "whatis" typedef stripping > > was the reason of the failure. After investigating what 'set print > object on' was supposed to do to the output of 'whatis', if made sense > initialize "val = evaluate_type (expr.get ());" all the time, not only > when we're dealing with the 'ptype' command. The point of the c973d0aa4a2c change was to bypass evaluating the expression if the opcode is OP_TYPE. The proposed patch adds back the evaluation, while the OP_TYPE shortcut is left in there too. I think calling allocate_value instead in the path that misses initializing 'val' would make more sense, to end up with a dummy not_lval value, like expression evaluation does when evaluating EVAL_AVOID_SIDE_EFFECTS. But even better still is to set VAL to NULL in this case (only), and skip the "real type" printing. It doesn't make any sense to try to fetch the dynamic type of a type that didn't come from an actual program value in the first place. There's nothing dynamic about a statically named type. > > I've regtested this on the BuildBot, without seeing any regressions. > I've also extended 'gdb.base/whatis.exp' to check if the segfault is > not there anymore. The "whatis struct" case was an example, but there's another similar path in the code that also lead to a crash that is not covered by the testcase. The value_rtti_indirect_type path here: if (opts.objectprint) { if (((TYPE_CODE (type) == TYPE_CODE_PTR) || TYPE_IS_REFERENCE (type)) && (TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_STRUCT)) real_type = value_rtti_indirect_type (val, &full, &top, &using_enc); else if (TYPE_CODE (type) == TYPE_CODE_STRUCT) real_type = value_rtti_type (val, &full, &top, &using_enc); } > --- a/gdb/testsuite/gdb.base/whatis.exp > +++ b/gdb/testsuite/gdb.base/whatis.exp > @@ -566,3 +566,10 @@ gdb_test "whatis int (*)(void, int, int)" \ > gdb_test "whatis int (*)(int, void, int)" \ > "'void' invalid as parameter type" \ > "whatis applied to function with 'void' parameter type" > + > +# Test that 'set print object on' + whatis doesn't segfault. > +clean_restart $binfile > +gdb_test_no_output "set print object on" > +gdb_test "whatis v_struct1" \ > + "type = struct t_struct" \ > + "whatis + set print object on doesn't segfault" At some point later on someone is going to read this test name/message and wonder why is it talking about a segfault. All the other 50k+ tests in the testsuite are indirectly checking that gdb doesn't segfault either. This one's not that special in that regard. Instead, we're testing that the "set print object on" + "whatis <struct>" combination _works_ as expected, which among other things obviously includes not segfaulting, but also includes checking that the output is reasonable. As I was writing/experimenting the above, I ended up addressing my own comments. What do you think of this patch instead? New in v2: - set val to NULL and skip dynamic type printing in the "whatis <type>" case. - rewrite the new tests to reuse preexisting tests, and add new tests for whatis with pointers and references to structs coupled with "set print object on". Try both "set print object on/off". From f86b4087d23be4cf20bf3dc2f8407cef3f28830d Mon Sep 17 00:00:00 2001 From: Sergio Durigan Junior <sergiodj@redhat.com> Date: Mon, 22 Jan 2018 17:33:13 +0000 Subject: [PATCH] Fix segfault when using 'set print object on' + whatis <struct> Compiling GDB with a recent GCC exposes a problem: ../../gdb/typeprint.c: In function 'void whatis_exp(const char*, int)': ../../gdb/typeprint.c:515:12: warning: 'val' may be used uninitialized in this function [-Wmaybe-uninitialized] real_type = value_rtti_type (val, &full, &top, &using_enc); ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ The warning is correct. There are indeed code paths that use uninitialized 'val', leading to crashes. Inside the value_rtti_indirect_type/value_rtti_type calls here in whatis_exp: if (opts.objectprint) { if (((TYPE_CODE (type) == TYPE_CODE_PTR) || TYPE_IS_REFERENCE (type)) && (TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_STRUCT)) real_type = value_rtti_indirect_type (val, &full, &top, &using_enc); else if (TYPE_CODE (type) == TYPE_CODE_STRUCT) real_type = value_rtti_type (val, &full, &top, &using_enc); } We reach those calls above with "set print object on", and then with any of: (gdb) whatis struct some_structure_type (gdb) whatis struct some_structure_type * (gdb) whatis struct some_structure_type & because "whatis" with a type argument enters this branch: /* The behavior of "whatis" depends on whether the user expression names a type directly, or a language expression (including variable names). If the former, then "whatis" strips one level of typedefs, only. If an expression, "whatis" prints the type of the expression without stripping any typedef level. "ptype" always strips all levels of typedefs. */ if (show == -1 && expr->elts[0].opcode == OP_TYPE) { which does not initialize VAL. Trying the above lead to crashes like this: (gdb) set print object on (gdb) whatis some_structure_type Thread 1 "gdb" received signal SIGSEGV, Segmentation fault. 0x00000000005dda90 in check_typedef (type=0x6120736573756170) at src/gdb/gdbtypes.c:2388 2388 int instance_flags = TYPE_INSTANCE_FLAGS (type); ... This is a regression caused by a recent-ish refactoring of the code on 'whatis_exp', introduced by: commit c973d0aa4a2c737ab527ae44a617f1c357e07364 Date: Mon Aug 21 11:34:32 2017 +0100 Fix type casts losing typedefs and reimplement "whatis" typedef stripping Fix this by setting VAL to NULL in the "whatis TYPE" case, and skipping fetching the dynamic type if there's no value to fetch it from. New tests included. gdb/ChangeLog: yyyy-mm-dd Sergio Durigan Junior <sergiodj@redhat.com> Pedro Alves <palves@redhat.com> * typeprint.c (whatis_exp): Initialize "val" in the "whatis type" case. gdb/testsuite/ChangeLog: yyyy-mm-dd Sergio Durigan Junior <sergiodj@redhat.com> Pedro Alves <palves@redhat.com> * gdb.base/whatis.exp: Add tests for 'set print object on' + 'whatis <struct>' 'whatis <struct> *' and 'whatis <struct> &'. --- gdb/testsuite/gdb.base/whatis.exp | 25 +++++++++++++++++++++---- gdb/typeprint.c | 6 +++++- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/gdb/testsuite/gdb.base/whatis.exp b/gdb/testsuite/gdb.base/whatis.exp index dd6aeb02f91..509183e2ea4 100644 --- a/gdb/testsuite/gdb.base/whatis.exp +++ b/gdb/testsuite/gdb.base/whatis.exp @@ -282,14 +282,31 @@ gdb_test "whatis v_double_pointer" \ # test whatis command with structure types + +# First with a type argument, with both "set print object" set to "on" +# and "off", ending with "off" for the following tests. +foreach_with_prefix print_object {"on" "off"} { + gdb_test_no_output "set print object $print_object" + + gdb_test "whatis struct t_struct" \ + "type = struct t_struct" \ + "whatis named structure using type name" + + gdb_test "whatis struct t_struct *" \ + "type = struct t_struct \\*" \ + "whatis named structure using type name and pointer" + + gdb_test "whatis struct t_struct &" \ + "type = struct t_struct &" \ + "whatis named structure using type name and reference" +} + +# Now with an expression argument. + gdb_test "whatis v_struct1" \ "type = struct t_struct" \ "whatis named structure" -gdb_test "whatis struct t_struct" \ - "type = struct t_struct" \ - "whatis named structure using type name" - gdb_test "whatis v_struct2" \ "type = struct \{\.\.\.\}" \ "whatis unnamed structure" diff --git a/gdb/typeprint.c b/gdb/typeprint.c index 9a125076a1b..c098a3f4261 100644 --- a/gdb/typeprint.c +++ b/gdb/typeprint.c @@ -489,6 +489,10 @@ whatis_exp (const char *exp, int show) check_typedef (type); if (TYPE_CODE (type) == TYPE_CODE_TYPEDEF) type = TYPE_TARGET_TYPE (type); + + /* If the expression is actually a type, then there's no + value to fetch the dynamic type from. */ + val = NULL; } else { @@ -506,7 +510,7 @@ whatis_exp (const char *exp, int show) } get_user_print_options (&opts); - if (opts.objectprint) + if (val != NULL && opts.objectprint) { if (((TYPE_CODE (type) == TYPE_CODE_PTR) || TYPE_IS_REFERENCE (type)) && (TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_STRUCT)) -- 2.14.3 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2] Fix segfault when using 'set print object on' + whatis <struct> (Re: [PATCH] Fix segfault when using 'set print object on' + whatis <struct>) 2018-01-22 17:42 ` [PATCH v2] Fix segfault when using 'set print object on' + whatis <struct> (Re: [PATCH] Fix segfault when using 'set print object on' + whatis <struct>) Pedro Alves @ 2018-01-22 18:04 ` Sergio Durigan Junior 2018-01-22 19:53 ` Pedro Alves 0 siblings, 1 reply; 40+ messages in thread From: Sergio Durigan Junior @ 2018-01-22 18:04 UTC (permalink / raw) To: Pedro Alves; +Cc: GDB Patches, Eli Zaretskii On Monday, January 22 2018, Pedro Alves wrote: > Hi Sergio, Hey, /me prepares himself for the "this is completely wrong" e-mail > On 01/20/2018 01:03 AM, Sergio Durigan Junior wrote: >> This problem was hidden behind a "maybe-uninitialized" warning >> generated when compiling GDB with a recent GCC. The warning is: >> >> ../../gdb/typeprint.c: In function 'void whatis_exp(const char*, int)': >> ../../gdb/typeprint.c:515:12: warning: 'val' may be used uninitialized in this function [-Wmaybe-uninitialized] >> real_type = value_rtti_type (val, &full, &top, &using_enc); >> ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> >> I submitted a patch fixing this by initializing "val" to NULL, but it >> was the wrong fix, as Pedro pointed out on >> <https://sourceware.org/ml/gdb-patches/2018-01/msg00346.html>: > > IMHO, adding such history to the commit log directly doesn't really > add much here. It's better IMO to leave that to "what changed in v2" > comments (that don't go in the commit log), and instead update > the commit log to go straight to the point. (please keep reading.) OK. I personally think that it's good to document the actions and reactions that led me to writing this patch, but this is a small detail. >> >> (gdb) set print object on >> (gdb) whatis some_structure_type >> >> Thread 1 "gdb" received signal SIGSEGV, Segmentation fault. >> 0x00000000005dda90 in check_typedef (type=0x6120736573756170) at src/gdb/gdbtypes.c:2388 >> 2388 int instance_flags = TYPE_INSTANCE_FLAGS (type); >> ... >> >> So I set off to find the cause of the problem. It turns out that a >> recent-ish refactoring of the code on 'whatis_exp', introduced by: >> >> commit c973d0aa4a2c737ab527ae44a617f1c357e07364 >> Date: Mon Aug 21 11:34:32 2017 +0100 >> >> Fix type casts losing typedefs and reimplement "whatis" typedef stripping >> >> was the reason of the failure. After investigating what 'set print >> object on' was supposed to do to the output of 'whatis', if made sense >> initialize "val = evaluate_type (expr.get ());" all the time, not only >> when we're dealing with the 'ptype' command. > > The point of the c973d0aa4a2c change was to bypass evaluating the > expression if the opcode is OP_TYPE. The proposed patch adds back > the evaluation, while the OP_TYPE shortcut is left in there too. I > think calling allocate_value instead in the path that misses > initializing 'val' would make more sense, to end up with a dummy > not_lval value, like expression evaluation does when evaluating > EVAL_AVOID_SIDE_EFFECTS. But even better still is to set VAL to NULL > in this case (only), and skip the "real type" printing. It doesn't make > any sense to try to fetch the dynamic type of a type that didn't come > from an actual program value in the first place. There's nothing > dynamic about a statically named type. Ah, OK. Thanks for explaining. I wasn't really sure about the right fix here, and somehow I felt like I was missing more context to make an informed decision on what the right fix was. That's why I asked (internally) what "whatis" is supposed to print when 'set print object' is on. Anyway, now it makes more sense. >> >> I've regtested this on the BuildBot, without seeing any regressions. >> I've also extended 'gdb.base/whatis.exp' to check if the segfault is >> not there anymore. > > The "whatis struct" case was an example, but there's another similar > path in the code that also lead to a crash that is not covered by the > testcase. The value_rtti_indirect_type path here: > > if (opts.objectprint) > { > if (((TYPE_CODE (type) == TYPE_CODE_PTR) || TYPE_IS_REFERENCE (type)) > && (TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_STRUCT)) > real_type = value_rtti_indirect_type (val, &full, &top, &using_enc); > else if (TYPE_CODE (type) == TYPE_CODE_STRUCT) > real_type = value_rtti_type (val, &full, &top, &using_enc); > } Right, I should have added one more test to cover this case as well, sorry. >> --- a/gdb/testsuite/gdb.base/whatis.exp >> +++ b/gdb/testsuite/gdb.base/whatis.exp >> @@ -566,3 +566,10 @@ gdb_test "whatis int (*)(void, int, int)" \ >> gdb_test "whatis int (*)(int, void, int)" \ >> "'void' invalid as parameter type" \ >> "whatis applied to function with 'void' parameter type" >> + >> +# Test that 'set print object on' + whatis doesn't segfault. >> +clean_restart $binfile >> +gdb_test_no_output "set print object on" >> +gdb_test "whatis v_struct1" \ >> + "type = struct t_struct" \ >> + "whatis + set print object on doesn't segfault" > > At some point later on someone is going to read this test > name/message and wonder why is it talking about a segfault. > All the other 50k+ tests in the testsuite are indirectly checking > that gdb doesn't segfault either. This one's not that special > in that regard. Instead, we're testing that the > "set print object on" + "whatis <struct>" combination _works_ as > expected, which among other things obviously includes > not segfaulting, but also includes checking that the output > is reasonable. > > As I was writing/experimenting the above, I ended up addressing > my own comments. What do you think of this patch instead? The patch is yours now, so you should be listed as the author, and you will probably want to rewrite the subject line to make it more correct. Other than that, and after your e-mail, I don't really have the expertise to criticize anything. Thanks for explaining things and taking care of v2. > New in v2: > > - set val to NULL and skip dynamic type printing in the "whatis <type>" > case. > - rewrite the new tests to reuse preexisting tests, and add new tests > for whatis with pointers and references to structs coupled with > "set print object on". Try both "set print object on/off". > > From f86b4087d23be4cf20bf3dc2f8407cef3f28830d Mon Sep 17 00:00:00 2001 > From: Sergio Durigan Junior <sergiodj@redhat.com> > Date: Mon, 22 Jan 2018 17:33:13 +0000 > Subject: [PATCH] Fix segfault when using 'set print object on' + whatis > <struct> > > Compiling GDB with a recent GCC exposes a problem: > > ../../gdb/typeprint.c: In function 'void whatis_exp(const char*, int)': > ../../gdb/typeprint.c:515:12: warning: 'val' may be used uninitialized in this function [-Wmaybe-uninitialized] > real_type = value_rtti_type (val, &full, &top, &using_enc); > ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > The warning is correct. There are indeed code paths that use > uninitialized 'val', leading to crashes. Inside the > value_rtti_indirect_type/value_rtti_type calls here in whatis_exp: > > if (opts.objectprint) > { > if (((TYPE_CODE (type) == TYPE_CODE_PTR) || TYPE_IS_REFERENCE (type)) > && (TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_STRUCT)) > real_type = value_rtti_indirect_type (val, &full, &top, &using_enc); > else if (TYPE_CODE (type) == TYPE_CODE_STRUCT) > real_type = value_rtti_type (val, &full, &top, &using_enc); > } > > We reach those calls above with "set print object on", and then with > any of: > > (gdb) whatis struct some_structure_type > (gdb) whatis struct some_structure_type * > (gdb) whatis struct some_structure_type & > > because "whatis" with a type argument enters this branch: > > /* The behavior of "whatis" depends on whether the user > expression names a type directly, or a language expression > (including variable names). If the former, then "whatis" > strips one level of typedefs, only. If an expression, > "whatis" prints the type of the expression without stripping > any typedef level. "ptype" always strips all levels of > typedefs. */ > if (show == -1 && expr->elts[0].opcode == OP_TYPE) > { > > which does not initialize VAL. Trying the above lead to crashes like > this: > > (gdb) set print object on > (gdb) whatis some_structure_type > > Thread 1 "gdb" received signal SIGSEGV, Segmentation fault. > 0x00000000005dda90 in check_typedef (type=0x6120736573756170) at src/gdb/gdbtypes.c:2388 > 2388 int instance_flags = TYPE_INSTANCE_FLAGS (type); > ... > > This is a regression caused by a recent-ish refactoring of the code on > 'whatis_exp', introduced by: > > commit c973d0aa4a2c737ab527ae44a617f1c357e07364 > Date: Mon Aug 21 11:34:32 2017 +0100 > > Fix type casts losing typedefs and reimplement "whatis" typedef stripping > > Fix this by setting VAL to NULL in the "whatis TYPE" case, and > skipping fetching the dynamic type if there's no value to fetch it > from. > > New tests included. > > gdb/ChangeLog: > yyyy-mm-dd Sergio Durigan Junior <sergiodj@redhat.com> > Pedro Alves <palves@redhat.com> > > * typeprint.c (whatis_exp): Initialize "val" in the "whatis type" > case. > > gdb/testsuite/ChangeLog: > yyyy-mm-dd Sergio Durigan Junior <sergiodj@redhat.com> > Pedro Alves <palves@redhat.com> > > * gdb.base/whatis.exp: Add tests for 'set print object on' + > 'whatis <struct>' 'whatis <struct> *' and 'whatis <struct> &'. > --- > gdb/testsuite/gdb.base/whatis.exp | 25 +++++++++++++++++++++---- > gdb/typeprint.c | 6 +++++- > 2 files changed, 26 insertions(+), 5 deletions(-) > > diff --git a/gdb/testsuite/gdb.base/whatis.exp b/gdb/testsuite/gdb.base/whatis.exp > index dd6aeb02f91..509183e2ea4 100644 > --- a/gdb/testsuite/gdb.base/whatis.exp > +++ b/gdb/testsuite/gdb.base/whatis.exp > @@ -282,14 +282,31 @@ gdb_test "whatis v_double_pointer" \ > > > # test whatis command with structure types > + > +# First with a type argument, with both "set print object" set to "on" > +# and "off", ending with "off" for the following tests. > +foreach_with_prefix print_object {"on" "off"} { > + gdb_test_no_output "set print object $print_object" > + > + gdb_test "whatis struct t_struct" \ > + "type = struct t_struct" \ > + "whatis named structure using type name" > + > + gdb_test "whatis struct t_struct *" \ > + "type = struct t_struct \\*" \ > + "whatis named structure using type name and pointer" > + > + gdb_test "whatis struct t_struct &" \ > + "type = struct t_struct &" \ > + "whatis named structure using type name and reference" > +} > + > +# Now with an expression argument. > + > gdb_test "whatis v_struct1" \ > "type = struct t_struct" \ > "whatis named structure" > > -gdb_test "whatis struct t_struct" \ > - "type = struct t_struct" \ > - "whatis named structure using type name" > - > gdb_test "whatis v_struct2" \ > "type = struct \{\.\.\.\}" \ > "whatis unnamed structure" > diff --git a/gdb/typeprint.c b/gdb/typeprint.c > index 9a125076a1b..c098a3f4261 100644 > --- a/gdb/typeprint.c > +++ b/gdb/typeprint.c > @@ -489,6 +489,10 @@ whatis_exp (const char *exp, int show) > check_typedef (type); > if (TYPE_CODE (type) == TYPE_CODE_TYPEDEF) > type = TYPE_TARGET_TYPE (type); > + > + /* If the expression is actually a type, then there's no > + value to fetch the dynamic type from. */ > + val = NULL; > } > else > { > @@ -506,7 +510,7 @@ whatis_exp (const char *exp, int show) > } > > get_user_print_options (&opts); > - if (opts.objectprint) > + if (val != NULL && opts.objectprint) > { > if (((TYPE_CODE (type) == TYPE_CODE_PTR) || TYPE_IS_REFERENCE (type)) > && (TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_STRUCT)) > -- > 2.14.3 -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/ ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2] Fix segfault when using 'set print object on' + whatis <struct> (Re: [PATCH] Fix segfault when using 'set print object on' + whatis <struct>) 2018-01-22 18:04 ` Sergio Durigan Junior @ 2018-01-22 19:53 ` Pedro Alves 2018-01-22 20:11 ` Sergio Durigan Junior 0 siblings, 1 reply; 40+ messages in thread From: Pedro Alves @ 2018-01-22 19:53 UTC (permalink / raw) To: Sergio Durigan Junior; +Cc: GDB Patches, Eli Zaretskii On 01/22/2018 06:04 PM, Sergio Durigan Junior wrote: > On Monday, January 22 2018, Pedro Alves wrote: > > /me prepares himself for the "this is completely wrong" e-mail > :-) >> On 01/20/2018 01:03 AM, Sergio Durigan Junior wrote: >>> This problem was hidden behind a "maybe-uninitialized" warning >>> generated when compiling GDB with a recent GCC. The warning is: >>> >>> ../../gdb/typeprint.c: In function 'void whatis_exp(const char*, int)': >>> ../../gdb/typeprint.c:515:12: warning: 'val' may be used uninitialized in this function [-Wmaybe-uninitialized] >>> real_type = value_rtti_type (val, &full, &top, &using_enc); >>> ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >>> >>> I submitted a patch fixing this by initializing "val" to NULL, but it >>> was the wrong fix, as Pedro pointed out on >>> <https://sourceware.org/ml/gdb-patches/2018-01/msg00346.html>: >> >> IMHO, adding such history to the commit log directly doesn't really >> add much here. It's better IMO to leave that to "what changed in v2" >> comments (that don't go in the commit log), and instead update >> the commit log to go straight to the point. (please keep reading.) > > OK. I personally think that it's good to document the actions and > reactions that led me to writing this patch, but this is a small detail. It's good to describe approaches that are seemingly-obvious-but-then-don't-actually-work. That spares the reader/reviewer from going through the same thought process, and/or immediately shooting back with a "why didn't you do it in the other seemingly obvious way" question. In this case, just explaining that the warning is valid obviously switches the previous approach to the "really-obviously-not-correct/incomplete" department, so there's no need to explain it. Commit messages are meant to help future readers understand why changes were made, and being concise and to the point helps. The thing that led to the discovery of the issue is the GCC warning, and that is still mentioned. > Right, I should have added one more test to cover this case as well, > sorry. No worries. This is team work. :-) >> >> As I was writing/experimenting the above, I ended up addressing >> my own comments. What do you think of this patch instead? > > The patch is yours now, so you should be listed as the author, and you > will probably want to rewrite the subject line to make it more correct. > Other than that, and after your e-mail, I don't really have the > expertise to criticize anything. I don't mind either way, but I've pushed the patch in respecting that. And now I realize that this should be needed on the 8.1 branch too. I'll take care of that. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2] Fix segfault when using 'set print object on' + whatis <struct> (Re: [PATCH] Fix segfault when using 'set print object on' + whatis <struct>) 2018-01-22 19:53 ` Pedro Alves @ 2018-01-22 20:11 ` Sergio Durigan Junior 0 siblings, 0 replies; 40+ messages in thread From: Sergio Durigan Junior @ 2018-01-22 20:11 UTC (permalink / raw) To: Pedro Alves; +Cc: GDB Patches, Eli Zaretskii On Monday, January 22 2018, Pedro Alves wrote: >>> On 01/20/2018 01:03 AM, Sergio Durigan Junior wrote: >>>> This problem was hidden behind a "maybe-uninitialized" warning >>>> generated when compiling GDB with a recent GCC. The warning is: >>>> >>>> ../../gdb/typeprint.c: In function 'void whatis_exp(const char*, int)': >>>> ../../gdb/typeprint.c:515:12: warning: 'val' may be used uninitialized in this function [-Wmaybe-uninitialized] >>>> real_type = value_rtti_type (val, &full, &top, &using_enc); >>>> ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >>>> >>>> I submitted a patch fixing this by initializing "val" to NULL, but it >>>> was the wrong fix, as Pedro pointed out on >>>> <https://sourceware.org/ml/gdb-patches/2018-01/msg00346.html>: >>> >>> IMHO, adding such history to the commit log directly doesn't really >>> add much here. It's better IMO to leave that to "what changed in v2" >>> comments (that don't go in the commit log), and instead update >>> the commit log to go straight to the point. (please keep reading.) >> >> OK. I personally think that it's good to document the actions and >> reactions that led me to writing this patch, but this is a small detail. > > It's good to describe approaches that are > seemingly-obvious-but-then-don't-actually-work. That spares the > reader/reviewer from going through the same thought process, and/or > immediately shooting back with a "why didn't you do it in the other > seemingly obvious way" question. In this case, just explaining that > the warning is valid obviously switches the previous approach to > the "really-obviously-not-correct/incomplete" department, so there's > no need to explain it. Commit messages are meant to help future > readers understand why changes were made, and being concise and to > the point helps. The thing that led to the discovery of the issue > is the GCC warning, and that is still mentioned. I understand your point. To the experienced reader, the final commit message (written by you) is indeed better because it removes the previous attempt I made to fix this issue by just initializing VAL to NULL. Since I was the one who made the mistake of not fully understanding the consequences of the warning, I thought it made sense to explain everything, from the very beginning, when I thought this was just a silly warning) to the point where you pointed me to the real bug. While this commit message made sense to me, and can probably be considered "too much" for the experienced reader, I still think it would be a nice addition. But I don't want to create a confusion/mess/issue out of this specific part, so I'm totally fine with your final message. >>> As I was writing/experimenting the above, I ended up addressing >>> my own comments. What do you think of this patch instead? >> >> The patch is yours now, so you should be listed as the author, and you >> will probably want to rewrite the subject line to make it more correct. >> Other than that, and after your e-mail, I don't really have the >> expertise to criticize anything. > > I don't mind either way, but I've pushed the patch in respecting that. > > And now I realize that this should be needed on the 8.1 branch too. > I'll take care of that. Yes, thanks for doing, I was going to send a message reminding you about the 8.1 branch. Thanks, -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/ ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [ANNOUNCEMENT] GDB 8.1 release branch created! 2018-01-16 19:02 ` Sergio Durigan Junior 2018-01-16 19:46 ` [PATCH] Fix warning on gdb/compile/compile.c (C++-ify "triplet_rx") Sergio Durigan Junior 2018-01-16 20:32 ` [PATCH] Fix unitialized warning on gdb/typeprint.c:whatis_exp Sergio Durigan Junior @ 2018-01-16 20:36 ` Sergio Durigan Junior 2018-01-17 3:36 ` Eli Zaretskii 2018-01-17 11:04 ` Pedro Alves 2 siblings, 2 replies; 40+ messages in thread From: Sergio Durigan Junior @ 2018-01-16 20:36 UTC (permalink / raw) To: Eli Zaretskii; +Cc: gdb-patches On Tuesday, January 16 2018, I wrote: >> g++ -x c++ -O2 -gdwarf-4 -g3 -I. -I. -I./common -I./config -DLOCALEDIR="\"d:/usr/share/locale\"" -DHAVE_CONFIG_H -I./../include/opcode -I./../opcodes/.. -I./../readline/.. -I../bfd -I./../bfd -I./../include -I../libdecnumber -I./../libdecnumber -I./gnulib/import -Ibuild-gnulib/import -DTUI=1 -Id:/usr/include -Id:/usr/include/guile/2.0 -Id:/usr/include -Id:/usr/Python26/include -Id:/usr/Python26/include -Wall -Wpointer-arith -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable -Wno-sign-compare -Wno-narrowing -Wno-error=maybe-uninitialized -Wno-format -c -o infrun.o -MT infrun.o -MMD -MP -MF ./.deps/infrun.Tpo infrun.c >> In file included from infrun.c:26:0: >> inferior.h: In function 'void handle_vfork_child_exec_or_exit(int)': >> inferior.h:537:39: warning: '*((void*)(& maybe_restore_inferior)+20).scoped_restore_current_inferior::m_saved_inf' may be used uninitialized in this function [-Wmaybe-uninitialized] >> { set_current_inferior (m_saved_inf); } >> ^ >> infrun.c:927:6: note: '*((void*)(& maybe_restore_inferior)+20).scoped_restore_current_inferior::m_saved_inf' was declared here >> maybe_restore_inferior; >> ^~~~~~~~~~~~~~~~~~~~~~ >> In file included from inferior.h:48:0, >> from infrun.c:26: >> progspace.h:285:47: warning: '*((void*)(& maybe_restore_inferior)+16).scoped_restore_current_program_space::m_saved_pspace' may be used uninitialized in this function [-Wmaybe-uninitialized] >> { set_current_program_space (m_saved_pspace); } >> ^ >> infrun.c:927:6: note: '*((void*)(& maybe_restore_inferior)+16).scoped_restore_current_program_space::m_saved_pspace' was declared here >> maybe_restore_inferior; >> ^~~~~~~~~~~~~~~~~~~~~~ I'm not sure why this one is happening. I think it has something to do with the fact that we're declaring "maybe_restore_inferior" as gdb::optional, because scoped_restore_current_inferior's constructor already takes care of initializing "m_saved_inf" (same goes for scoped_restore_current_program_space). If that is the case, then maybe we can mark the "uninitialized variable" warning for "m_saved_inf" and "m_saved_pspace". WDYT? -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/ ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [ANNOUNCEMENT] GDB 8.1 release branch created! 2018-01-16 20:36 ` [ANNOUNCEMENT] GDB 8.1 release branch created! Sergio Durigan Junior @ 2018-01-17 3:36 ` Eli Zaretskii 2018-01-17 16:46 ` Sergio Durigan Junior 2018-01-17 11:04 ` Pedro Alves 1 sibling, 1 reply; 40+ messages in thread From: Eli Zaretskii @ 2018-01-17 3:36 UTC (permalink / raw) To: Sergio Durigan Junior; +Cc: gdb-patches > From: Sergio Durigan Junior <sergiodj@redhat.com> > Cc: gdb-patches@sourceware.org > Date: Tue, 16 Jan 2018 15:36:22 -0500 > > I'm not sure why this one is happening. I think it has something to do > with the fact that we're declaring "maybe_restore_inferior" as > gdb::optional, because scoped_restore_current_inferior's constructor > already takes care of initializing "m_saved_inf" (same goes for > scoped_restore_current_program_space). > > If that is the case, then maybe we can mark the "uninitialized variable" > warning for "m_saved_inf" and "m_saved_pspace". Sorry, I'm not sure I understand what does your suggestion here mean in practice. Can you elaborate, or show a proposed patch? Thanks. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [ANNOUNCEMENT] GDB 8.1 release branch created! 2018-01-17 3:36 ` Eli Zaretskii @ 2018-01-17 16:46 ` Sergio Durigan Junior 0 siblings, 0 replies; 40+ messages in thread From: Sergio Durigan Junior @ 2018-01-17 16:46 UTC (permalink / raw) To: Eli Zaretskii; +Cc: gdb-patches On Tuesday, January 16 2018, Eli Zaretskii wrote: >> From: Sergio Durigan Junior <sergiodj@redhat.com> >> Cc: gdb-patches@sourceware.org >> Date: Tue, 16 Jan 2018 15:36:22 -0500 >> >> I'm not sure why this one is happening. I think it has something to do >> with the fact that we're declaring "maybe_restore_inferior" as >> gdb::optional, because scoped_restore_current_inferior's constructor >> already takes care of initializing "m_saved_inf" (same goes for >> scoped_restore_current_program_space). >> >> If that is the case, then maybe we can mark the "uninitialized variable" >> warning for "m_saved_inf" and "m_saved_pspace". > > Sorry, I'm not sure I understand what does your suggestion here mean > in practice. Can you elaborate, or show a proposed patch? Eli, with Pedro's reply, mentioning: https://sourceware.org/ml/gdb-patches/2017-05/msg00130.html I think there's not much we can do about the warning. Thanks, -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/ ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [ANNOUNCEMENT] GDB 8.1 release branch created! 2018-01-16 20:36 ` [ANNOUNCEMENT] GDB 8.1 release branch created! Sergio Durigan Junior 2018-01-17 3:36 ` Eli Zaretskii @ 2018-01-17 11:04 ` Pedro Alves 2018-01-17 16:38 ` Sergio Durigan Junior 1 sibling, 1 reply; 40+ messages in thread From: Pedro Alves @ 2018-01-17 11:04 UTC (permalink / raw) To: Sergio Durigan Junior, Eli Zaretskii; +Cc: gdb-patches On 01/16/2018 08:36 PM, Sergio Durigan Junior wrote: > I'm not sure why this one is happening. I think it has something to do > with the fact that we're declaring "maybe_restore_inferior" as > gdb::optional, because scoped_restore_current_inferior's constructor > already takes care of initializing "m_saved_inf" (same goes for > scoped_restore_current_program_space). https://sourceware.org/ml/gdb-patches/2017-05/msg00130.html Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [ANNOUNCEMENT] GDB 8.1 release branch created! 2018-01-17 11:04 ` Pedro Alves @ 2018-01-17 16:38 ` Sergio Durigan Junior 2018-01-17 16:46 ` Eli Zaretskii 0 siblings, 1 reply; 40+ messages in thread From: Sergio Durigan Junior @ 2018-01-17 16:38 UTC (permalink / raw) To: Pedro Alves; +Cc: Eli Zaretskii, gdb-patches On Wednesday, January 17 2018, Pedro Alves wrote: > On 01/16/2018 08:36 PM, Sergio Durigan Junior wrote: > >> I'm not sure why this one is happening. I think it has something to do >> with the fact that we're declaring "maybe_restore_inferior" as >> gdb::optional, because scoped_restore_current_inferior's constructor >> already takes care of initializing "m_saved_inf" (same goes for >> scoped_restore_current_program_space). > > https://sourceware.org/ml/gdb-patches/2017-05/msg00130.html Ah, there you go. Thanks, Pedro. I had missed that exchange of messages. -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/ ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [ANNOUNCEMENT] GDB 8.1 release branch created! 2018-01-17 16:38 ` Sergio Durigan Junior @ 2018-01-17 16:46 ` Eli Zaretskii 2018-01-17 16:50 ` Pedro Alves 0 siblings, 1 reply; 40+ messages in thread From: Eli Zaretskii @ 2018-01-17 16:46 UTC (permalink / raw) To: Sergio Durigan Junior; +Cc: palves, gdb-patches > From: Sergio Durigan Junior <sergiodj@redhat.com> > Cc: Eli Zaretskii <eliz@gnu.org>, gdb-patches@sourceware.org > Date: Wed, 17 Jan 2018 11:38:08 -0500 > > > https://sourceware.org/ml/gdb-patches/2017-05/msg00130.html > > Ah, there you go. Thanks, Pedro. I had missed that exchange of > messages. How about using GCC pragma to shut up this warning in that particular place? ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [ANNOUNCEMENT] GDB 8.1 release branch created! 2018-01-17 16:46 ` Eli Zaretskii @ 2018-01-17 16:50 ` Pedro Alves 2018-01-17 18:21 ` Eli Zaretskii 0 siblings, 1 reply; 40+ messages in thread From: Pedro Alves @ 2018-01-17 16:50 UTC (permalink / raw) To: Eli Zaretskii, Sergio Durigan Junior; +Cc: gdb-patches On 01/17/2018 04:46 PM, Eli Zaretskii wrote: >> From: Sergio Durigan Junior <sergiodj@redhat.com> >> Cc: Eli Zaretskii <eliz@gnu.org>, gdb-patches@sourceware.org >> Date: Wed, 17 Jan 2018 11:38:08 -0500 >> >>> https://sourceware.org/ml/gdb-patches/2017-05/msg00130.html >> >> Ah, there you go. Thanks, Pedro. I had missed that exchange of >> messages. > > How about using GCC pragma to shut up this warning in that particular > place? IIRC, last I tried, it make the code exceedingly ugly. See: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635#c9 Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [ANNOUNCEMENT] GDB 8.1 release branch created! 2018-01-17 16:50 ` Pedro Alves @ 2018-01-17 18:21 ` Eli Zaretskii 0 siblings, 0 replies; 40+ messages in thread From: Eli Zaretskii @ 2018-01-17 18:21 UTC (permalink / raw) To: Pedro Alves; +Cc: sergiodj, gdb-patches > Cc: gdb-patches@sourceware.org > From: Pedro Alves <palves@redhat.com> > Date: Wed, 17 Jan 2018 16:49:58 +0000 > > > How about using GCC pragma to shut up this warning in that particular > > place? > IIRC, last I tried, it make the code exceedingly ugly. See: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635#c9 It doesn't look too ugly to me, FWIW. Its advantage is that it documents exactly what is the problem being worked around here, and makes finding the affected places easy. By contrast, leaving the warnings around and keeping the information about the problem in some mailing list posting makes its discoverability much lower, and the probability that someone else will spend time trying to understand it higher. And the warning is quite cryptic, at least to my naïve eyes. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [ANNOUNCEMENT] GDB 8.1 release branch created! 2018-01-16 17:31 ` [ANNOUNCEMENT] GDB 8.1 release branch created! Eli Zaretskii 2018-01-16 19:02 ` Sergio Durigan Junior @ 2018-01-18 15:53 ` Eli Zaretskii 2018-01-25 16:58 ` Eli Zaretskii 1 sibling, 1 reply; 40+ messages in thread From: Eli Zaretskii @ 2018-01-18 15:53 UTC (permalink / raw) To: gdb-patches Ping! Is it OK to install the patch proposed below? > Date: Tue, 16 Jan 2018 19:31:37 +0200 > From: Eli Zaretskii <eliz@gnu.org> > > g++ -x c++ -O2 -gdwarf-4 -g3 -I. -I. -I./common -I./config -DLOCALEDIR="\"d:/usr/share/locale\"" -DHAVE_CONFIG_H -I./../include/opcode -I./../opcodes/.. -I./../readline/.. -I../bfd -I./../bfd -I./../include -I../libdecnumber -I./../libdecnumber -I./gnulib/import -Ibuild-gnulib/import -DTUI=1 -Id:/usr/include -Id:/usr/include/guile/2.0 -Id:/usr/include -Id:/usr/Python26/include -Id:/usr/Python26/include -Wall -Wpointer-arith -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable -Wno-sign-compare -Wno-narrowing -Wno-error=maybe-uninitialized -Wno-format -fno-strict-aliasing -DNDEBUG -fwrapv -c -o python/py-arch.o -MT python/py-arch.o -MMD -MP -MF python/.deps/py-arch.Tpo python/py-arch.c > In file included from d:\usr\lib\gcc\mingw32\6.3.0\include\c++\math.h:36:0, > from build-gnulib/import/math.h:27, > from d:/usr/Python26/include/pyport.h:235, > from d:/usr/Python26/include/Python.h:58, > from python/python-internal.h:94, > from python/py-arch.c:24: > d:\usr\lib\gcc\mingw32\6.3.0\include\c++\cmath:1157:11: error: '::hypot' has not been declared > using ::hypot; > ^~~~~ > Makefile:1618: recipe for target `python/py-arch.o' failed > > Googling suggests the following solution; is it okay to push this > (with the necessary logs and after doing the "paperwork" required for > branch changes)? Or does someone have better ideas? (Does this work > in MinGW64?) > > --- gdb/python/python-internal.h~0 2018-01-12 05:31:04.000000000 +0200 > +++ gdb/python/python-internal.h 2018-01-16 08:56:10.717759900 +0200 > @@ -85,6 +85,12 @@ > #define HAVE_SNPRINTF 1 > #endif > > +/* Another kludge to avoid compilation errors because MinGW defines > + 'hypot' to '_hypot', but the C++ headers says "using ::hypot". */ > +#if defined(__MINGW32__) && defined(__cplusplus) > +# define _hypot hypot > +#endif > + > /* Request clean size types from Python. */ > #define PY_SSIZE_T_CLEAN > > ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [ANNOUNCEMENT] GDB 8.1 release branch created! 2018-01-18 15:53 ` Eli Zaretskii @ 2018-01-25 16:58 ` Eli Zaretskii 2018-01-26 14:18 ` Eli Zaretskii 0 siblings, 1 reply; 40+ messages in thread From: Eli Zaretskii @ 2018-01-25 16:58 UTC (permalink / raw) To: gdb-patches > Date: Thu, 18 Jan 2018 17:52:53 +0200 > From: Eli Zaretskii <eliz@gnu.org> Ping! Ping! OK to push this? > Ping! Is it OK to install the patch proposed below? > > > Date: Tue, 16 Jan 2018 19:31:37 +0200 > > From: Eli Zaretskii <eliz@gnu.org> > > > > g++ -x c++ -O2 -gdwarf-4 -g3 -I. -I. -I./common -I./config -DLOCALEDIR="\"d:/usr/share/locale\"" -DHAVE_CONFIG_H -I./../include/opcode -I./../opcodes/.. -I./../readline/.. -I../bfd -I./../bfd -I./../include -I../libdecnumber -I./../libdecnumber -I./gnulib/import -Ibuild-gnulib/import -DTUI=1 -Id:/usr/include -Id:/usr/include/guile/2.0 -Id:/usr/include -Id:/usr/Python26/include -Id:/usr/Python26/include -Wall -Wpointer-arith -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable -Wno-sign-compare -Wno-narrowing -Wno-error=maybe-uninitialized -Wno-format -fno-strict-aliasing -DNDEBUG -fwrapv -c -o python/py-arch.o -MT python/py-arch.o -MMD -MP -MF python/.deps/py-arch.Tpo python/py-arch.c > > In file included from d:\usr\lib\gcc\mingw32\6.3.0\include\c++\math.h:36:0, > > from build-gnulib/import/math.h:27, > > from d:/usr/Python26/include/pyport.h:235, > > from d:/usr/Python26/include/Python.h:58, > > from python/python-internal.h:94, > > from python/py-arch.c:24: > > d:\usr\lib\gcc\mingw32\6.3.0\include\c++\cmath:1157:11: error: '::hypot' has not been declared > > using ::hypot; > > ^~~~~ > > Makefile:1618: recipe for target `python/py-arch.o' failed > > > > Googling suggests the following solution; is it okay to push this > > (with the necessary logs and after doing the "paperwork" required for > > branch changes)? Or does someone have better ideas? (Does this work > > in MinGW64?) > > > > --- gdb/python/python-internal.h~0 2018-01-12 05:31:04.000000000 +0200 > > +++ gdb/python/python-internal.h 2018-01-16 08:56:10.717759900 +0200 > > @@ -85,6 +85,12 @@ > > #define HAVE_SNPRINTF 1 > > #endif > > > > +/* Another kludge to avoid compilation errors because MinGW defines > > + 'hypot' to '_hypot', but the C++ headers says "using ::hypot". */ > > +#if defined(__MINGW32__) && defined(__cplusplus) > > +# define _hypot hypot > > +#endif > > + > > /* Request clean size types from Python. */ > > #define PY_SSIZE_T_CLEAN > > > > > ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [ANNOUNCEMENT] GDB 8.1 release branch created! 2018-01-25 16:58 ` Eli Zaretskii @ 2018-01-26 14:18 ` Eli Zaretskii 2018-01-26 15:37 ` Simon Marchi 0 siblings, 1 reply; 40+ messages in thread From: Eli Zaretskii @ 2018-01-26 14:18 UTC (permalink / raw) To: gdb-patches Ping! Ping! Ping! This is delaying the release of GDB 8.1. Should I just stop waiting for approval and push this? > Date: Thu, 25 Jan 2018 18:58:30 +0200 > From: Eli Zaretskii <eliz@gnu.org> > > > Date: Thu, 18 Jan 2018 17:52:53 +0200 > > From: Eli Zaretskii <eliz@gnu.org> > > Ping! Ping! OK to push this? > > > Ping! Is it OK to install the patch proposed below? > > > > > Date: Tue, 16 Jan 2018 19:31:37 +0200 > > > From: Eli Zaretskii <eliz@gnu.org> > > > > > > g++ -x c++ -O2 -gdwarf-4 -g3 -I. -I. -I./common -I./config -DLOCALEDIR="\"d:/usr/share/locale\"" -DHAVE_CONFIG_H -I./../include/opcode -I./../opcodes/.. -I./../readline/.. -I../bfd -I./../bfd -I./../include -I../libdecnumber -I./../libdecnumber -I./gnulib/import -Ibuild-gnulib/import -DTUI=1 -Id:/usr/include -Id:/usr/include/guile/2.0 -Id:/usr/include -Id:/usr/Python26/include -Id:/usr/Python26/include -Wall -Wpointer-arith -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable -Wno-sign-compare -Wno-narrowing -Wno-error=maybe-uninitialized -Wno-format -fno-strict-aliasing -DNDEBUG -fwrapv -c -o python/py-arch.o -MT python/py-arch.o -MMD -MP -MF python/.deps/py-arch.Tpo python/py-arch.c > > > In file included from d:\usr\lib\gcc\mingw32\6.3.0\include\c++\math.h:36:0, > > > from build-gnulib/import/math.h:27, > > > from d:/usr/Python26/include/pyport.h:235, > > > from d:/usr/Python26/include/Python.h:58, > > > from python/python-internal.h:94, > > > from python/py-arch.c:24: > > > d:\usr\lib\gcc\mingw32\6.3.0\include\c++\cmath:1157:11: error: '::hypot' has not been declared > > > using ::hypot; > > > ^~~~~ > > > Makefile:1618: recipe for target `python/py-arch.o' failed > > > > > > Googling suggests the following solution; is it okay to push this > > > (with the necessary logs and after doing the "paperwork" required for > > > branch changes)? Or does someone have better ideas? (Does this work > > > in MinGW64?) > > > > > > --- gdb/python/python-internal.h~0 2018-01-12 05:31:04.000000000 +0200 > > > +++ gdb/python/python-internal.h 2018-01-16 08:56:10.717759900 +0200 > > > @@ -85,6 +85,12 @@ > > > #define HAVE_SNPRINTF 1 > > > #endif > > > > > > +/* Another kludge to avoid compilation errors because MinGW defines > > > + 'hypot' to '_hypot', but the C++ headers says "using ::hypot". */ > > > +#if defined(__MINGW32__) && defined(__cplusplus) > > > +# define _hypot hypot > > > +#endif > > > + > > > /* Request clean size types from Python. */ > > > #define PY_SSIZE_T_CLEAN > > > > > > > > > ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [ANNOUNCEMENT] GDB 8.1 release branch created! 2018-01-26 14:18 ` Eli Zaretskii @ 2018-01-26 15:37 ` Simon Marchi 2018-01-26 18:53 ` Eli Zaretskii 0 siblings, 1 reply; 40+ messages in thread From: Simon Marchi @ 2018-01-26 15:37 UTC (permalink / raw) To: Eli Zaretskii, gdb-patches On 2018-01-26 09:18 AM, Eli Zaretskii wrote: > Ping! Ping! Ping! This is delaying the release of GDB 8.1. > > Should I just stop waiting for approval and push this? Hi Eli, I didn't reply to your pings because I don't feel qualified to review changes related to mingw (you are probably the most qualified person here, since you use it). Given the lack of response, I'd say you can go ahead with the change, tt seems relatively safe. I have one question below. >> Date: Thu, 25 Jan 2018 18:58:30 +0200 >> From: Eli Zaretskii <eliz@gnu.org> >> >>> Date: Thu, 18 Jan 2018 17:52:53 +0200 >>> From: Eli Zaretskii <eliz@gnu.org> >> >> Ping! Ping! OK to push this? >> >>> Ping! Is it OK to install the patch proposed below? >>> >>>> Date: Tue, 16 Jan 2018 19:31:37 +0200 >>>> From: Eli Zaretskii <eliz@gnu.org> >>>> >>>> g++ -x c++ -O2 -gdwarf-4 -g3 -I. -I. -I./common -I./config -DLOCALEDIR="\"d:/usr/share/locale\"" -DHAVE_CONFIG_H -I./../include/opcode -I./../opcodes/.. -I./../readline/.. -I../bfd -I./../bfd -I./../include -I../libdecnumber -I./../libdecnumber -I./gnulib/import -Ibuild-gnulib/import -DTUI=1 -Id:/usr/include -Id:/usr/include/guile/2.0 -Id:/usr/include -Id:/usr/Python26/include -Id:/usr/Python26/include -Wall -Wpointer-arith -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable -Wno-sign-compare -Wno-narrowing -Wno-error=maybe-uninitialized -Wno-format -fno-strict-aliasing -DNDEBUG -fwrapv -c -o python/py-arch.o -MT python/py-arch.o -MMD -MP -MF python/.deps/py-arch.Tpo python/py-arch.c >>>> In file included from d:\usr\lib\gcc\mingw32\6.3.0\include\c++\math.h:36:0, >>>> from build-gnulib/import/math.h:27, >>>> from d:/usr/Python26/include/pyport.h:235, >>>> from d:/usr/Python26/include/Python.h:58, >>>> from python/python-internal.h:94, >>>> from python/py-arch.c:24: >>>> d:\usr\lib\gcc\mingw32\6.3.0\include\c++\cmath:1157:11: error: '::hypot' has not been declared >>>> using ::hypot; >>>> ^~~~~ >>>> Makefile:1618: recipe for target `python/py-arch.o' failed >>>> >>>> Googling suggests the following solution; is it okay to push this >>>> (with the necessary logs and after doing the "paperwork" required for >>>> branch changes)? Or does someone have better ideas? (Does this work >>>> in MinGW64?) >>>> >>>> --- gdb/python/python-internal.h~0 2018-01-12 05:31:04.000000000 +0200 >>>> +++ gdb/python/python-internal.h 2018-01-16 08:56:10.717759900 +0200 >>>> @@ -85,6 +85,12 @@ >>>> #define HAVE_SNPRINTF 1 >>>> #endif >>>> >>>> +/* Another kludge to avoid compilation errors because MinGW defines >>>> + 'hypot' to '_hypot', but the C++ headers says "using ::hypot". */ >>>> +#if defined(__MINGW32__) && defined(__cplusplus) Do we need "defined(__cplusplus)", since we are always building GDB as a C++ program? Thanks, Simon ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [ANNOUNCEMENT] GDB 8.1 release branch created! 2018-01-26 15:37 ` Simon Marchi @ 2018-01-26 18:53 ` Eli Zaretskii 2018-01-27 16:42 ` Eli Zaretskii 0 siblings, 1 reply; 40+ messages in thread From: Eli Zaretskii @ 2018-01-26 18:53 UTC (permalink / raw) To: Simon Marchi; +Cc: gdb-patches > From: Simon Marchi <simon.marchi@ericsson.com> > Date: Fri, 26 Jan 2018 10:36:56 -0500 > >>>> > >>>> +/* Another kludge to avoid compilation errors because MinGW defines > >>>> + 'hypot' to '_hypot', but the C++ headers says "using ::hypot". */ > >>>> +#if defined(__MINGW32__) && defined(__cplusplus) > > Do we need "defined(__cplusplus)", since we are always building GDB as a C++ program? I'm okay with dropping it. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [ANNOUNCEMENT] GDB 8.1 release branch created! 2018-01-26 18:53 ` Eli Zaretskii @ 2018-01-27 16:42 ` Eli Zaretskii 2018-02-01 15:12 ` Yao Qi 0 siblings, 1 reply; 40+ messages in thread From: Eli Zaretskii @ 2018-01-27 16:42 UTC (permalink / raw) To: simon.marchi; +Cc: gdb-patches > Date: Fri, 26 Jan 2018 20:52:56 +0200 > From: Eli Zaretskii <eliz@gnu.org> > CC: gdb-patches@sourceware.org > > > From: Simon Marchi <simon.marchi@ericsson.com> > > Date: Fri, 26 Jan 2018 10:36:56 -0500 > > >>>> > > >>>> +/* Another kludge to avoid compilation errors because MinGW defines > > >>>> + 'hypot' to '_hypot', but the C++ headers says "using ::hypot". */ > > >>>> +#if defined(__MINGW32__) && defined(__cplusplus) > > > > Do we need "defined(__cplusplus)", since we are always building GDB as a C++ program? > > I'm okay with dropping it. No further comments, so I went ahead and pushed the change, without the __cplusplus part, to master, and then cherry-picked to gdb-8.1-branch. Thanks for the feedback, Simon. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [ANNOUNCEMENT] GDB 8.1 release branch created! 2018-01-27 16:42 ` Eli Zaretskii @ 2018-02-01 15:12 ` Yao Qi 2018-02-01 16:27 ` Eli Zaretskii 0 siblings, 1 reply; 40+ messages in thread From: Yao Qi @ 2018-02-01 15:12 UTC (permalink / raw) To: Eli Zaretskii; +Cc: simon.marchi, gdb-patches Eli Zaretskii <eliz@gnu.org> writes: > No further comments, so I went ahead and pushed the change, without > the __cplusplus part, to master, and then cherry-picked to > gdb-8.1-branch. Hi Eli, Is it intended to include error messages in the changelog entry? If not, can we remove it? in the patch below, 2018-01-27 Eli Zaretskii <eliz@gnu.org> Avoid compilation errors in MinGW native builds The error is triggered by including python-internal.h, and the error message is: In file included from d:\usr\lib\gcc\mingw32\6.3.0\include\c++\math.h:36:0, from build-gnulib/import/math.h:27, from d:/usr/Python26/include/pyport.h:235, from d:/usr/Python26/include/Python.h:58, from python/python-internal.h:94, from python/py-arch.c:24: d:\usr\lib\gcc\mingw32\6.3.0\include\c++\cmath:1157:11: error: '::hypot' has not been declared using ::hypot; ^~~~~ This happens because Python headers define 'hypot' to expand t '_hypot' in the Windows builds. * python/python-internal.h (_hypot) [__MINGW32__]: Define back to 'hypoth'. This avoids a compilation error. -- Yao (齐尧) From 65951173ddd802b5c26f3f706ad845f41dc78f8b Mon Sep 17 00:00:00 2001 From: Yao Qi <yao.qi@linaro.org> Date: Thu, 1 Feb 2018 15:11:46 +0000 Subject: [PATCH] Remove unnecessary bits from ChangeLog Remove something shouldn't be put in ChangeLog. diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 5c3338f..40f4008 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -144,23 +144,6 @@ 2018-01-27 Eli Zaretskii <eliz@gnu.org> - Avoid compilation errors in MinGW native builds - - The error is triggered by including python-internal.h, and the - error message is: - - In file included from d:\usr\lib\gcc\mingw32\6.3.0\include\c++\math.h:36:0, - from build-gnulib/import/math.h:27, - from d:/usr/Python26/include/pyport.h:235, - from d:/usr/Python26/include/Python.h:58, - from python/python-internal.h:94, - from python/py-arch.c:24: - d:\usr\lib\gcc\mingw32\6.3.0\include\c++\cmath:1157:11: error: '::hypot' has not been declared - using ::hypot; - ^~~~~ - - This happens because Python headers define 'hypot' to expand t - '_hypot' in the Windows builds. * python/python-internal.h (_hypot) [__MINGW32__]: Define back to 'hypoth'. This avoids a compilation error. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [ANNOUNCEMENT] GDB 8.1 release branch created! 2018-02-01 15:12 ` Yao Qi @ 2018-02-01 16:27 ` Eli Zaretskii 2018-02-01 16:51 ` Yao Qi 0 siblings, 1 reply; 40+ messages in thread From: Eli Zaretskii @ 2018-02-01 16:27 UTC (permalink / raw) To: Yao Qi; +Cc: simon.marchi, gdb-patches > From: Yao Qi <qiyaoltc@gmail.com> > Cc: simon.marchi@ericsson.com, gdb-patches@sourceware.org > Date: Thu, 01 Feb 2018 15:12:47 +0000 > > Eli Zaretskii <eliz@gnu.org> writes: > > > No further comments, so I went ahead and pushed the change, without > > the __cplusplus part, to master, and then cherry-picked to > > gdb-8.1-branch. > > Hi Eli, > Is it intended to include error messages in the changelog entry? If > not, can we remove it? in the patch below, Maybe I'm missing something, but isn't the commit log entry supposed to have the same text as the ChangeLog for that change? And what's wrong with having error messages in ChangeLog's, anyway? ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [ANNOUNCEMENT] GDB 8.1 release branch created! 2018-02-01 16:27 ` Eli Zaretskii @ 2018-02-01 16:51 ` Yao Qi 2018-02-01 17:33 ` Eli Zaretskii 0 siblings, 1 reply; 40+ messages in thread From: Yao Qi @ 2018-02-01 16:51 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Simon Marchi, GDB Patches On Thu, Feb 1, 2018 at 4:27 PM, Eli Zaretskii <eliz@gnu.org> wrote: > Maybe I'm missing something, but isn't the commit log entry supposed > to have the same text as the ChangeLog for that change? My understanding is that ChangeLog entry should be copied at the end of commit log https://sourceware.org/gdb/wiki/ContributionChecklist#Properly_formatted_commit_messages > > And what's wrong with having error messages in ChangeLog's, anyway? Each entry in ChangeLog describes the changes to the code, https://www.gnu.org/prep/standards/standards.html#Change-Log-Concepts IMO, error messages are out of the scope of what ChangeLog is intended to describe. Also, I've never seen such ChangeLog entry (with error messages) before. -- Yao (齐尧) ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [ANNOUNCEMENT] GDB 8.1 release branch created! 2018-02-01 16:51 ` Yao Qi @ 2018-02-01 17:33 ` Eli Zaretskii 2018-02-01 21:32 ` Yao Qi 0 siblings, 1 reply; 40+ messages in thread From: Eli Zaretskii @ 2018-02-01 17:33 UTC (permalink / raw) To: Yao Qi; +Cc: simon.marchi, gdb-patches > From: Yao Qi <qiyaoltc@gmail.com> > Date: Thu, 1 Feb 2018 16:51:36 +0000 > Cc: Simon Marchi <simon.marchi@ericsson.com>, GDB Patches <gdb-patches@sourceware.org> > > On Thu, Feb 1, 2018 at 4:27 PM, Eli Zaretskii <eliz@gnu.org> wrote: > > Maybe I'm missing something, but isn't the commit log entry supposed > > to have the same text as the ChangeLog for that change? > > My understanding is that ChangeLog entry should be copied at the end > of commit log I believe I did that, except that I moved the motivation for the change before the ChangeLog header, as it belongs to the preamble of the Git log. (In ChangeLog, there's no subdivision of the entry.) > > And what's wrong with having error messages in ChangeLog's, anyway? > > Each entry in ChangeLog describes the changes to the code, > https://www.gnu.org/prep/standards/standards.html#Change-Log-Concepts > IMO, error messages are out of the scope of what ChangeLog is intended > to describe. This is changing, and some GNU projects already do it differently. I believe the next version of standards.texi will change the recommendations to be more lax. And using different style for each project is a pain. Is GDB really so rigid as to not tolerate this? Please note that the error message I cited is just part of the motivation for the change, so we are really talking about including the motivation in the ChangeLog. > Also, I've never seen such ChangeLog entry (with error messages) > before. That's not necessarily a reason for rejecting such entries. So: what is the project's take on this, if there is one? ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [ANNOUNCEMENT] GDB 8.1 release branch created! 2018-02-01 17:33 ` Eli Zaretskii @ 2018-02-01 21:32 ` Yao Qi 2018-02-02 15:23 ` Eli Zaretskii 0 siblings, 1 reply; 40+ messages in thread From: Yao Qi @ 2018-02-01 21:32 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Simon Marchi, GDB Patches On Thu, Feb 1, 2018 at 5:34 PM, Eli Zaretskii <eliz@gnu.org> wrote: >> From: Yao Qi <qiyaoltc@gmail.com> >> Date: Thu, 1 Feb 2018 16:51:36 +0000 >> Cc: Simon Marchi <simon.marchi@ericsson.com>, GDB Patches <gdb-patches@sourceware.org> >> >> On Thu, Feb 1, 2018 at 4:27 PM, Eli Zaretskii <eliz@gnu.org> wrote: >> > Maybe I'm missing something, but isn't the commit log entry supposed >> > to have the same text as the ChangeLog for that change? >> >> My understanding is that ChangeLog entry should be copied at the end >> of commit log > > I believe I did that, except that I moved the motivation for the > change before the ChangeLog header, as it belongs to the preamble of > the Git log. (In ChangeLog, there's no subdivision of the entry.) > It is absolutely fine to put the reason of the change in the commit log, but it is questionable to put commit log into ChangeLog. At least, nowadays, "commit log" is more than ChangeLog entry. This is the convention we followed for several years. >> > And what's wrong with having error messages in ChangeLog's, anyway? >> >> Each entry in ChangeLog describes the changes to the code, >> https://www.gnu.org/prep/standards/standards.html#Change-Log-Concepts >> IMO, error messages are out of the scope of what ChangeLog is intended >> to describe. > > This is changing, and some GNU projects already do it differently. I > believe the next version of standards.texi will change the > recommendations to be more lax. And using different style for each > project is a pain. Is GDB really so rigid as to not tolerate this? > GDB is evolving, so I am completely fine to changes, but you made the change of the way of writing changelog, which is not discussed before. > Please note that the error message I cited is just part of the > motivation for the change, so we are really talking about including > the motivation in the ChangeLog. > Yes, do we want to change the rule of writing ChangeLog, so that we can/should put the motivation for the change? Is the motivation mandatory in the changelog or optional? >> Also, I've never seen such ChangeLog entry (with error messages) >> before. > > That's not necessarily a reason for rejecting such entries. > > So: what is the project's take on this, if there is one? We need a clear rule on writing changelog, otherwise how do we write changelog or review patches? -- Yao (齐尧) ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [ANNOUNCEMENT] GDB 8.1 release branch created! 2018-02-01 21:32 ` Yao Qi @ 2018-02-02 15:23 ` Eli Zaretskii 2018-02-02 15:53 ` Joel Brobecker 0 siblings, 1 reply; 40+ messages in thread From: Eli Zaretskii @ 2018-02-02 15:23 UTC (permalink / raw) To: Yao Qi; +Cc: simon.marchi, gdb-patches > From: Yao Qi <qiyaoltc@gmail.com> > Date: Thu, 1 Feb 2018 21:32:13 +0000 > Cc: Simon Marchi <simon.marchi@ericsson.com>, GDB Patches <gdb-patches@sourceware.org> > > > Please note that the error message I cited is just part of the > > motivation for the change, so we are really talking about including > > the motivation in the ChangeLog. > > > > Yes, do we want to change the rule of writing ChangeLog, so that > we can/should put the motivation for the change? Is the motivation > mandatory in the changelog or optional? > > >> Also, I've never seen such ChangeLog entry (with error messages) > >> before. > > > > That's not necessarily a reason for rejecting such entries. > > > > So: what is the project's take on this, if there is one? > > We need a clear rule on writing changelog, otherwise how do we > write changelog or review patches? I agree. I hope Someone(TM) will answer these questions. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [ANNOUNCEMENT] GDB 8.1 release branch created! 2018-02-02 15:23 ` Eli Zaretskii @ 2018-02-02 15:53 ` Joel Brobecker 2018-02-02 16:27 ` Simon Marchi 2018-02-02 17:42 ` Joseph Myers 0 siblings, 2 replies; 40+ messages in thread From: Joel Brobecker @ 2018-02-02 15:53 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Yao Qi, simon.marchi, gdb-patches > > We need a clear rule on writing changelog, otherwise how do we > > write changelog or review patches? > > I agree. I hope Someone(TM) will answer these questions. Well, if it were me, I would stop using ChangeLog files with immediate effect. The purpose of ChangeLog files, as I understand it, is to describe the "what changed", and is aimed at people who download a given release and want to know what changed (because they do not have access to the version control information). While the above might have been essential 20 or 25 years ago, it is trivial for anyone today to have access to the change history of our program. Two options: - git clone our public repository; or... - ... if they don't want to use git, they can use the gitweb interface. I think that this is an acceptable way for us to provide the log of changes that were made. Currently, the situation is that we all pay a price in term of time wasted populating those files, and when we add the time collectively spent on those, I cannot imagine the few people's convenience would be worth that much effort. I vote for we stop using those ChangeLog files right now. We can still have them as part of the ChangeLog if people want -- although, I am also open to considering the pros and cons of keeping it, but I think they might not be worth the investiment either. -- Joel ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [ANNOUNCEMENT] GDB 8.1 release branch created! 2018-02-02 15:53 ` Joel Brobecker @ 2018-02-02 16:27 ` Simon Marchi 2018-02-02 17:42 ` Joseph Myers 1 sibling, 0 replies; 40+ messages in thread From: Simon Marchi @ 2018-02-02 16:27 UTC (permalink / raw) To: Joel Brobecker; +Cc: Eli Zaretskii, Yao Qi, simon.marchi, gdb-patches On 2018-02-02 10:53, Joel Brobecker wrote: > Well, if it were me, I would stop using ChangeLog files with > immediate effect. > > The purpose of ChangeLog files, as I understand it, is to describe > the "what changed", and is aimed at people who download a given > release and want to know what changed (because they do not have > access to the version control information). > > While the above might have been essential 20 or 25 years ago, > it is trivial for anyone today to have access to the change > history of our program. Two options: > - git clone our public repository; or... > - ... if they don't want to use git, they can use the gitweb > interface. > > I think that this is an acceptable way for us to provide the log > of changes that were made. > > Currently, the situation is that we all pay a price in term of time > wasted populating those files, and when we add the time collectively > spent on those, I cannot imagine the few people's convenience would > be worth that much effort. > > I vote for we stop using those ChangeLog files right now. We can > still have them as part of the ChangeLog if people want -- although, > I am also open to considering the pros and cons of keeping it, > but I think they might not be worth the investiment either. I agree about the time consumed producing ChangeLog entries. I have to admit that it sometimes discourages me to do structural changes that would affect a lot files/functions. I also feel like I'm filling some administrative paperwork, which is not my favorite way of spending my time. Maintaining an internal port of GDB, one use I have for the ChangeLog is to find what commit removed a given function. Let's say I rebase our code and find out a function we use was removed, grepping for that function would find the ChangeLog entry, leading me to the commit, which helps in fixing the code. However, I recently discovered "git blame --reverse", which allows to ask git about which commit removed a particular line. So the ChangeLog is not really required for that use case. Since with git the whole history is available offline, I think the usefulness of this kind of ChangeLogs is not as big as it used to be. To be fair I'll also state a pro I see for the ChangeLog. When I write the ChangeLog entry, it makes me look at my change a last time in details and sometimes find some additional issues I might not have found otherwise. It might be worth starting a new thread to get the attention of everybody to discuss this issue. Simon ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [ANNOUNCEMENT] GDB 8.1 release branch created! 2018-02-02 15:53 ` Joel Brobecker 2018-02-02 16:27 ` Simon Marchi @ 2018-02-02 17:42 ` Joseph Myers 1 sibling, 0 replies; 40+ messages in thread From: Joseph Myers @ 2018-02-02 17:42 UTC (permalink / raw) To: Joel Brobecker; +Cc: Eli Zaretskii, Yao Qi, simon.marchi, gdb-patches On Fri, 2 Feb 2018, Joel Brobecker wrote: > > > We need a clear rule on writing changelog, otherwise how do we > > > write changelog or review patches? > > > > I agree. I hope Someone(TM) will answer these questions. > > Well, if it were me, I would stop using ChangeLog files with > immediate effect. Feel free to join the discussion on bug-standards (which has been going for several months) of eliminating the GNU Coding Standards requirement for the ChangeLog *format*. (It's already been allowed by the GCS for over 20 years to generate the ChangeLog *file* at release time for the release tarball from version control history rather than having it checked in, but then of course you need to make sure that commit messages are appropriately formatted to derive ChangeLog entries - so I consider that the format, not the file, is the main issue.) -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 40+ messages in thread
end of thread, other threads:[~2018-02-02 17:42 UTC | newest] Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <announce.20180105041805.3FC35808E9@joel.gnat.com> 2018-01-16 17:31 ` [ANNOUNCEMENT] GDB 8.1 release branch created! Eli Zaretskii 2018-01-16 19:02 ` Sergio Durigan Junior 2018-01-16 19:46 ` [PATCH] Fix warning on gdb/compile/compile.c (C++-ify "triplet_rx") Sergio Durigan Junior 2018-01-17 15:33 ` Eli Zaretskii 2018-01-17 17:17 ` Simon Marchi 2018-01-17 23:07 ` Sergio Durigan Junior 2018-01-17 23:42 ` Simon Marchi 2018-01-17 23:48 ` Sergio Durigan Junior 2018-01-16 20:32 ` [PATCH] Fix unitialized warning on gdb/typeprint.c:whatis_exp Sergio Durigan Junior 2018-01-17 15:34 ` Eli Zaretskii 2018-01-17 16:48 ` Pedro Alves 2018-01-17 18:03 ` Sergio Durigan Junior 2018-01-20 1:03 ` [PATCH] Fix segfault when using 'set print object on' + whatis <struct> Sergio Durigan Junior 2018-01-22 17:42 ` [PATCH v2] Fix segfault when using 'set print object on' + whatis <struct> (Re: [PATCH] Fix segfault when using 'set print object on' + whatis <struct>) Pedro Alves 2018-01-22 18:04 ` Sergio Durigan Junior 2018-01-22 19:53 ` Pedro Alves 2018-01-22 20:11 ` Sergio Durigan Junior 2018-01-16 20:36 ` [ANNOUNCEMENT] GDB 8.1 release branch created! Sergio Durigan Junior 2018-01-17 3:36 ` Eli Zaretskii 2018-01-17 16:46 ` Sergio Durigan Junior 2018-01-17 11:04 ` Pedro Alves 2018-01-17 16:38 ` Sergio Durigan Junior 2018-01-17 16:46 ` Eli Zaretskii 2018-01-17 16:50 ` Pedro Alves 2018-01-17 18:21 ` Eli Zaretskii 2018-01-18 15:53 ` Eli Zaretskii 2018-01-25 16:58 ` Eli Zaretskii 2018-01-26 14:18 ` Eli Zaretskii 2018-01-26 15:37 ` Simon Marchi 2018-01-26 18:53 ` Eli Zaretskii 2018-01-27 16:42 ` Eli Zaretskii 2018-02-01 15:12 ` Yao Qi 2018-02-01 16:27 ` Eli Zaretskii 2018-02-01 16:51 ` Yao Qi 2018-02-01 17:33 ` Eli Zaretskii 2018-02-01 21:32 ` Yao Qi 2018-02-02 15:23 ` Eli Zaretskii 2018-02-02 15:53 ` Joel Brobecker 2018-02-02 16:27 ` Simon Marchi 2018-02-02 17:42 ` Joseph Myers
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).