On Mon, Mar 18, 2024 at 1:24 PM Tom Tromey <tom@tromey.com> wrote: > > >>>>> "Will" == Will Hawkins <hawkinsw@obs.cr> writes: > > Will> While DW_LNCT_source is not yet finalized in the DWARF standard > Will> (https://dwarfstd.org/issues/180201.1.html), LLVM does emit it. > > Will> This patch adds support for it in gdb. > > Thanks for the patch. > > Will> ChangeLog: > > gdb doesn't use ChangeLogs any more, so you can delete this part. Tom, Thank you so much for the comments! I am glad to see that (based on the volume of comments) I am on the right track. I really appreciate you taking the time to respond! And, yes, I *thought* that gdb had "retired" ChangeLogs, but I wasn't sure. I was trying to be as "correct" as possible. I will definitely drop it in v2! > > Will> + if (fe.source) > Will> + sf->symtab->source = cu->per_objfile->objfile->intern (fe.source); > > IIUC the 'source' here is the full text of the source code. > > In this case you don't want to use intern. That will make a copy of the > text. Instead, because the section data is read-in and not unmapped or > freed until the objfile is destroyed, you can just use the pointer > directly. Thank you for confirming! I thought that was the case (that the lifetime of the objfile outlives the symtab) but I wasn't sure. > > Will> +/* Open an embedded source file given a symtab S. Returns a file descriptor > Will> + or negative errno for error. */ > Will> + > Will> +scoped_fd > Will> +open_embedded_source(struct symtab *s, > Will> + gdb::unique_xmalloc_ptr<char> *fullname) > Will> +{ > > I think some other refactoring should be done so that gdb doesn't have > to write the source to a file and then re-read it. > > Some kind of abstraction here would be appropriate. Absolutely! I will do that! > > Will> --- a/gdb/symtab.h > Will> +++ b/gdb/symtab.h > Will> @@ -1755,6 +1755,8 @@ struct symtab > > Will> const char *filename; > > Will> + const char *source; > > This line could use an explanatory comment. I *thought* I added one -- sorry! > > Will> diff --git a/include/dwarf2.h b/include/dwarf2.h > Will> index b3d3731ee83..abe0359926b 100644 > Will> --- a/include/dwarf2.h > Will> +++ b/include/dwarf2.h > Will> @@ -288,7 +288,9 @@ enum dwarf_line_number_content_type > Will> DW_LNCT_timestamp = 0x3, > Will> DW_LNCT_size = 0x4, > Will> DW_LNCT_MD5 = 0x5, > Will> + DW_LNCT_SOURCE = 0x6, > Will> DW_LNCT_lo_user = 0x2000, > Will> + DW_LNCT_llvm_SOURCE = 0x2001, > > This should probably use "LLVM" and not "llvm". > Also for vendor extensions we like to have some kind of comment or > documentation explaining what it is about -- in the more distant past > this wasn't done and as a result there are some mystery extensions. You got it! > > Also I would not assign DW_LNCT_SOURCE until DWARF officially blesses > it. > Will do!! Thank you, again, Tom! Will > Tom
Move some more of the things that should be seen everywhere by all compilation units from common-defs.h to gdbsupport.inc.h. After this change, to the best of my knowledge, common-defs.h, server.h and defs.h are no longer special, they don't need to be included first anymore. A lot of source files don't need what is directly defined in common-defs.h / server.h / defs.h , but rather use things in header files included transitively. There's no rush to do so, but for these files, we'll be able to remove the inclusion of common-defs.h / server.h / defs.h and make them include what they actually use. Ultimately, I think we could just get rid of these header files, since they pretty much just server as big bags of includes. Moving to more fine-graned includes means less unnecessary includes, meaning less unnecessary recompilation when some header file is modified. The things moved to gdbsupport.inc.h are: - __STDC_CONSTANT_MACROS, __STDC_LIMIT_MACROS, __STDC_FORMAT_MACROS: according to the comment, they must be defined before including some standard headers. Are these defines still useful though? - _FORTIFY_SOURCE: must come before any standard library include. - _WIN32_WINNT: must come before including the Windows API headers. - ATTRIBUTE_PRINTF, ATTRIBUTE_NONNULL: we override what ansidecl.h gives us. So better define it in gdbsupport.inc.h, so that there is no chance of some source file including ansidecl.h without having the overrides. - HAVE_USEFUL_SBRK: this needs to be seen everywhere, in case one does `#ifdef HAVE_USEFUL_SBRK`. - Inclusion of poison.h, so that all code everywhere, uses our overrides of xfree & co. This requires including liberty.h in poison.h, since it overrides some macros from it. - Since poison.h uses xfree and xfree uses some traits defined in poison.h, but gnulib also defines its own (non-templated) version of xfree, I had some strange error due to some non-compatible redefinitions. All in all, it becomes simpler to move xfree to poison.h and get rid of gdb-xfree.h, so do that. Change-Id: I21ad69e5f38f9fd7a3943d9f96d3782739d4b6df --- gdbsupport/common-defs.h | 147 ----------------------------------- gdbsupport/common-utils.cc | 1 - gdbsupport/gdb-xfree.h | 41 ---------- gdbsupport/gdb_unique_ptr.h | 1 - gdbsupport/gdbsupport.inc.h | 149 ++++++++++++++++++++++++++++++++++++ gdbsupport/poison.h | 20 ++++- 6 files changed, 167 insertions(+), 192 deletions(-) delete mode 100644 gdbsupport/gdb-xfree.h diff --git a/gdbsupport/common-defs.h b/gdbsupport/common-defs.h index e7eb814f3251..401e3c4ef099 100644 --- a/gdbsupport/common-defs.h +++ b/gdbsupport/common-defs.h @@ -20,60 +20,6 @@ #ifndef COMMON_COMMON_DEFS_H #define COMMON_COMMON_DEFS_H -/* From: - https://www.gnu.org/software/gnulib/manual/html_node/stdint_002eh.html - - "On some hosts that predate C++11, when using C++ one must define - __STDC_CONSTANT_MACROS to make visible the definitions of constant - macros such as INTMAX_C, and one must define __STDC_LIMIT_MACROS to - make visible the definitions of limit macros such as INTMAX_MAX.". - - And: - https://www.gnu.org/software/gnulib/manual/html_node/inttypes_002eh.html - - "On some hosts that predate C++11, when using C++ one must define - __STDC_FORMAT_MACROS to make visible the declarations of format - macros such as PRIdMAX." - - Must do this before including any system header, since other system - headers may include stdint.h/inttypes.h. */ -#define __STDC_CONSTANT_MACROS 1 -#define __STDC_LIMIT_MACROS 1 -#define __STDC_FORMAT_MACROS 1 - -/* Some distros enable _FORTIFY_SOURCE by default, which on occasion - has caused build failures with -Wunused-result when a patch is - developed on a distro that does not enable _FORTIFY_SOURCE. We - enable it here in order to try to catch these problems earlier; - plus this seems like a reasonable safety measure. The check for - optimization is required because _FORTIFY_SOURCE only works when - optimization is enabled. If _FORTIFY_SOURCE is already defined, - then we don't do anything. Also, on MinGW, fortify requires - linking to -lssp, and to avoid the hassle of checking for - that and linking to it statically, we just don't define - _FORTIFY_SOURCE there. */ - -#if (!defined _FORTIFY_SOURCE && defined __OPTIMIZE__ && __OPTIMIZE__ > 0 \ - && !defined(__MINGW32__)) -#define _FORTIFY_SOURCE 2 -#endif - -/* We don't support Windows versions before XP, so we define - _WIN32_WINNT correspondingly to ensure the Windows API headers - expose the required symbols. - - NOTE: this must be kept in sync with common.m4. */ -#if defined (__MINGW32__) || defined (__CYGWIN__) -# ifdef _WIN32_WINNT -# if _WIN32_WINNT < 0x0501 -# undef _WIN32_WINNT -# define _WIN32_WINNT 0x0501 -# endif -# else -# define _WIN32_WINNT 0x0501 -# endif -#endif /* __MINGW32__ || __CYGWIN__ */ - #include <stdarg.h> #include <stdio.h> @@ -93,89 +39,6 @@ #include <alloca.h> #endif -#include "ansidecl.h" -/* This is defined by ansidecl.h, but we prefer gnulib's version. On - MinGW, gnulib might enable __USE_MINGW_ANSI_STDIO, which may or not - require use of attribute gnu_printf instead of printf. gnulib - checks that at configure time. Since _GL_ATTRIBUTE_FORMAT_PRINTF_STANDARD - is compatible with ATTRIBUTE_PRINTF, simply use it. */ -#undef ATTRIBUTE_PRINTF -#define ATTRIBUTE_PRINTF _GL_ATTRIBUTE_FORMAT_PRINTF_STANDARD - -/* This is defined by ansidecl.h, but we disable the attribute. - - Say a developer starts out with: - ... - extern void foo (void *ptr) __attribute__((nonnull (1))); - void foo (void *ptr) {} - ... - with the idea in mind to catch: - ... - foo (nullptr); - ... - at compile time with -Werror=nonnull, and then adds: - ... - void foo (void *ptr) { - + gdb_assert (ptr != nullptr); - } - ... - to catch: - ... - foo (variable_with_nullptr_value); - ... - at runtime as well. - - Said developer then verifies that the assert works (using -O0), and commits - the code. - - Some other developer then checks out the code and accidentally writes some - variant of: - ... - foo (variable_with_nullptr_value); - ... - and builds with -O2, and ... the assert doesn't trigger, because it's - optimized away by gcc. - - There's no suppported recipe to prevent the assertion from being optimized - away (other than: build with -O0, or remove the nonnull attribute). Note - that -fno-delete-null-pointer-checks does not help. A patch was submitted - to improve gcc documentation to point this out more clearly ( - https://gcc.gnu.org/pipermail/gcc-patches/2021-July/576218.html ). The - patch also mentions a possible workaround that obfuscates the pointer - using: - ... - void foo (void *ptr) { - + asm ("" : "+r"(ptr)); - gdb_assert (ptr != nullptr); - } - ... - but that still requires the developer to manually add this in all cases - where that's necessary. - - A warning was added to detect the situation: -Wnonnull-compare, which does - help in detecting those cases, but each new gcc release may indicate a new - batch of locations that needs fixing, which means we've added a maintenance - burden. - - We could try to deal with the problem more proactively by introducing a - gdb_assert variant like: - ... - void gdb_assert_non_null (void *ptr) { - asm ("" : "+r"(ptr)); - gdb_assert (ptr != nullptr); - } - void foo (void *ptr) { - gdb_assert_nonnull (ptr); - } - ... - and make it a coding style to use it everywhere, but again, maintenance - burden. - - With all these things considered, for now we go with the solution with the - least maintenance burden: disable the attribute, such that we reliably deal - with it everywhere. */ -#undef ATTRIBUTE_NONNULL -#define ATTRIBUTE_NONNULL(m) #define ATTRIBUTE_UNUSED_RESULT __attribute__ ((__warn_unused_result__)) #define ATTRIBUTE_USED __attribute__ ((__used__)) @@ -193,18 +56,8 @@ #include "common-debug.h" #include "cleanups.h" #include "common-exceptions.h" -#include "gdbsupport/poison.h" /* Pull in gdb::unique_xmalloc_ptr. */ #include "gdbsupport/gdb_unique_ptr.h" -/* sbrk on macOS is not useful for our purposes, since sbrk(0) always - returns the same value. brk/sbrk on macOS is just an emulation - that always returns a pointer to a 4MB section reserved for - that. */ - -#if defined (HAVE_SBRK) && !__APPLE__ -#define HAVE_USEFUL_SBRK 1 -#endif - #endif /* COMMON_COMMON_DEFS_H */ diff --git a/gdbsupport/common-utils.cc b/gdbsupport/common-utils.cc index 99b9cb8609bb..35a4c66f4546 100644 --- a/gdbsupport/common-utils.cc +++ b/gdbsupport/common-utils.cc @@ -21,7 +21,6 @@ #include "common-utils.h" #include "host-defs.h" #include "gdbsupport/gdb-safe-ctype.h" -#include "gdbsupport/gdb-xfree.h" void * xzalloc (size_t size) diff --git a/gdbsupport/gdb-xfree.h b/gdbsupport/gdb-xfree.h deleted file mode 100644 index 09c75395b62f..000000000000 --- a/gdbsupport/gdb-xfree.h +++ /dev/null @@ -1,41 +0,0 @@ -/* Copyright (C) 1986-2024 Free Software Foundation, Inc. - - This file is part of GDB. - - This program is free software; you can redistribute it and/or modify - it under the terms of the GNU General Public License as published by - the Free Software Foundation; either version 3 of the License, or - (at your option) any later version. - - This program is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - GNU General Public License for more details. - - You should have received a copy of the GNU General Public License - along with this program. If not, see <http://www.gnu.org/licenses/>. */ - -#ifndef GDBSUPPORT_GDB_XFREE_H -#define GDBSUPPORT_GDB_XFREE_H - -#include "gdbsupport/poison.h" - -/* GDB uses this instead of 'free', it detects when it is called on an - invalid type. */ - -template <typename T> -static void -xfree (T *ptr) -{ - static_assert (IsFreeable<T>::value, "Trying to use xfree with a non-POD \ -data type. Use operator delete instead."); - - if (ptr != NULL) -#ifdef GNULIB_NAMESPACE - GNULIB_NAMESPACE::free (ptr); /* ARI: free */ -#else - free (ptr); /* ARI: free */ -#endif -} - -#endif /* GDBSUPPORT_GDB_XFREE_H */ diff --git a/gdbsupport/gdb_unique_ptr.h b/gdbsupport/gdb_unique_ptr.h index 19b1581dab5c..3c2ec4098bed 100644 --- a/gdbsupport/gdb_unique_ptr.h +++ b/gdbsupport/gdb_unique_ptr.h @@ -22,7 +22,6 @@ #include <memory> #include <string> -#include "gdbsupport/gdb-xfree.h" namespace gdb { diff --git a/gdbsupport/gdbsupport.inc.h b/gdbsupport/gdbsupport.inc.h index ab0999579528..bca15460e87b 100644 --- a/gdbsupport/gdbsupport.inc.h +++ b/gdbsupport/gdbsupport.inc.h @@ -33,3 +33,152 @@ #undef PACKAGE_STRING #undef PACKAGE_TARNAME #undef PACKAGE_VERSION + +/* From: + https://www.gnu.org/software/gnulib/manual/html_node/stdint_002eh.html + + "On some hosts that predate C++11, when using C++ one must define + __STDC_CONSTANT_MACROS to make visible the definitions of constant + macros such as INTMAX_C, and one must define __STDC_LIMIT_MACROS to + make visible the definitions of limit macros such as INTMAX_MAX.". + + And: + https://www.gnu.org/software/gnulib/manual/html_node/inttypes_002eh.html + + "On some hosts that predate C++11, when using C++ one must define + __STDC_FORMAT_MACROS to make visible the declarations of format + macros such as PRIdMAX." + + Must do this before including any system header, since other system + headers may include stdint.h/inttypes.h. */ +#define __STDC_CONSTANT_MACROS 1 +#define __STDC_LIMIT_MACROS 1 +#define __STDC_FORMAT_MACROS 1 + +/* Some distros enable _FORTIFY_SOURCE by default, which on occasion + has caused build failures with -Wunused-result when a patch is + developed on a distro that does not enable _FORTIFY_SOURCE. We + enable it here in order to try to catch these problems earlier; + plus this seems like a reasonable safety measure. The check for + optimization is required because _FORTIFY_SOURCE only works when + optimization is enabled. If _FORTIFY_SOURCE is already defined, + then we don't do anything. Also, on MinGW, fortify requires + linking to -lssp, and to avoid the hassle of checking for + that and linking to it statically, we just don't define + _FORTIFY_SOURCE there. */ + +#if (!defined _FORTIFY_SOURCE && defined __OPTIMIZE__ && __OPTIMIZE__ > 0 \ + && !defined(__MINGW32__)) +#define _FORTIFY_SOURCE 2 +#endif + +/* We don't support Windows versions before XP, so we define + _WIN32_WINNT correspondingly to ensure the Windows API headers + expose the required symbols. + + NOTE: this must be kept in sync with common.m4. */ +#if defined (__MINGW32__) || defined (__CYGWIN__) +# ifdef _WIN32_WINNT +# if _WIN32_WINNT < 0x0501 +# undef _WIN32_WINNT +# define _WIN32_WINNT 0x0501 +# endif +# else +# define _WIN32_WINNT 0x0501 +# endif +#endif /* __MINGW32__ || __CYGWIN__ */ + +#include "ansidecl.h" +/* This is defined by ansidecl.h, but we prefer gnulib's version. On + MinGW, gnulib might enable __USE_MINGW_ANSI_STDIO, which may or not + require use of attribute gnu_printf instead of printf. gnulib + checks that at configure time. Since _GL_ATTRIBUTE_FORMAT_PRINTF_STANDARD + is compatible with ATTRIBUTE_PRINTF, simply use it. */ +#undef ATTRIBUTE_PRINTF +#define ATTRIBUTE_PRINTF _GL_ATTRIBUTE_FORMAT_PRINTF_STANDARD + +/* This is defined by ansidecl.h, but we disable the attribute. + + Say a developer starts out with: + ... + extern void foo (void *ptr) __attribute__((nonnull (1))); + void foo (void *ptr) {} + ... + with the idea in mind to catch: + ... + foo (nullptr); + ... + at compile time with -Werror=nonnull, and then adds: + ... + void foo (void *ptr) { + + gdb_assert (ptr != nullptr); + } + ... + to catch: + ... + foo (variable_with_nullptr_value); + ... + at runtime as well. + + Said developer then verifies that the assert works (using -O0), and commits + the code. + + Some other developer then checks out the code and accidentally writes some + variant of: + ... + foo (variable_with_nullptr_value); + ... + and builds with -O2, and ... the assert doesn't trigger, because it's + optimized away by gcc. + + There's no suppported recipe to prevent the assertion from being optimized + away (other than: build with -O0, or remove the nonnull attribute). Note + that -fno-delete-null-pointer-checks does not help. A patch was submitted + to improve gcc documentation to point this out more clearly ( + https://gcc.gnu.org/pipermail/gcc-patches/2021-July/576218.html ). The + patch also mentions a possible workaround that obfuscates the pointer + using: + ... + void foo (void *ptr) { + + asm ("" : "+r"(ptr)); + gdb_assert (ptr != nullptr); + } + ... + but that still requires the developer to manually add this in all cases + where that's necessary. + + A warning was added to detect the situation: -Wnonnull-compare, which does + help in detecting those cases, but each new gcc release may indicate a new + batch of locations that needs fixing, which means we've added a maintenance + burden. + + We could try to deal with the problem more proactively by introducing a + gdb_assert variant like: + ... + void gdb_assert_non_null (void *ptr) { + asm ("" : "+r"(ptr)); + gdb_assert (ptr != nullptr); + } + void foo (void *ptr) { + gdb_assert_nonnull (ptr); + } + ... + and make it a coding style to use it everywhere, but again, maintenance + burden. + + With all these things considered, for now we go with the solution with the + least maintenance burden: disable the attribute, such that we reliably deal + with it everywhere. */ +#undef ATTRIBUTE_NONNULL +#define ATTRIBUTE_NONNULL(m) + +#include "gdbsupport/poison.h" + +/* sbrk on macOS is not useful for our purposes, since sbrk(0) always + returns the same value. brk/sbrk on macOS is just an emulation + that always returns a pointer to a 4MB section reserved for + that. */ + +#if defined (HAVE_SBRK) && !__APPLE__ +#define HAVE_USEFUL_SBRK 1 +#endif diff --git a/gdbsupport/poison.h b/gdbsupport/poison.h index 7b4f8e8a178d..cb1f474b5c5b 100644 --- a/gdbsupport/poison.h +++ b/gdbsupport/poison.h @@ -20,6 +20,7 @@ #ifndef COMMON_POISON_H #define COMMON_POISON_H +#include "libiberty.h" #include "traits.h" #include "obstack.h" @@ -90,8 +91,23 @@ using IsMallocable = std::is_trivially_constructible<T>; template<typename T> using IsFreeable = gdb::Or<std::is_trivially_destructible<T>, std::is_void<T>>; -template <typename T, typename = gdb::Requires<gdb::Not<IsFreeable<T>>>> -void free (T *ptr) = delete; +/* GDB uses this instead of 'free', it detects when it is called on an + invalid type. */ + +template <typename T> +static void +xfree (T *ptr) +{ + static_assert (IsFreeable<T>::value, "Trying to use xfree with a non-POD \ +data type. Use operator delete instead."); + + if (ptr != NULL) +#ifdef GNULIB_NAMESPACE + GNULIB_NAMESPACE::free (ptr); /* ARI: free */ +#else + free (ptr); /* ARI: free */ +#endif +} template<typename T> static T * -- 2.44.0
The motivation for this change is for analysis tools and IDEs to be better at analyzing header files on their own. There are some definitions and includes we want to occur at the very beginning of all translation units. The way we currently do that is by requiring all source files (.c and .cc files) to include one of defs.h (for gdb), server.h (for gdbserver) of common-defs.h (for gdbsupport and shared source files). These special header files define and include everything that needs to be included at the very beginning. Other header files are written in a way that assume that these special "prologue" header files have already been included. My problem with that is that my editor (clangd-based) provides a very bad experience when editing source files. Since clangd doesn't know that one of defs.h/server.h/common-defs.h was included already, a lot of things are flagged as errors. For instance, CORE_ADDR is not known. It's possible to edit the files in this state, but a lot of the power of the editor is unavailable. My proposal to help with this is to include those things we always want to be there using the compilers' `-include` option. Tom Tromey said that the current approach might exist because not all compilers used to have an option like this. But I believe that it's safe to assume they do today. With this change, clangd picks up the -include option from the compile command, and is able to analyze the header file correctly, as it sees all that stuff included or defined but that -include option. That works because when editing a header file, clangd tries to get the compilation flags from a source file that includes said header file. This change is a bit, because it addresses one of my frustrations when editing header files, but it might help others too. I'd be curious to know if others encounter the same kinds of problems when editing header files. Also, even if the change is not necessary by any means, I think the solution of using -include for stuff we always want to be there is more elegant than the current solution. Even with this -include flag, many header files currently don't include what they use, but rather depend on files included before them. This will still cause errors when editing them, but it should be easily fixable by adding the appropriate include. There's no rush to do so, as long as the code still copiles, it's just a convenience thing. This patch does the small step of moving the inclusion of the various config.h files to that new method. The changes are: - Add three files meant to be included with -include: gdb/gdb.inc.h, gdbserver/gdbserver.inc.h and gdbsupport/gdbsupport.inc.h. - Move the inclusion of the various config.h files there - Modify the compilation flags of all three subdirectories to add the appropriate -include option. Change-Id: If3e345d00a9fc42336322f1d8286687d22134340 --- gdb/Makefile.in | 1 + gdb/defs.h | 7 ------- gdb/gdb.inc.h | 22 ++++++++++++++++++++++ gdbserver/Makefile.in | 3 ++- gdbserver/gdbserver.inc.h | 22 ++++++++++++++++++++++ gdbserver/server.h | 8 -------- gdbsupport/Makefile.am | 1 + gdbsupport/Makefile.in | 16 ++++++++++++---- gdbsupport/common-defs.h | 10 ---------- gdbsupport/gdbsupport.inc.h | 35 +++++++++++++++++++++++++++++++++++ 10 files changed, 95 insertions(+), 30 deletions(-) create mode 100644 gdb/gdb.inc.h create mode 100644 gdbserver/gdbserver.inc.h create mode 100644 gdbsupport/gdbsupport.inc.h diff --git a/gdb/Makefile.in b/gdb/Makefile.in index 95709ae395a2..3b144fa3034c 100644 --- a/gdb/Makefile.in +++ b/gdb/Makefile.in @@ -605,6 +605,7 @@ GDB_CFLAGS = \ -I. \ -I$(srcdir) \ -I$(srcdir)/config \ + -include $(srcdir)/gdb.inc.h \ -DLOCALEDIR="\"$(localedir)\"" \ $(DEFS) diff --git a/gdb/defs.h b/gdb/defs.h index cf471bf5d662..8a9ff3aba0f7 100644 --- a/gdb/defs.h +++ b/gdb/defs.h @@ -25,13 +25,6 @@ #include "gdbsupport/common-defs.h" -#undef PACKAGE -#undef PACKAGE_NAME -#undef PACKAGE_VERSION -#undef PACKAGE_STRING -#undef PACKAGE_TARNAME - -#include <config.h> #include "bfd.h" #include <sys/types.h> diff --git a/gdb/gdb.inc.h b/gdb/gdb.inc.h new file mode 100644 index 000000000000..7be4c8eea939 --- /dev/null +++ b/gdb/gdb.inc.h @@ -0,0 +1,22 @@ +/* Copyright (C) 2024 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +/* This file in included automatically via `-include` at the beginning of each + source file compiled in gdb/. */ + +#include "gdbsupport/gdbsupport.inc.h" +#include "config.h" diff --git a/gdbserver/Makefile.in b/gdbserver/Makefile.in index c17a0a522df2..c92b881650a4 100644 --- a/gdbserver/Makefile.in +++ b/gdbserver/Makefile.in @@ -134,7 +134,8 @@ INCLUDE_CFLAGS = \ -I$(srcdir)/../gdb \ $(INCGNU) \ $(INCSUPPORT) \ - $(INTL_CFLAGS) + $(INTL_CFLAGS) \ + -include $(srcdir)/gdbserver.inc.h # M{H,T}_CFLAGS, if defined, has host- and target-dependent CFLAGS # from the config/ directory. diff --git a/gdbserver/gdbserver.inc.h b/gdbserver/gdbserver.inc.h new file mode 100644 index 000000000000..22ec0dd94318 --- /dev/null +++ b/gdbserver/gdbserver.inc.h @@ -0,0 +1,22 @@ +/* Copyright (C) 2024 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +/* This file in included automatically via `-include` at the beginning of each + source file compiled in gdbserver/. */ + +#include "gdbsupport/gdbsupport.inc.h" +#include "config.h" diff --git a/gdbserver/server.h b/gdbserver/server.h index 0074818c6df0..ee27d2b3f5c2 100644 --- a/gdbserver/server.h +++ b/gdbserver/server.h @@ -21,14 +21,6 @@ #include "gdbsupport/common-defs.h" -#undef PACKAGE -#undef PACKAGE_NAME -#undef PACKAGE_VERSION -#undef PACKAGE_STRING -#undef PACKAGE_TARNAME - -#include <config.h> - static_assert (sizeof (CORE_ADDR) >= sizeof (void *)); #include "gdbsupport/version.h" diff --git a/gdbsupport/Makefile.am b/gdbsupport/Makefile.am index 7c360aa413ef..db1eee88059a 100644 --- a/gdbsupport/Makefile.am +++ b/gdbsupport/Makefile.am @@ -35,6 +35,7 @@ AM_CPPFLAGS = \ $(INCINTL) \ -I../bfd \ -I$(srcdir)/../bfd \ + -include $(srcdir)/gdbsupport.inc.h \ @LARGEFILE_CPPFLAGS@ override CXX += $(CXX_DIALECT) diff --git a/gdbsupport/Makefile.in b/gdbsupport/Makefile.in index 6f8dacc157f9..9b1063f31ab3 100644 --- a/gdbsupport/Makefile.in +++ b/gdbsupport/Makefile.in @@ -393,10 +393,18 @@ ACLOCAL_AMFLAGS = -I . -I ../config # from Automake, as gdbsupport uses AM_GNU_GETTEXT through # ZW_GNU_GETTEXT_SISTER_DIR, but doesn't have any translations (currently). SUBDIRS = -AM_CPPFLAGS = -I$(srcdir)/../include -I$(srcdir)/../gdb \ - -I../gnulib/import -I$(srcdir)/../gnulib/import \ - -I.. -I$(srcdir)/.. $(INCINTL) -I../bfd -I$(srcdir)/../bfd \ - @LARGEFILE_CPPFLAGS@ +AM_CPPFLAGS = \ + -I$(srcdir)/../include \ + -I$(srcdir)/../gdb \ + -I../gnulib/import \ + -I$(srcdir)/../gnulib/import \ + -I.. \ + -I$(srcdir)/.. \ + $(INCINTL) \ + -I../bfd \ + -I$(srcdir)/../bfd \ + -include $(srcdir)/gdbsupport.inc.h \ + @LARGEFILE_CPPFLAGS@ AM_CXXFLAGS = $(WARN_CFLAGS) $(WERROR_CFLAGS) noinst_LIBRARIES = libgdbsupport.a diff --git a/gdbsupport/common-defs.h b/gdbsupport/common-defs.h index 6120719480b8..e7eb814f3251 100644 --- a/gdbsupport/common-defs.h +++ b/gdbsupport/common-defs.h @@ -20,16 +20,6 @@ #ifndef COMMON_COMMON_DEFS_H #define COMMON_COMMON_DEFS_H -#include <gdbsupport/config.h> - -#undef PACKAGE_NAME -#undef PACKAGE -#undef PACKAGE_VERSION -#undef PACKAGE_STRING -#undef PACKAGE_TARNAME - -#include "gnulib/config.h" - /* From: https://www.gnu.org/software/gnulib/manual/html_node/stdint_002eh.html diff --git a/gdbsupport/gdbsupport.inc.h b/gdbsupport/gdbsupport.inc.h new file mode 100644 index 000000000000..ab0999579528 --- /dev/null +++ b/gdbsupport/gdbsupport.inc.h @@ -0,0 +1,35 @@ +/* Copyright (C) 2024 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +/* This file in included automatically via `-include` at the beginning of each + source file compiled in gdbsupport/. */ + +#include "gdbsupport/config.h" + +#undef PACKAGE +#undef PACKAGE_NAME +#undef PACKAGE_STRING +#undef PACKAGE_TARNAME +#undef PACKAGE_VERSION + +#include "gnulib/config.h" + +#undef PACKAGE +#undef PACKAGE_NAME +#undef PACKAGE_STRING +#undef PACKAGE_TARNAME +#undef PACKAGE_VERSION -- 2.44.0
Reformat some variables that the subsequent patches are going to touch. I think it makes them easier to read, and it also makes diffs clearer. Change-Id: I82f63ba0e6d0fe268eb1f1ad5ab22c3cd016ab02 --- gdb/Makefile.in | 8 ++++++-- gdbserver/Makefile.in | 12 +++++++++--- gdbsupport/Makefile.am | 15 +++++++++++---- 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/gdb/Makefile.in b/gdb/Makefile.in index 331620375aed..95709ae395a2 100644 --- a/gdb/Makefile.in +++ b/gdb/Makefile.in @@ -601,8 +601,12 @@ CONFIG_DEP_SUBDIR = $(addsuffix /$(DEPDIR),$(CONFIG_SRC_SUBDIR)) # your system doesn't have fcntl.h in /usr/include (which is where it # should be according to Posix). DEFS = @DEFS@ -GDB_CFLAGS = -I. -I$(srcdir) -I$(srcdir)/config \ - -DLOCALEDIR="\"$(localedir)\"" $(DEFS) +GDB_CFLAGS = \ + -I. \ + -I$(srcdir) \ + -I$(srcdir)/config \ + -DLOCALEDIR="\"$(localedir)\"" \ + $(DEFS) # MH_CFLAGS, if defined, has host-dependent CFLAGS from the config directory. GLOBAL_CFLAGS = $(MH_CFLAGS) diff --git a/gdbserver/Makefile.in b/gdbserver/Makefile.in index c7120895a26d..c17a0a522df2 100644 --- a/gdbserver/Makefile.in +++ b/gdbserver/Makefile.in @@ -125,9 +125,15 @@ INCSUPPORT = -I$(srcdir)/.. -I.. # in those directories should be included with the subdirectory. # e.g.: "target/wait.h". # -INCLUDE_CFLAGS = -I. -I${srcdir} \ - -I$(srcdir)/../gdb/regformats -I$(srcdir)/.. -I$(INCLUDE_DIR) \ - -I$(srcdir)/../gdb $(INCGNU) $(INCSUPPORT) \ +INCLUDE_CFLAGS = \ + -I. \ + -I${srcdir} \ + -I$(srcdir)/../gdb/regformats \ + -I$(srcdir)/.. \ + -I$(INCLUDE_DIR) \ + -I$(srcdir)/../gdb \ + $(INCGNU) \ + $(INCSUPPORT) \ $(INTL_CFLAGS) # M{H,T}_CFLAGS, if defined, has host- and target-dependent CFLAGS diff --git a/gdbsupport/Makefile.am b/gdbsupport/Makefile.am index 88414b4c927a..7c360aa413ef 100644 --- a/gdbsupport/Makefile.am +++ b/gdbsupport/Makefile.am @@ -25,10 +25,17 @@ ACLOCAL_AMFLAGS = -I . -I ../config # ZW_GNU_GETTEXT_SISTER_DIR, but doesn't have any translations (currently). SUBDIRS = -AM_CPPFLAGS = -I$(srcdir)/../include -I$(srcdir)/../gdb \ - -I../gnulib/import -I$(srcdir)/../gnulib/import \ - -I.. -I$(srcdir)/.. $(INCINTL) -I../bfd -I$(srcdir)/../bfd \ - @LARGEFILE_CPPFLAGS@ +AM_CPPFLAGS = \ + -I$(srcdir)/../include \ + -I$(srcdir)/../gdb \ + -I../gnulib/import \ + -I$(srcdir)/../gnulib/import \ + -I.. \ + -I$(srcdir)/.. \ + $(INCINTL) \ + -I../bfd \ + -I$(srcdir)/../bfd \ + @LARGEFILE_CPPFLAGS@ override CXX += $(CXX_DIALECT) base-commit: 0273b5967e21404d0442273b189b0e275cfafa74 -- 2.44.0
> Sorry for the late reply. I was going to finish all the paperwork before > continuing, but it takes too long. Do you know the current status of it? Unfortunately we can't accept the patches without it. >> Anyway what I was really asking is if you ran the gdb testsuite. > I've run the testsuite locally. When patch is applied I get one > additional 'KFAIL ingdb.threads/process-dies-while-handling-bp.exp'. > But this test seems to be flaky on my machine and if I rerun it in > master, it also fails. Yeah, there are some flaky tests. Tom
On 2024-03-18 13:29, Pedro Alves wrote: > On 2024-03-15 18:27, Pedro Alves wrote: > + /* If we already know we have an all-zero block at the next > + offset, we can skip calling get_all_zero_block_size for > + it again. */ > + if (next_all_zero_block.offset != 0) > + data_offset += next_all_zero_block.offset; Err, all the effort to pass down the size, only to typo and not use it... Sigh. That last line should be: data_offset += next_all_zero_block.size; Here's the corrected patch... From adb681ce583fa640c4fb6883a827f3ab6b28b1c0 Mon Sep 17 00:00:00 2001 From: Pedro Alves <pedro@palves.net> Date: Mon, 18 Mar 2024 13:16:10 +0000 Subject: [PATCH v3] Teach GDB to generate sparse core files (PR corefiles/31494) This commit teaches GDB's gcore command to generate sparse core files (if supported by the filesystem). To create a sparse file, all you have to do is skip writing zeros to the file, instead lseek'ing-ahead over them. The sparse logic is applied when writing the memory sections, as that's where the bulk of the data and the zeros are. The commit also tweaks gdb.base/bigcore.exp to make it exercise gdb-generated cores in addition to kernel-generated cores. We couldn't do that before, because GDB's gcore on that test's program would generate a multi-GB non-sparse core (16GB on my system). After this commit, gdb.base/bigcore.exp generates, when testing with GDB's gcore, a much smaller core file, roughly in line with what the kernel produces: real sizes: $ du --hu testsuite/outputs/gdb.base/bigcore/bigcore.corefile.* 2.2M testsuite/outputs/gdb.base/bigcore/bigcore.corefile.gdb 2.0M testsuite/outputs/gdb.base/bigcore/bigcore.corefile.kernel apparent sizes: $ du --hu --apparent-size testsuite/outputs/gdb.base/bigcore/bigcore.corefile.* 16G testsuite/outputs/gdb.base/bigcore/bigcore.corefile.gdb 16G testsuite/outputs/gdb.base/bigcore/bigcore.corefile.kernel Time to generate the core also goes down significantly. On my machine, I get: when writing to an SSD, from 21.0s, down to 8.0s when writing to an HDD, from 31.0s, down to 8.5s The changes to gdb.base/bigcore.exp are smaller than they look at first sight. It's basically mostly refactoring -- moving most of the code to a new procedure which takes as argument who should dump the core, and then calling the procedure twice. I purposedly did not modernize any of the refactored code in this patch. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31494 Reviewed-By: Lancelot Six <lancelot.six@amd.com> Reviewed-By: Eli Zaretskii <eliz@gnu.org> Change-Id: I2554a6a4a72d8c199ce31f176e0ead0c0c76cff1 --- gdb/NEWS | 4 + gdb/doc/gdb.texinfo | 3 + gdb/gcore.c | 177 ++++++++++++++++++++- gdb/testsuite/gdb.base/bigcore.exp | 238 ++++++++++++++++------------- 4 files changed, 314 insertions(+), 108 deletions(-) diff --git a/gdb/NEWS b/gdb/NEWS index d8ac0bb06a7..d1d25e4c24d 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -23,6 +23,10 @@ disassemble command will now give an error. Previously the 'b' flag would always override the 'r' flag. +gcore +generate-core-file + GDB now generates sparse core files, on systems that support it. + maintenance info line-table Add an EPILOGUE-BEGIN column to the output of the command. It indicates if the line is considered the start of the epilgoue, and thus a point at diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index f093ee269e2..9224829bd93 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -13867,6 +13867,9 @@ Produce a core dump of the inferior process. The optional argument specified, the file name defaults to @file{core.@var{pid}}, where @var{pid} is the inferior process ID. +If supported by the filesystem where the core is written to, +@value{GDBN} generates a sparse core dump file. + Note that this command is implemented only for some systems (as of this writing, @sc{gnu}/Linux, FreeBSD, Solaris, and S390). diff --git a/gdb/gcore.c b/gdb/gcore.c index 7c12aa3a777..23e8066745a 100644 --- a/gdb/gcore.c +++ b/gdb/gcore.c @@ -39,10 +39,21 @@ #include "gdbsupport/byte-vector.h" #include "gdbsupport/scope-exit.h" +/* To generate sparse cores, we look at the data to write in chunks of + this size when considering whether to skip the write. Only if we + have a full block of this size with all zeros do we skip writing + it. A simpler algorithm that would try to skip all zeros would + result in potentially many more write/lseek syscalls, as normal + data is typically sprinkled with many small holes of zeros. Also, + it's much more efficient to memcmp a block of data against an + all-zero buffer than to check each and every data byte against zero + one by one. */ +#define SPARSE_BLOCK_SIZE 0x1000 + /* The largest amount of memory to read from the target at once. We must throttle it to limit the amount of memory used by GDB during generate-core-file for programs with large resident data. */ -#define MAX_COPY_BYTES (1024 * 1024) +#define MAX_COPY_BYTES (256 * SPARSE_BLOCK_SIZE) static const char *default_gcore_target (void); static enum bfd_architecture default_gcore_arch (void); @@ -98,7 +109,12 @@ write_gcore_file_1 (bfd *obfd) bfd_set_section_alignment (note_sec, 0); bfd_set_section_size (note_sec, note_size); - /* Now create the memory/load sections. */ + /* Now create the memory/load sections. Note + gcore_memory_sections's sparse logic is assuming that we'll + always write something afterwards, which we do: just below, we + write the note section. So there's no need for an ftruncate-like + call to grow the file to the right size if the last memory + sections were zeros and we skipped writing them. */ if (gcore_memory_sections (obfd) == 0) error (_("gcore: failed to get corefile memory sections from target.")); @@ -567,6 +583,158 @@ objfile_find_memory_regions (struct target_ops *self, return 0; } +/* Check if we have a block full of zeros at DATA within the [DATA, + DATA+SIZE) buffer. Returns the size of the all-zero block found. + Returns at most the minimum between SIZE and SPARSE_BLOCK_SIZE. */ + +static size_t +get_all_zero_block_size (const gdb_byte *data, size_t size) +{ + size = std::min (size, (size_t) SPARSE_BLOCK_SIZE); + + /* A memcmp of a whole block is much faster than a simple for loop. + This makes a big difference, as with a for loop, this code would + dominate the performance and result in doubling the time to + generate a core, at the time of writing. With an optimized + memcmp, this doesn't even show up in the perf trace. */ + static const gdb_byte all_zero_block[SPARSE_BLOCK_SIZE] = {}; + if (memcmp (data, all_zero_block, size) == 0) + return size; + return 0; +} + +/* Basically a named-elements pair, used as return type of + find_next_all_zero_block. */ + +struct offset_and_size +{ + size_t offset; + size_t size; +}; + +/* Find the next all-zero block at DATA+OFFSET within the [DATA, + DATA+SIZE) buffer. Returns the offset and the size of the all-zero + block if found, or zero if not found. */ + +static offset_and_size +find_next_all_zero_block (const gdb_byte *data, size_t offset, size_t size) +{ + for (; offset < size; offset += SPARSE_BLOCK_SIZE) + { + size_t zero_block_size + = get_all_zero_block_size (data + offset, size - offset); + if (zero_block_size != 0) + return {offset, zero_block_size}; + } + return {0, 0}; +} + +/* Wrapper around bfd_set_section_contents that avoids writing + all-zero blocks to disk, so we create a sparse core file. + SKIP_ALIGN is a recursion helper -- if true, we'll skip aligning + the file position to SPARSE_BLOCK_SIZE. */ + +static bool +sparse_bfd_set_section_contents (bfd *obfd, asection *osec, + const gdb_byte *data, + size_t sec_offset, + size_t size, + bool skip_align = false) +{ + /* Note, we don't have to have special handling for the case of the + last memory region ending with zeros, because our caller always + writes out the note section after the memory/load sections. If + it didn't, we'd have to seek+write the last byte to make the file + size correct. (Or add an ftruncate abstraction to bfd and call + that.) */ + + if (!skip_align) + { + /* Align the all-zero block search with SPARSE_BLOCK_SIZE, to + better align with filesystem blocks. If we find we're + misaligned, then write/skip the bytes needed to make us + aligned. We do that with (one level) recursion. */ + + /* We need to know the section's file offset on disk. We can + only look at it after the bfd's 'output_has_begun' flag has + been set, as bfd hasn't computed the file offsets + otherwise. */ + if (!obfd->output_has_begun) + { + gdb_byte dummy = 0; + + /* A write forces BFD to compute the bfd's section file + positions. Zero size works for that too. */ + if (!bfd_set_section_contents (obfd, osec, &dummy, 0, 0)) + return false; + + gdb_assert (obfd->output_has_begun); + } + + /* How much we need to write/skip in order to find the next + SPARSE_BLOCK_SIZE filepos-aligned block. */ + size_t align_remainder + = (SPARSE_BLOCK_SIZE + - (osec->filepos + sec_offset) % SPARSE_BLOCK_SIZE); + + /* How much we'll actually write in the recursion call. */ + size_t align_write_size = std::min (size, align_remainder); + + if (align_write_size != 0) + { + /* Recurse, skipping the alignment code. */ + if (!sparse_bfd_set_section_contents (obfd, osec, data, + sec_offset, + align_write_size, true)) + return false; + + /* Skip over what we've written, and proceed with + assumes-aligned logic. */ + data += align_write_size; + sec_offset += align_write_size; + size -= align_write_size; + } + } + + size_t data_offset = 0; + while (data_offset < size) + { + size_t all_zero_block_size + = get_all_zero_block_size (data + data_offset, size - data_offset); + if (all_zero_block_size != 0) + data_offset += all_zero_block_size; + else + { + /* We have some non-zero data to write to file. Find the + next all-zero block within the data, and only write up to + it. */ + + offset_and_size next_all_zero_block + = find_next_all_zero_block (data, + data_offset + SPARSE_BLOCK_SIZE, + size); + size_t next_data_offset = (next_all_zero_block.offset == 0 + ? size + : next_all_zero_block.offset); + + if (!bfd_set_section_contents (obfd, osec, data + data_offset, + sec_offset + data_offset, + next_data_offset - data_offset)) + return false; + + data_offset = next_data_offset; + + /* If we already know we have an all-zero block at the next + offset, we can skip calling get_all_zero_block_size for + it again. */ + if (next_all_zero_block.offset != 0) + data_offset += next_all_zero_block.size; + } + } + + return true; +} + static void gcore_copy_callback (bfd *obfd, asection *osec) { @@ -599,8 +767,9 @@ gcore_copy_callback (bfd *obfd, asection *osec) bfd_section_vma (osec))); break; } - if (!bfd_set_section_contents (obfd, osec, memhunk.data (), - offset, size)) + + if (!sparse_bfd_set_section_contents (obfd, osec, memhunk.data (), + offset, size)) { warning (_("Failed to write corefile contents (%s)."), bfd_errmsg (bfd_get_error ())); diff --git a/gdb/testsuite/gdb.base/bigcore.exp b/gdb/testsuite/gdb.base/bigcore.exp index 3f9ae48abf2..6c64d402502 100644 --- a/gdb/testsuite/gdb.base/bigcore.exp +++ b/gdb/testsuite/gdb.base/bigcore.exp @@ -43,23 +43,6 @@ if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {deb return -1 } -# Run GDB on the bigcore program up-to where it will dump core. - -clean_restart ${binfile} -gdb_test_no_output "set print sevenbit-strings" -gdb_test_no_output "set width 0" - -# Get the core into the output directory. -set_inferior_cwd_to_output_dir - -if {![runto_main]} { - return 0 -} -set print_core_line [gdb_get_line_number "Dump core"] -gdb_test "tbreak $print_core_line" -gdb_test continue ".*print_string.*" -gdb_test next ".*0 = 0.*" - # Traverse part of bigcore's linked list of memory chunks (forward or # backward), saving each chunk's address. @@ -92,92 +75,11 @@ proc extract_heap { dir } { } return $heap } -set next_heap [extract_heap next] -set prev_heap [extract_heap prev] - -# Save the total allocated size within GDB so that we can check -# the core size later. -gdb_test_no_output "set \$bytes_allocated = bytes_allocated" "save heap size" - -# Now create a core dump - -# Rename the core file to "TESTFILE.corefile" rather than just "core", -# to avoid problems with sys admin types that like to regularly prune -# all files named "core" from the system. - -# Some systems append "core" to the name of the program; others append -# the name of the program to "core"; still others (like Linux, as of -# May 2003) create cores named "core.PID". - -# Save the process ID. Some systems dump the core into core.PID. -set inferior_pid [get_inferior_pid] - -# Dump core using SIGABRT -set oldtimeout $timeout -set timeout 600 -gdb_test "signal SIGABRT" "Program terminated with signal SIGABRT, .*" -set timeout $oldtimeout - -# Find the corefile. -set file [find_core_file $inferior_pid] -if { $file != "" } { - remote_exec build "mv $file $corefile" -} else { - untested "can't generate a core file" - return 0 -} -# Check that the corefile is plausibly large enough. We're trying to -# detect the case where the operating system has truncated the file -# just before signed wraparound. TCL, unfortunately, has a similar -# problem - so use catch. It can handle the "bad" size but not -# necessarily the "good" one. And we must use GDB for the comparison, -# similarly. - -if {[catch {file size $corefile} core_size] == 0} { - set core_ok 0 - gdb_test_multiple "print \$bytes_allocated < $core_size" "check core size" { - -re " = 1\r\n$gdb_prompt $" { - pass "check core size" - set core_ok 1 - } - -re " = 0\r\n$gdb_prompt $" { - pass "check core size" - set core_ok 0 - } - } -} { - # Probably failed due to the TCL build having problems with very - # large values. Since GDB uses a 64-bit off_t (when possible) it - # shouldn't have this problem. Assume that things are going to - # work. Without this assumption the test is skiped on systems - # (such as i386 GNU/Linux with patched kernel) which do pass. - pass "check core size" - set core_ok 1 -} -if {! $core_ok} { - untested "check core size (system does not support large corefiles)" - return 0 -} - -# Now load up that core file - -set test "load corefile" -gdb_test_multiple "core $corefile" "$test" { - -re "A program is being debugged already. Kill it. .y or n. " { - send_gdb "y\n" - exp_continue - } - -re "Core was generated by.*$gdb_prompt $" { - pass "$test" - } -} - -# Finally, re-traverse bigcore's linked list, checking each chunk's -# address against the executable. Don't use gdb_test_multiple as want -# only one pass/fail. Don't use exp_continue as the regular -# expression involving $heap needs to be re-evaluated for each new -# response. +# Re-traverse bigcore's linked list, checking each chunk's address +# against the executable. Don't use gdb_test_multiple as want only +# one pass/fail. Don't use exp_continue as the regular expression +# involving $heap needs to be re-evaluated for each new response. proc check_heap { dir heap } { global gdb_prompt @@ -208,5 +110,133 @@ proc check_heap { dir heap } { } } -check_heap next $next_heap -check_heap prev $prev_heap +# The bulk of the testcase. DUMPER indicates who is supposed to dump +# the core. It can be either "kernel", or "gdb". +proc test {dumper} { + global binfile timeout corefile gdb_prompt + + # Run GDB on the bigcore program up-to where it will dump core. + + clean_restart ${binfile} + gdb_test_no_output "set print sevenbit-strings" + gdb_test_no_output "set width 0" + + # Get the core into the output directory. + set_inferior_cwd_to_output_dir + + if {![runto_main]} { + return 0 + } + set print_core_line [gdb_get_line_number "Dump core"] + gdb_test "tbreak $print_core_line" + gdb_test continue ".*print_string.*" + gdb_test next ".*0 = 0.*" + + set next_heap [extract_heap next] + set prev_heap [extract_heap prev] + + # Save the total allocated size within GDB so that we can check + # the core size later. + gdb_test_no_output "set \$bytes_allocated = bytes_allocated" \ + "save heap size" + + # Now create a core dump. + + if {$dumper == "kernel"} { + # Rename the core file to "TESTFILE.corefile.$dumper" rather + # than just "core", to avoid problems with sys admin types + # that like to regularly prune all files named "core" from the + # system. + + # Some systems append "core" to the name of the program; + # others append the name of the program to "core"; still + # others (like Linux, as of May 2003) create cores named + # "core.PID". + + # Save the process ID. Some systems dump the core into + # core.PID. + set inferior_pid [get_inferior_pid] + + # Dump core using SIGABRT. + set oldtimeout $timeout + set timeout 600 + gdb_test "signal SIGABRT" "Program terminated with signal SIGABRT, .*" + set timeout $oldtimeout + + # Find the corefile. + set file [find_core_file $inferior_pid] + if { $file != "" } { + remote_exec build "mv $file $corefile.$dumper" + } else { + untested "can't generate a core file" + return 0 + } + } elseif {$dumper == "gdb"} { + gdb_gcore_cmd "$corefile.$dumper" "gcore corefile" + } else { + error "unhandled dumper: $dumper" + } + + # Check that the corefile is plausibly large enough. We're trying + # to detect the case where the operating system has truncated the + # file just before signed wraparound. TCL, unfortunately, has a + # similar problem - so use catch. It can handle the "bad" size + # but not necessarily the "good" one. And we must use GDB for the + # comparison, similarly. + + if {[catch {file size $corefile.$dumper} core_size] == 0} { + set core_ok 0 + gdb_test_multiple "print \$bytes_allocated < $core_size" \ + "check core size" { + -re " = 1\r\n$gdb_prompt $" { + pass "check core size" + set core_ok 1 + } + -re " = 0\r\n$gdb_prompt $" { + pass "check core size" + set core_ok 0 + } + } + } { + # Probably failed due to the TCL build having problems with + # very large values. Since GDB uses a 64-bit off_t (when + # possible) it shouldn't have this problem. Assume that + # things are going to work. Without this assumption the test + # is skiped on systems (such as i386 GNU/Linux with patched + # kernel) which do pass. + pass "check core size" + set core_ok 1 + } + if {! $core_ok} { + untested "check core size (system does not support large corefiles)" + return 0 + } + + # Now load up that core file. + + set test "load corefile" + gdb_test_multiple "core $corefile.$dumper" "$test" { + -re "A program is being debugged already. Kill it. .y or n. " { + send_gdb "y\n" + exp_continue + } + -re "Core was generated by.*$gdb_prompt $" { + pass "$test" + } + } + + # Finally, re-traverse bigcore's linked list, checking each + # chunk's address against the executable. + + check_heap next $next_heap + check_heap prev $prev_heap +} + +foreach_with_prefix dumper {kernel gdb} { + # GDB's gcore is too slow when testing with the extended-gdbserver + # board, since it requires reading all the inferior memory. + if {$dumper == "gdb" && [target_info gdb_protocol] != ""} { + continue + } + test $dumper +} base-commit: d0eb2625bff1387744304bdc70ec0a85a20b8a3f -- 2.43.2
svens@stackframe.org (Sven Schnelle) writes:
> With qemu supporting 64 bit now, add some code to determine the
> register size of a hppa remote target. This first checks the
> PROPERTY_GP32/PROPERTY_GP64 flags in target_desc, and uses
> bfd_arch_info as fallback.
>
> Signed-off-by: Sven Schnelle <svens@stackframe.org>
> ---
> gdb/hppa-tdep.c | 47 ++++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 42 insertions(+), 5 deletions(-)
Gentle ping?
Thanks!
Sven
>>>>> "Will" == Will Hawkins <hawkinsw@obs.cr> writes: Will> While DW_LNCT_source is not yet finalized in the DWARF standard Will> (https://dwarfstd.org/issues/180201.1.html), LLVM does emit it. Will> This patch adds support for it in gdb. Thanks for the patch. Will> ChangeLog: gdb doesn't use ChangeLogs any more, so you can delete this part. Will> + if (fe.source) Will> + sf->symtab->source = cu->per_objfile->objfile->intern (fe.source); IIUC the 'source' here is the full text of the source code. In this case you don't want to use intern. That will make a copy of the text. Instead, because the section data is read-in and not unmapped or freed until the objfile is destroyed, you can just use the pointer directly. Will> +/* Open an embedded source file given a symtab S. Returns a file descriptor Will> + or negative errno for error. */ Will> + Will> +scoped_fd Will> +open_embedded_source(struct symtab *s, Will> + gdb::unique_xmalloc_ptr<char> *fullname) Will> +{ I think some other refactoring should be done so that gdb doesn't have to write the source to a file and then re-read it. Some kind of abstraction here would be appropriate. Will> --- a/gdb/symtab.h Will> +++ b/gdb/symtab.h Will> @@ -1755,6 +1755,8 @@ struct symtab Will> const char *filename; Will> + const char *source; This line could use an explanatory comment. Will> diff --git a/include/dwarf2.h b/include/dwarf2.h Will> index b3d3731ee83..abe0359926b 100644 Will> --- a/include/dwarf2.h Will> +++ b/include/dwarf2.h Will> @@ -288,7 +288,9 @@ enum dwarf_line_number_content_type Will> DW_LNCT_timestamp = 0x3, Will> DW_LNCT_size = 0x4, Will> DW_LNCT_MD5 = 0x5, Will> + DW_LNCT_SOURCE = 0x6, Will> DW_LNCT_lo_user = 0x2000, Will> + DW_LNCT_llvm_SOURCE = 0x2001, This should probably use "LLVM" and not "llvm". Also for vendor extensions we like to have some kind of comment or documentation explaining what it is about -- in the more distant past this wasn't done and as a result there are some mystery extensions. Also I would not assign DW_LNCT_SOURCE until DWARF officially blesses it. Tom
>>>>> "Will" == Will Hawkins <hawkinsw@obs.cr> writes:
Will> Hello everyone!
Will> First, thank you for all that you do to keep gdb the best open-source debugger
Will> available. I am a long-time user and minor contributor. This is my first
Will> major-ish contribution.
Thanks. I think you might as well get started on your copyright
assignment. Email 'assign@gnu.org' and tell them you have gdb patches
to contribute; they will tell you what to do. Be sure to let us know
when this is done.
Will> I just wanted to pass this along early to get your sense of things.
I don't have a problem with gdb supporting it. I'll send some notes on
the patch.
Tom
Hi Tom,
Sorry for the late reply. I was going to finish all the paperwork before
continuing, but it takes too long.
> Anyway what I was really asking is if you ran the gdb testsuite.
I've run the testsuite locally. When patch is applied I get one
additional 'KFAIL ingdb.threads/process-dies-while-handling-bp.exp'.
But this test seems to be flaky on my machine and if I rerun it in
master, it also fails.
Do you think I should fix the formatting and re-submit the
series, or is it better to wait till all papers are signed and
accepted?
Dmitry
>>>>> Guinevere Larsen <blarsen@redhat.com> writes: > My one nitpick is that I think this code fits more naturally right > after the pointer check, instead of the last check in the function, > but feel free to disagree. Either way: Reviewed-By: Guinevere Larsen > <blarsen@redhat.com> This would make sense too, but I'm not sure how much it matters. In most languages, array types don't have a name anyway. Also I wonder if we really want to keep all these different type-equality functions around. Right now there is types_equal, types_deeply_equal, and also ada_type_match. The Ada one is maybe hard to remove as long as GNAT encodings are relevant. However maybe we could replace types_equal with types_deeply_equal everywhere. Tom
While DW_LNCT_source is not yet finalized in the DWARF standard (https://dwarfstd.org/issues/180201.1.html), LLVM does emit it. This patch adds support for it in gdb. ChangeLog: * gdb/dwarf2/line-header.c (line_header::add_file_name): Add source as parameter. (read_formatted_entries): Handle additional source parameter. (dwarf_decode_line_header): Read DW_LNCT_[llvm]_source. * gdb/dwarf2/line-header.h (struct file_entry): Add source member. * gdb/dwarf2/read.c (dwarf_decode_lines_1): Pass through source. (dwarf_decode_lines): Assign source, if present. * gdb/source.c (open_embedded_source): New function for "opening" an embedded source file. (open_source_file): Call open_embedded_source. * gdb/source.h (open_embedded_source): Add declaration for open_embedded_source. * gdb/symtab.h (struct symtab): Add source member. * gdb/testsuite/lib/dwarf.exp: Update testing library to support adding embedded source to a file entry in a line table program. * gdb/testsuite/gdb.dwarf2/dw2-lnct-source.c: New test. * gdb/testsuite/gdb.dwarf2/dw2-lnct-source.exp: New test. Signed-off-by: Will Hawkins <hawkinsw@obs.cr> --- gdb/dwarf2/line-header.c | 22 +++-- gdb/dwarf2/line-header.h | 9 ++- gdb/dwarf2/read.c | 11 ++- gdb/source.c | 73 ++++++++++++++++- gdb/source.h | 4 + gdb/symtab.h | 2 + gdb/testsuite/gdb.dwarf2/dw2-lnct-source.c | 24 ++++++ gdb/testsuite/gdb.dwarf2/dw2-lnct-source.exp | 85 ++++++++++++++++++++ gdb/testsuite/lib/dwarf.exp | 14 +++- include/dwarf2.h | 2 + 10 files changed, 227 insertions(+), 19 deletions(-) create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-lnct-source.c create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-lnct-source.exp diff --git a/gdb/dwarf2/line-header.c b/gdb/dwarf2/line-header.c index a3ca49b64f5..837d9b5cfa4 100644 --- a/gdb/dwarf2/line-header.c +++ b/gdb/dwarf2/line-header.c @@ -45,6 +45,7 @@ line_header::add_include_dir (const char *include_dir) void line_header::add_file_name (const char *name, dir_index d_index, + const char *source, unsigned int mod_time, unsigned int length) { @@ -54,7 +55,7 @@ line_header::add_file_name (const char *name, if (dwarf_line_debug >= 2) gdb_printf (gdb_stdlog, "Adding file %d: %s\n", index, name); - m_file_names.emplace_back (name, index, d_index, mod_time, length); + m_file_names.emplace_back (name, index, d_index, source, mod_time, length); } std::string @@ -125,6 +126,7 @@ read_formatted_entries (dwarf2_per_objfile *per_objfile, bfd *abfd, void (*callback) (struct line_header *lh, const char *name, dir_index d_index, + const char *source, unsigned int mod_time, unsigned int length)) { @@ -239,13 +241,17 @@ read_formatted_entries (dwarf2_per_objfile *per_objfile, bfd *abfd, break; case DW_LNCT_MD5: break; + case DW_LNCT_llvm_SOURCE: + case DW_LNCT_SOURCE: + fe.source = *string; + break; default: complaint (_("Unknown format content type %s"), pulongest (content_type)); } } - callback (lh, fe.name, fe.d_index, fe.mod_time, fe.length); + callback (lh, fe.name, fe.d_index, fe.source, fe.mod_time, fe.length); } *bufp = buf; @@ -368,8 +374,8 @@ dwarf_decode_line_header (sect_offset sect_off, bool is_dwz, read_formatted_entries (per_objfile, abfd, &line_ptr, lh.get (), offset_size, [] (struct line_header *header, const char *name, - dir_index d_index, unsigned int mod_time, - unsigned int length) + dir_index d_index, const char *source, + unsigned int mod_time, unsigned int length) { header->add_include_dir (name); }); @@ -378,10 +384,10 @@ dwarf_decode_line_header (sect_offset sect_off, bool is_dwz, read_formatted_entries (per_objfile, abfd, &line_ptr, lh.get (), offset_size, [] (struct line_header *header, const char *name, - dir_index d_index, unsigned int mod_time, - unsigned int length) + dir_index d_index, const char *source, + unsigned int mod_time, unsigned int length) { - header->add_file_name (name, d_index, mod_time, length); + header->add_file_name (name, d_index, source, mod_time, length); }); } else @@ -408,7 +414,7 @@ dwarf_decode_line_header (sect_offset sect_off, bool is_dwz, length = read_unsigned_leb128 (abfd, line_ptr, &bytes_read); line_ptr += bytes_read; - lh->add_file_name (cur_file, d_index, mod_time, length); + lh->add_file_name (cur_file, d_index, nullptr, mod_time, length); } line_ptr += bytes_read; } diff --git a/gdb/dwarf2/line-header.h b/gdb/dwarf2/line-header.h index c068dff70a3..abc95f3ee87 100644 --- a/gdb/dwarf2/line-header.h +++ b/gdb/dwarf2/line-header.h @@ -35,9 +35,10 @@ struct file_entry file_entry () = default; file_entry (const char *name_, file_name_index index_, dir_index d_index_, - unsigned int mod_time_, unsigned int length_) + const char *source_, unsigned int mod_time_, unsigned int length_) : name (name_), index (index_), + source (source_), d_index (d_index_), mod_time (mod_time_), length (length_) @@ -54,6 +55,10 @@ struct file_entry /* The index of this file in the file table. */ file_name_index index {}; + /* The file's contents (if not null). Note this is an observing pointer. + The memory is owned by debug_line_buffer. */ + const char *source {}; + /* The directory index (1-based). */ dir_index d_index {}; @@ -88,7 +93,7 @@ struct line_header void add_include_dir (const char *include_dir); /* Add an entry to the file name table. */ - void add_file_name (const char *name, dir_index d_index, + void add_file_name (const char *name, dir_index d_index, const char *source, unsigned int mod_time, unsigned int length); /* Return the include dir at INDEX (0-based in DWARF 5 and 1-based before). diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 4afb026b8ce..80f60a54f8e 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -18426,7 +18426,7 @@ dwarf_decode_lines_1 (struct line_header *lh, struct dwarf2_cu *cu, length = read_unsigned_leb128 (abfd, line_ptr, &bytes_read); line_ptr += bytes_read; - lh->add_file_name (cur_file, dindex, mod_time, length); + lh->add_file_name (cur_file, dindex, nullptr, mod_time, length); } break; case DW_LNE_set_discriminator: @@ -18581,9 +18581,12 @@ dwarf_decode_lines (struct line_header *lh, struct dwarf2_cu *cu, subfile *sf = builder->get_current_subfile (); if (sf->symtab == nullptr) - sf->symtab = allocate_symtab (cust, sf->name.c_str (), - sf->name_for_id.c_str ()); - + { + sf->symtab = allocate_symtab (cust, sf->name.c_str (), + sf->name_for_id.c_str ()); + if (fe.source) + sf->symtab->source = cu->per_objfile->objfile->intern (fe.source); + } fe.symtab = sf->symtab; } } diff --git a/gdb/source.c b/gdb/source.c index bbeb4154258..b7f3aa65883 100644 --- a/gdb/source.c +++ b/gdb/source.c @@ -29,9 +29,11 @@ #include "gdbsupport/filestuff.h" #include <sys/types.h> +#include <sys/time.h> #include <fcntl.h> #include "gdbcore.h" #include "gdbsupport/gdb_regex.h" +#include "gdbsupport/gdb_unlinker.h" #include "symfile.h" #include "objfiles.h" #include "annotate.h" @@ -1135,6 +1137,67 @@ find_and_open_source (const char *filename, return scoped_fd (result); } +/* Open an embedded source file given a symtab S. Returns a file descriptor + or negative errno for error. */ + +scoped_fd +open_embedded_source(struct symtab *s, + gdb::unique_xmalloc_ptr<char> *fullname) +{ + if (!s->source || !strlen (s->source)) + return scoped_fd (-1); + + /* Make a temporary file in the same directory as the object file. */ + std::string dirname = ldirname (objfile_name (s->compunit ()-> objfile + ())); + std::string filename = lbasename (s->filename); + gdb::char_vector temp_name = make_temp_filename (dirname + + SLASH_STRING + + filename); + scoped_fd fd = gdb_mkostemp_cloexec (temp_name.data (), + O_TEXT); + if (fd.get () < 0) + return fd; + + /* Always unlink. Noone else needs the file by name. */ + gdb::unlinker unlink (temp_name.data ()); + + /* Write the source contents into it. */ + size_t source_len = strlen (s->source); + if (source_len != write(fd.get (), s->source, source_len)) + { + close (fd.get ()); + return scoped_fd (-1); + } + + if (lseek(fd.get (), 0, SEEK_SET)) + { + close (fd.get ()); + return scoped_fd (-1); + } + + /* Adjust its time(s) so that gdb doesn't print an awkward + message about it being more recent than the exe. */ + time_t mtime = 0; + if (s->compunit ()->objfile () != NULL + && s->compunit ()->objfile ()->obfd != NULL) + mtime = s->compunit ()->objfile ()->mtime; + else if (current_program_space->exec_bfd ()) + mtime = current_program_space->ebfd_mtime; + + struct timeval tvs[2] = {}; + tvs[0].tv_sec = mtime; + tvs[1].tv_sec = mtime; + if (futimes (fd.get (), tvs) < 0) + { + close (fd.get ()); + return scoped_fd (-1); + } + + *fullname = make_unique_xstrdup (temp_name.data ()); + return fd; +} + /* Open a source file given a symtab S. Returns a file descriptor or negative errno for error. @@ -1148,7 +1211,15 @@ open_source_file (struct symtab *s) gdb::unique_xmalloc_ptr<char> fullname (s->fullname); s->fullname = NULL; - scoped_fd fd = find_and_open_source (s->filename, s->compunit ()->dirname (), + + scoped_fd fd = open_embedded_source(s, &fullname); + if (fd.get () >= 0) + { + s->fullname = fullname.release (); + return fd; + } + + fd = find_and_open_source (s->filename, s->compunit ()->dirname (), &fullname); if (fd.get () < 0) diff --git a/gdb/source.h b/gdb/source.h index 144ee48f722..711d49749df 100644 --- a/gdb/source.h +++ b/gdb/source.h @@ -85,6 +85,10 @@ extern gdb::unique_xmalloc_ptr<char> find_source_or_rewrite negative errno indicating the reason for the failure. */ extern scoped_fd open_source_file (struct symtab *s); +extern scoped_fd +open_embedded_source(struct symtab *s, + gdb::unique_xmalloc_ptr<char> *fullname); + extern gdb::unique_xmalloc_ptr<char> rewrite_source_path (const char *path); extern const char *symtab_to_fullname (struct symtab *s); diff --git a/gdb/symtab.h b/gdb/symtab.h index 5bd63979132..4a925050d6f 100644 --- a/gdb/symtab.h +++ b/gdb/symtab.h @@ -1755,6 +1755,8 @@ struct symtab const char *filename; + const char *source; + /* Filename for this source file, used as an identifier to link with related objects such as associated macro_source_file objects. It must therefore match the name of any macro_source_file object created for this diff --git a/gdb/testsuite/gdb.dwarf2/dw2-lnct-source.c b/gdb/testsuite/gdb.dwarf2/dw2-lnct-source.c new file mode 100644 index 00000000000..6c03c84d4e7 --- /dev/null +++ b/gdb/testsuite/gdb.dwarf2/dw2-lnct-source.c @@ -0,0 +1,24 @@ +/* Copyright 2022-2024 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + + +int +main (void) +{ /* main prologue */ + asm ("main_label: .global main_label"); + int m = 42; /* main assign m */ + asm ("main_end: .global main_end"); /* main end */ + return m; +} diff --git a/gdb/testsuite/gdb.dwarf2/dw2-lnct-source.exp b/gdb/testsuite/gdb.dwarf2/dw2-lnct-source.exp new file mode 100644 index 00000000000..19deebfbbc2 --- /dev/null +++ b/gdb/testsuite/gdb.dwarf2/dw2-lnct-source.exp @@ -0,0 +1,85 @@ +# Copyright 2022-2024 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +# Check that GDB can honor LNCT_llvm_SOURCE. + +load_lib dwarf.exp + +# This test can only be run on targets which support DWARF-2 and use gas. +require dwarf2_support + +standard_testfile .c .S + +set asm_file [standard_output_file $srcfile2] + +set fp [open "${srcdir}/${subdir}/${srcfile}" r] +set srcfile_data [read $fp] +close $fp + +Dwarf::assemble $asm_file { + global srcdir subdir srcfile srcfile2 srcfile_data + declare_labels lines_label + + get_func_info main + + cu {} { + compile_unit { + {language @DW_LANG_C} + {name missing-file.c} + {stmt_list ${lines_label} DW_FORM_sec_offset} + } { + subprogram { + {external 1 flag} + {name main} + {low_pc $main_start addr} + {high_pc "$main_start + $main_len" addr} + } + } + } + + lines {version 5} lines_label { + set diridx [include_dir "${srcdir}/${subdir}"] + file_name "missing-file.c" $diridx "${srcfile_data}" + + program { + DW_LNS_set_file $diridx + DW_LNE_set_address $main_start + line [gdb_get_line_number "main prologue"] + DW_LNS_copy + + DW_LNE_set_address main_label + line [gdb_get_line_number "main assign m"] + DW_LNS_copy + + DW_LNE_set_address main_end + line [gdb_get_line_number "main end"] + DW_LNS_copy + + DW_LNE_end_sequence + } + } +} + +if { [prepare_for_testing "failed to prepare" ${testfile} \ + [list $srcfile $asm_file] {nodebug}] } { + return -1 +} + +if ![runto_main] { + return -1 +} + +set assign_m_line [gdb_get_line_number "main assign m"] +gdb_test "frame" ".*main \\\(\\\) at \[^\r\n\]*:$assign_m_line\r\n.*" diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp index d085f835f07..43ee29a7185 100644 --- a/gdb/testsuite/lib/dwarf.exp +++ b/gdb/testsuite/lib/dwarf.exp @@ -2423,9 +2423,9 @@ namespace eval Dwarf { # Add a file name entry to the line table header's file names table. # # Return the index by which this entry can be referred to. - proc file_name {filename diridx} { + proc file_name {filename diridx { source "" }} { variable _line_file_names - lappend _line_file_names $filename $diridx + lappend _line_file_names $filename $diridx $source if { $Dwarf::_line_unit_version >= 5 } { return [expr [llength $_line_file_names] - 1] @@ -2481,7 +2481,7 @@ namespace eval Dwarf { } } - _op .byte 2 "file_name_entry_format_count" + _op .byte 3 "file_name_entry_format_count" _op .uleb128 1 \ "file_name_entry_format (content type code: DW_LNCT_path)" switch $_line_string_form { @@ -2494,6 +2494,11 @@ namespace eval Dwarf { "directory_entry_format (form: DW_FORM_line_strp)" } } + _op .uleb128 0x2001 \ + "file_name_entry_format (content type code: + DW_LNCT_llvm_source)" + _op .uleb128 0x08 \ + "directory_entry_format (form: DW_FORM_string)" _op .uleb128 2 \ "file_name_entry_format (content type code: DW_LNCT_directory_index)" _op .uleb128 0x0f \ @@ -2502,7 +2507,7 @@ namespace eval Dwarf { set nr_files [expr [llength $_line_file_names] / 2] _op .byte $nr_files "file_names_count" - foreach { filename diridx } $_line_file_names { + foreach { filename diridx source } $_line_file_names { switch $_line_string_form { string { _op .ascii [_quote $filename] @@ -2516,6 +2521,7 @@ namespace eval Dwarf { _op_offset [expr $_line_is_64 ? 8 : 4] $string_ptr } } + _op .ascii [_quote [string map { "\"" "\\\"" "\n" "\\n" } "$source"]] _op .uleb128 $diridx } } else { diff --git a/include/dwarf2.h b/include/dwarf2.h index b3d3731ee83..abe0359926b 100644 --- a/include/dwarf2.h +++ b/include/dwarf2.h @@ -288,7 +288,9 @@ enum dwarf_line_number_content_type DW_LNCT_timestamp = 0x3, DW_LNCT_size = 0x4, DW_LNCT_MD5 = 0x5, + DW_LNCT_SOURCE = 0x6, DW_LNCT_lo_user = 0x2000, + DW_LNCT_llvm_SOURCE = 0x2001, DW_LNCT_hi_user = 0x3fff }; -- 2.44.0
Hello everyone! First, thank you for all that you do to keep gdb the best open-source debugger available. I am a long-time user and minor contributor. This is my first major-ish contribution. Second, I know that cover letters are not generally encouraged but because this patch is an RFC, I thought it would be a good opportunity to introduce the patch's current status. As it stands, DWARF has not finalized the DW_LNCT_source. However, LLVM does emit it, when asked. It would be great if gdb supported it. This patch adds support for the to-be-standardized version and the vendor extension that LLVM currently uses. If this is something that you think gdb would like to add, then I am happy to continue cleaning up this patch by - cleaning up the included test - adding additional tests - handling corner cases that I am sure you will discover. I just wanted to pass this along early to get your sense of things. I have attempted to follow all the coding guidelines and the best practices but I know that I have probably fallen short. Again, if this is something that you think you would like to see in gdb, I am 100% willing to learn from you and get this into better shape. No matter what, thank you again for all your volunteer work to keep gdb going. Will Will Hawkins (1): gdb: Support embedded source in DWARF gdb/dwarf2/line-header.c | 22 +++-- gdb/dwarf2/line-header.h | 9 ++- gdb/dwarf2/read.c | 11 ++- gdb/source.c | 73 ++++++++++++++++- gdb/source.h | 4 + gdb/symtab.h | 2 + gdb/testsuite/gdb.dwarf2/dw2-lnct-source.c | 24 ++++++ gdb/testsuite/gdb.dwarf2/dw2-lnct-source.exp | 85 ++++++++++++++++++++ gdb/testsuite/lib/dwarf.exp | 14 +++- include/dwarf2.h | 2 + 10 files changed, 227 insertions(+), 19 deletions(-) create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-lnct-source.c create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-lnct-source.exp -- 2.44.0
Some stubs take exception to this. For example we observe RTEMS's libdebugger freezing when asked to examine address zero on aarch64/xilinx_zynqmp_lp64_qemu. As of 2024-02-02 "gdb, types: Resolve pointer types dynamically" (f18fc7e56fb) this happens as early as 'target remote'. Ordinarily we would be greeted with… _User_extensions_Thread_switch (executing=0x0, heir=<optimized out>) at […]/cpukit/include/rtems/score/userextimpl.h:382 … but now, as language_defn::read_var_value calls resolve_dynamic_type with a "dummy" address and value, resolve_dynamic_type_internal receives a similarly "dummy" addr_stack, and attempts to read memory address zero: guard against that. --- gdb/gdbtypes.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c index 599a696839e..917c997ad8c 100644 --- a/gdb/gdbtypes.c +++ b/gdb/gdbtypes.c @@ -2805,7 +2805,7 @@ resolve_dynamic_type_internal (struct type *type, if (addr_stack->valaddr.data () != NULL) pinfo.addr = extract_typed_address (addr_stack->valaddr.data (), type); - else + else if (addr_stack->addr != 0) pinfo.addr = read_memory_typed_address (addr_stack->addr, type); pinfo.next = addr_stack; -- 2.34.1
>>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:
Hannes> This happens because types_equal doesn't handle array types, so the
Hannes> function is never even considered as a possibility.
Hannes> With array type handling added, by comparing element types and array
Hannes> bounds, the same works:
Sorry about the delay on this. This is OK.
Approved-By: Tom Tromey <tom@tromey.com>
Tom
>>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes:
Tom> ada_bitwise_operation differs from the "usual" bitwise operations only
Tom> in that it calls value_cast at the end. However, because gdb is
Tom> generally fairly lax about integer types, and because (perhaps oddly)
Tom> C-style binary promotion is done here anyway, it seems to me that this
Tom> code isn't needed.
I'm checking this in.
Tom
>>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes:
...
Tom> Instead, this patch arranges to just leave such types alone in this
Tom> situation. I don't think this should have an extra effects, because
Tom> things like array subscripting still work on thick pointers.
I'm checking this in.
Tom
>>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes:
Tom> quirk_rust_enum makes string copies to store in an unordered_map.
Tom> However, the original strings have an appropriate lifetime, and so no
Tom> copying is needed. With C++17, we can rely on string_view having a
Tom> std::hash specialization, so this patch changes this code to use
Tom> string_view rather than string.
I'm checking this in.
Tom
On 2024-03-15 18:27, Pedro Alves wrote: > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31494 > Reviewed-By: Lancelot Six <lancelot.six@amd.com> Lancelot, I didn't know whether I should keep the tag of not, since I modified the patch. I kept it. > +/* To generate sparse cores, we look at the data to write in chunks of > + this size when considering whether to skip the write. Only if we > + have a full block of this size with all zeros do we skip writing > + it. A simpler algorithm that would try to skip all zeros would > + result in potentially many more write/lseek syscalls, as normal > + data is typically sprinkled with many small holes of zeros. */ > +#define SPARSE_BLOCK_SIZE 0x1000 I tweaked this comment a little to mention the memcmp efficiency. > +/* Return true if we have a block full of zeros at DATA within the > + [DATA, DATA+SIZE) buffer, false otherwise. */ > + > +static bool > +is_all_zero_block (const gdb_byte *data, size_t size) > +{ > + if (size < SPARSE_BLOCK_SIZE) > + return false; I was looking at this, and realized that there is really no reason to force a write of zeros to the file if it happens that the section is smaller than SPARSE_BLOCK_SIZE. So I tweaked the code to do the memcmp on a min of size and SPARSE_BLOCK_SIZE. And if I do that, then I need to return the size of the all-zero block considered, so the function's prototype & name changed. > + > + /* A memcmp of a whole block is much faster than a simple for loop. > + This makes a big difference, as with a for loop, this code would > + dominate the performance and result in doubling the time to > + generate a core, at the time of writing. With an optimized > + memcmp, this doesn't even show up in the perf trace. */ > + static const gdb_byte all_zero_block[SPARSE_BLOCK_SIZE] = {}; > + return memcmp (data, all_zero_block, SPARSE_BLOCK_SIZE) == 0; > +} > + > +/* Find the next all-zero block at DATA+OFFSET within the [DATA, > + DATA+SIZE) buffer. Returns the offset to the all-zero block if > + found, or zero if not found. */ > + > +static size_t > +next_all_zero_block (const gdb_byte *data, size_t offset, size_t size) > +{ Same for this function, I needed to return the size of the all-zero block found. > +/* Wrapper around bfd_set_section_contents that avoids writing > + all-zero blocks to disk, so we create a sparse core file. */ > + > +static bool > +sparse_bfd_set_section_contents (bfd *obfd, asection *osec, > + const gdb_byte *data, > + size_t sec_offset, > + size_t size) > +{ > + /* Note, we don't have to have special handling for the case of the > + last memory region ending with zeros, because our caller always > + writes out the note section after the memory/load sections. If > + it didn't, we'd have to seek+write the last byte to make the file > + size correct. (Or add an ftruncate abstraction to bfd and call > + that.) > + > + If the blocks we skip are not aligned with the filesystem blocks, > + on filesystems with fixed blocked size, we may waste a tiny bit > + of file size, as the blocks that are adjacent to all-zero blocks > + will have to include an amount of zeros. However, in practice, > + sections have some alignment, and so SEC_OFFSET will be aligned > + too, as our caller reads-in memory in chunks of SPARSE_BLOCK_SIZE > + multiples. It's just not worth the bother to worry about > + alignment here. */ And while I was hacking on the patch again, I realized how this comment about sections being usually aligned is kind of bogus. It doesn't matter that a section's VMA is aligned. What matters is the section's file position on disk. I took a look, and indeed, _that_ is usually not aligned. I added code to align the writes/skips now. I originally thought it would too complicated, but I found out how to make bfd compute the section file offsets before writing anything. Just write 0 bytes: bool _bfd_elf_set_section_contents (bfd *abfd, sec_ptr section, const void *location, file_ptr offset, bfd_size_type count) { Elf_Internal_Shdr *hdr; if (! abfd->output_has_begun && ! _bfd_elf_compute_section_file_positions (abfd, NULL)) return false; if (!count) return true; ... I checked a few other backends, like e.g., coff_set_section_contents and they all do the same, always before the count==0 check. This makes sense, as this way, any valid bfd_set_section_contents call marks that output has begun. > + > + /* If we already know we have an all-zero block at the next > + offset, we can skip calling is_all_zero_block for it > + again. */ > + if (next_all_zero_block_offset != 0) > + data_offset += SPARSE_BLOCK_SIZE; This += is why I needed to get the size of the all-zero block found, as it can be less than SPARSE_BLOCK_SIZE after the other changes. Here's a v2, with Eli's reviewed-by added. Documentation hasn't changed. ---- >8 ---- From 461d9425e4043c82877b8784a3b26c7a9c95add9 Mon Sep 17 00:00:00 2001 From: Pedro Alves <pedro@palves.net> Date: Mon, 18 Mar 2024 13:16:10 +0000 Subject: [PATCH v2] Teach GDB to generate sparse core files (PR corefiles/31494) This commit teaches GDB's gcore command to generate sparse core files (if supported by the filesystem). To create a sparse file, all you have to do is skip writing zeros to the file, instead lseek'ing-ahead over them. The sparse logic is applied when writing the memory sections, as that's where the bulk of the data and the zeros are. The commit also tweaks gdb.base/bigcore.exp to make it exercise gdb-generated cores in addition to kernel-generated cores. We couldn't do that before, because GDB's gcore on that test's program would generate a multi-GB non-sparse core (16GB on my system). After this commit, gdb.base/bigcore.exp generates, when testing with GDB's gcore, a much smaller core file, roughly in line with what the kernel produces: real sizes: $ du --hu testsuite/outputs/gdb.base/bigcore/bigcore.corefile.* 2.2M testsuite/outputs/gdb.base/bigcore/bigcore.corefile.gdb 2.0M testsuite/outputs/gdb.base/bigcore/bigcore.corefile.kernel apparent sizes: $ du --hu --apparent-size testsuite/outputs/gdb.base/bigcore/bigcore.corefile.* 16G testsuite/outputs/gdb.base/bigcore/bigcore.corefile.gdb 16G testsuite/outputs/gdb.base/bigcore/bigcore.corefile.kernel Time to generate the core also goes down significantly. On my machine, I get: when writing to an SSD, from 21.0s, down to 8.0s when writing to an HDD, from 31.0s, down to 8.5s The changes to gdb.base/bigcore.exp are smaller than they look at first sight. It's basically mostly refactoring -- moving most of the code to a new procedure which takes as argument who should dump the core, and then calling the procedure twice. I purposedly did not modernize any of the refactored code in this patch. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31494 Reviewed-By: Lancelot Six <lancelot.six@amd.com> Reviewed-By: Eli Zaretskii <eliz@gnu.org> Change-Id: I2554a6a4a72d8c199ce31f176e0ead0c0c76cff1 --- gdb/NEWS | 4 + gdb/doc/gdb.texinfo | 3 + gdb/gcore.c | 177 ++++++++++++++++++++- gdb/testsuite/gdb.base/bigcore.exp | 238 ++++++++++++++++------------- 4 files changed, 314 insertions(+), 108 deletions(-) diff --git a/gdb/NEWS b/gdb/NEWS index d8ac0bb06a7..d1d25e4c24d 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -23,6 +23,10 @@ disassemble command will now give an error. Previously the 'b' flag would always override the 'r' flag. +gcore +generate-core-file + GDB now generates sparse core files, on systems that support it. + maintenance info line-table Add an EPILOGUE-BEGIN column to the output of the command. It indicates if the line is considered the start of the epilgoue, and thus a point at diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index f093ee269e2..9224829bd93 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -13867,6 +13867,9 @@ Produce a core dump of the inferior process. The optional argument specified, the file name defaults to @file{core.@var{pid}}, where @var{pid} is the inferior process ID. +If supported by the filesystem where the core is written to, +@value{GDBN} generates a sparse core dump file. + Note that this command is implemented only for some systems (as of this writing, @sc{gnu}/Linux, FreeBSD, Solaris, and S390). diff --git a/gdb/gcore.c b/gdb/gcore.c index 7c12aa3a777..a9303a99ab1 100644 --- a/gdb/gcore.c +++ b/gdb/gcore.c @@ -39,10 +39,21 @@ #include "gdbsupport/byte-vector.h" #include "gdbsupport/scope-exit.h" +/* To generate sparse cores, we look at the data to write in chunks of + this size when considering whether to skip the write. Only if we + have a full block of this size with all zeros do we skip writing + it. A simpler algorithm that would try to skip all zeros would + result in potentially many more write/lseek syscalls, as normal + data is typically sprinkled with many small holes of zeros. Also, + it's much more efficient to memcmp a block of data against an + all-zero buffer than to check each and every data byte against zero + one by one. */ +#define SPARSE_BLOCK_SIZE 0x1000 + /* The largest amount of memory to read from the target at once. We must throttle it to limit the amount of memory used by GDB during generate-core-file for programs with large resident data. */ -#define MAX_COPY_BYTES (1024 * 1024) +#define MAX_COPY_BYTES (256 * SPARSE_BLOCK_SIZE) static const char *default_gcore_target (void); static enum bfd_architecture default_gcore_arch (void); @@ -98,7 +109,12 @@ write_gcore_file_1 (bfd *obfd) bfd_set_section_alignment (note_sec, 0); bfd_set_section_size (note_sec, note_size); - /* Now create the memory/load sections. */ + /* Now create the memory/load sections. Note + gcore_memory_sections's sparse logic is assuming that we'll + always write something afterwards, which we do: just below, we + write the note section. So there's no need for an ftruncate-like + call to grow the file to the right size if the last memory + sections were zeros and we skipped writing them. */ if (gcore_memory_sections (obfd) == 0) error (_("gcore: failed to get corefile memory sections from target.")); @@ -567,6 +583,158 @@ objfile_find_memory_regions (struct target_ops *self, return 0; } +/* Check if we have a block full of zeros at DATA within the [DATA, + DATA+SIZE) buffer. Returns the size of the all-zero block found. + Returns at most the minimum between SIZE and SPARSE_BLOCK_SIZE. */ + +static size_t +get_all_zero_block_size (const gdb_byte *data, size_t size) +{ + size = std::min (size, (size_t) SPARSE_BLOCK_SIZE); + + /* A memcmp of a whole block is much faster than a simple for loop. + This makes a big difference, as with a for loop, this code would + dominate the performance and result in doubling the time to + generate a core, at the time of writing. With an optimized + memcmp, this doesn't even show up in the perf trace. */ + static const gdb_byte all_zero_block[SPARSE_BLOCK_SIZE] = {}; + if (memcmp (data, all_zero_block, size) == 0) + return size; + return 0; +} + +/* Basically a named-elements pair, used as return type of + find_next_all_zero_block. */ + +struct offset_and_size +{ + size_t offset; + size_t size; +}; + +/* Find the next all-zero block at DATA+OFFSET within the [DATA, + DATA+SIZE) buffer. Returns the offset and the size of the all-zero + block if found, or zero if not found. */ + +static offset_and_size +find_next_all_zero_block (const gdb_byte *data, size_t offset, size_t size) +{ + for (; offset < size; offset += SPARSE_BLOCK_SIZE) + { + size_t zero_block_size + = get_all_zero_block_size (data + offset, size - offset); + if (zero_block_size != 0) + return {offset, zero_block_size}; + } + return {0, 0}; +} + +/* Wrapper around bfd_set_section_contents that avoids writing + all-zero blocks to disk, so we create a sparse core file. + SKIP_ALIGN is a recursion helper -- if true, we'll skip aligning + the file position to SPARSE_BLOCK_SIZE. */ + +static bool +sparse_bfd_set_section_contents (bfd *obfd, asection *osec, + const gdb_byte *data, + size_t sec_offset, + size_t size, + bool skip_align = false) +{ + /* Note, we don't have to have special handling for the case of the + last memory region ending with zeros, because our caller always + writes out the note section after the memory/load sections. If + it didn't, we'd have to seek+write the last byte to make the file + size correct. (Or add an ftruncate abstraction to bfd and call + that.) */ + + if (!skip_align) + { + /* Align the all-zero block search with SPARSE_BLOCK_SIZE, to + better align with filesystem blocks. If we find we're + misaligned, then write/skip the bytes needed to make us + aligned. We do that with (one level) recursion. */ + + /* We need to know the section's file offset on disk. We can + only look at it after the bfd's 'output_has_begun' flag has + been set, as bfd hasn't computed the file offsets + otherwise. */ + if (!obfd->output_has_begun) + { + gdb_byte dummy = 0; + + /* A write forces BFD to compute the bfd's section file + positions. Zero size works for that too. */ + if (!bfd_set_section_contents (obfd, osec, &dummy, 0, 0)) + return false; + + gdb_assert (obfd->output_has_begun); + } + + /* How much we need to write/skip in order to find the next + SPARSE_BLOCK_SIZE filepos-aligned block. */ + size_t align_remainder + = (SPARSE_BLOCK_SIZE + - (osec->filepos + sec_offset) % SPARSE_BLOCK_SIZE); + + /* How much we'll actually write in the recursion call. */ + size_t align_write_size = std::min (size, align_remainder); + + if (align_write_size != 0) + { + /* Recurse, skipping the alignment code. */ + if (!sparse_bfd_set_section_contents (obfd, osec, data, + sec_offset, + align_write_size, true)) + return false; + + /* Skip over what we've written, and proceed with + assumes-aligned logic. */ + data += align_write_size; + sec_offset += align_write_size; + size -= align_write_size; + } + } + + size_t data_offset = 0; + while (data_offset < size) + { + size_t all_zero_block_size + = get_all_zero_block_size (data + data_offset, size - data_offset); + if (all_zero_block_size != 0) + data_offset += all_zero_block_size; + else + { + /* We have some non-zero data to write to file. Find the + next all-zero block within the data, and only write up to + it. */ + + offset_and_size next_all_zero_block + = find_next_all_zero_block (data, + data_offset + SPARSE_BLOCK_SIZE, + size); + size_t next_data_offset = (next_all_zero_block.offset == 0 + ? size + : next_all_zero_block.offset); + + if (!bfd_set_section_contents (obfd, osec, data + data_offset, + sec_offset + data_offset, + next_data_offset - data_offset)) + return false; + + data_offset = next_data_offset; + + /* If we already know we have an all-zero block at the next + offset, we can skip calling get_all_zero_block_size for + it again. */ + if (next_all_zero_block.offset != 0) + data_offset += next_all_zero_block.offset; + } + } + + return true; +} + static void gcore_copy_callback (bfd *obfd, asection *osec) { @@ -599,8 +767,9 @@ gcore_copy_callback (bfd *obfd, asection *osec) bfd_section_vma (osec))); break; } - if (!bfd_set_section_contents (obfd, osec, memhunk.data (), - offset, size)) + + if (!sparse_bfd_set_section_contents (obfd, osec, memhunk.data (), + offset, size)) { warning (_("Failed to write corefile contents (%s)."), bfd_errmsg (bfd_get_error ())); diff --git a/gdb/testsuite/gdb.base/bigcore.exp b/gdb/testsuite/gdb.base/bigcore.exp index 3f9ae48abf2..6c64d402502 100644 --- a/gdb/testsuite/gdb.base/bigcore.exp +++ b/gdb/testsuite/gdb.base/bigcore.exp @@ -43,23 +43,6 @@ if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {deb return -1 } -# Run GDB on the bigcore program up-to where it will dump core. - -clean_restart ${binfile} -gdb_test_no_output "set print sevenbit-strings" -gdb_test_no_output "set width 0" - -# Get the core into the output directory. -set_inferior_cwd_to_output_dir - -if {![runto_main]} { - return 0 -} -set print_core_line [gdb_get_line_number "Dump core"] -gdb_test "tbreak $print_core_line" -gdb_test continue ".*print_string.*" -gdb_test next ".*0 = 0.*" - # Traverse part of bigcore's linked list of memory chunks (forward or # backward), saving each chunk's address. @@ -92,92 +75,11 @@ proc extract_heap { dir } { } return $heap } -set next_heap [extract_heap next] -set prev_heap [extract_heap prev] - -# Save the total allocated size within GDB so that we can check -# the core size later. -gdb_test_no_output "set \$bytes_allocated = bytes_allocated" "save heap size" - -# Now create a core dump - -# Rename the core file to "TESTFILE.corefile" rather than just "core", -# to avoid problems with sys admin types that like to regularly prune -# all files named "core" from the system. - -# Some systems append "core" to the name of the program; others append -# the name of the program to "core"; still others (like Linux, as of -# May 2003) create cores named "core.PID". - -# Save the process ID. Some systems dump the core into core.PID. -set inferior_pid [get_inferior_pid] - -# Dump core using SIGABRT -set oldtimeout $timeout -set timeout 600 -gdb_test "signal SIGABRT" "Program terminated with signal SIGABRT, .*" -set timeout $oldtimeout - -# Find the corefile. -set file [find_core_file $inferior_pid] -if { $file != "" } { - remote_exec build "mv $file $corefile" -} else { - untested "can't generate a core file" - return 0 -} -# Check that the corefile is plausibly large enough. We're trying to -# detect the case where the operating system has truncated the file -# just before signed wraparound. TCL, unfortunately, has a similar -# problem - so use catch. It can handle the "bad" size but not -# necessarily the "good" one. And we must use GDB for the comparison, -# similarly. - -if {[catch {file size $corefile} core_size] == 0} { - set core_ok 0 - gdb_test_multiple "print \$bytes_allocated < $core_size" "check core size" { - -re " = 1\r\n$gdb_prompt $" { - pass "check core size" - set core_ok 1 - } - -re " = 0\r\n$gdb_prompt $" { - pass "check core size" - set core_ok 0 - } - } -} { - # Probably failed due to the TCL build having problems with very - # large values. Since GDB uses a 64-bit off_t (when possible) it - # shouldn't have this problem. Assume that things are going to - # work. Without this assumption the test is skiped on systems - # (such as i386 GNU/Linux with patched kernel) which do pass. - pass "check core size" - set core_ok 1 -} -if {! $core_ok} { - untested "check core size (system does not support large corefiles)" - return 0 -} - -# Now load up that core file - -set test "load corefile" -gdb_test_multiple "core $corefile" "$test" { - -re "A program is being debugged already. Kill it. .y or n. " { - send_gdb "y\n" - exp_continue - } - -re "Core was generated by.*$gdb_prompt $" { - pass "$test" - } -} - -# Finally, re-traverse bigcore's linked list, checking each chunk's -# address against the executable. Don't use gdb_test_multiple as want -# only one pass/fail. Don't use exp_continue as the regular -# expression involving $heap needs to be re-evaluated for each new -# response. +# Re-traverse bigcore's linked list, checking each chunk's address +# against the executable. Don't use gdb_test_multiple as want only +# one pass/fail. Don't use exp_continue as the regular expression +# involving $heap needs to be re-evaluated for each new response. proc check_heap { dir heap } { global gdb_prompt @@ -208,5 +110,133 @@ proc check_heap { dir heap } { } } -check_heap next $next_heap -check_heap prev $prev_heap +# The bulk of the testcase. DUMPER indicates who is supposed to dump +# the core. It can be either "kernel", or "gdb". +proc test {dumper} { + global binfile timeout corefile gdb_prompt + + # Run GDB on the bigcore program up-to where it will dump core. + + clean_restart ${binfile} + gdb_test_no_output "set print sevenbit-strings" + gdb_test_no_output "set width 0" + + # Get the core into the output directory. + set_inferior_cwd_to_output_dir + + if {![runto_main]} { + return 0 + } + set print_core_line [gdb_get_line_number "Dump core"] + gdb_test "tbreak $print_core_line" + gdb_test continue ".*print_string.*" + gdb_test next ".*0 = 0.*" + + set next_heap [extract_heap next] + set prev_heap [extract_heap prev] + + # Save the total allocated size within GDB so that we can check + # the core size later. + gdb_test_no_output "set \$bytes_allocated = bytes_allocated" \ + "save heap size" + + # Now create a core dump. + + if {$dumper == "kernel"} { + # Rename the core file to "TESTFILE.corefile.$dumper" rather + # than just "core", to avoid problems with sys admin types + # that like to regularly prune all files named "core" from the + # system. + + # Some systems append "core" to the name of the program; + # others append the name of the program to "core"; still + # others (like Linux, as of May 2003) create cores named + # "core.PID". + + # Save the process ID. Some systems dump the core into + # core.PID. + set inferior_pid [get_inferior_pid] + + # Dump core using SIGABRT. + set oldtimeout $timeout + set timeout 600 + gdb_test "signal SIGABRT" "Program terminated with signal SIGABRT, .*" + set timeout $oldtimeout + + # Find the corefile. + set file [find_core_file $inferior_pid] + if { $file != "" } { + remote_exec build "mv $file $corefile.$dumper" + } else { + untested "can't generate a core file" + return 0 + } + } elseif {$dumper == "gdb"} { + gdb_gcore_cmd "$corefile.$dumper" "gcore corefile" + } else { + error "unhandled dumper: $dumper" + } + + # Check that the corefile is plausibly large enough. We're trying + # to detect the case where the operating system has truncated the + # file just before signed wraparound. TCL, unfortunately, has a + # similar problem - so use catch. It can handle the "bad" size + # but not necessarily the "good" one. And we must use GDB for the + # comparison, similarly. + + if {[catch {file size $corefile.$dumper} core_size] == 0} { + set core_ok 0 + gdb_test_multiple "print \$bytes_allocated < $core_size" \ + "check core size" { + -re " = 1\r\n$gdb_prompt $" { + pass "check core size" + set core_ok 1 + } + -re " = 0\r\n$gdb_prompt $" { + pass "check core size" + set core_ok 0 + } + } + } { + # Probably failed due to the TCL build having problems with + # very large values. Since GDB uses a 64-bit off_t (when + # possible) it shouldn't have this problem. Assume that + # things are going to work. Without this assumption the test + # is skiped on systems (such as i386 GNU/Linux with patched + # kernel) which do pass. + pass "check core size" + set core_ok 1 + } + if {! $core_ok} { + untested "check core size (system does not support large corefiles)" + return 0 + } + + # Now load up that core file. + + set test "load corefile" + gdb_test_multiple "core $corefile.$dumper" "$test" { + -re "A program is being debugged already. Kill it. .y or n. " { + send_gdb "y\n" + exp_continue + } + -re "Core was generated by.*$gdb_prompt $" { + pass "$test" + } + } + + # Finally, re-traverse bigcore's linked list, checking each + # chunk's address against the executable. + + check_heap next $next_heap + check_heap prev $prev_heap +} + +foreach_with_prefix dumper {kernel gdb} { + # GDB's gcore is too slow when testing with the extended-gdbserver + # board, since it requires reading all the inferior memory. + if {$dumper == "gdb" && [target_info gdb_protocol] != ""} { + continue + } + test $dumper +} base-commit: d0eb2625bff1387744304bdc70ec0a85a20b8a3f -- 2.43.2
On 02/29/2024 04:39 PM, Tiezhu Yang wrote:
> Tiezhu Yang (7):
> gdb: syscalls: Update linux-defaults.xml.in
> gdb: syscalls: Update .xml.in files for some archs
> gdb: syscalls: Update .xml files for some archs
> gdb: syscalls: Add loongarch-linux.xml.in
> gdb: syscalls: Generate loongarch-linux.xml
> gdb: syscalls: Add loongarch case in update-linux-from-src.sh
> gdb: LoongArch: Set the correct XML syscall filename
>
> gdb/data-directory/Makefile.in | 1 +
> gdb/loongarch-linux-tdep.c | 7 +
> gdb/syscalls/amd64-linux.xml | 11 +
> gdb/syscalls/amd64-linux.xml.in | 11 +
> gdb/syscalls/i386-linux.xml | 11 +
> gdb/syscalls/i386-linux.xml.in | 11 +
> gdb/syscalls/linux-defaults.xml.in | 3 +
> gdb/syscalls/loongarch-linux.xml | 327 +++++++++++++++++++++++++
> gdb/syscalls/loongarch-linux.xml.in | 331 ++++++++++++++++++++++++++
> gdb/syscalls/mips-n32-linux.xml | 11 +
> gdb/syscalls/mips-n32-linux.xml.in | 11 +
> gdb/syscalls/mips-n64-linux.xml | 11 +
> gdb/syscalls/mips-n64-linux.xml.in | 11 +
> gdb/syscalls/mips-o32-linux.xml | 11 +
> gdb/syscalls/mips-o32-linux.xml.in | 11 +
> gdb/syscalls/ppc-linux.xml | 11 +
> gdb/syscalls/ppc-linux.xml.in | 11 +
> gdb/syscalls/ppc64-linux.xml | 11 +
> gdb/syscalls/ppc64-linux.xml.in | 11 +
> gdb/syscalls/s390-linux.xml | 12 +
> gdb/syscalls/s390-linux.xml.in | 12 +
> gdb/syscalls/s390x-linux.xml | 12 +
> gdb/syscalls/s390x-linux.xml.in | 12 +
> gdb/syscalls/sparc-linux.xml | 11 +
> gdb/syscalls/sparc-linux.xml.in | 11 +
> gdb/syscalls/sparc64-linux.xml | 11 +
> gdb/syscalls/sparc64-linux.xml.in | 11 +
> gdb/syscalls/update-linux-from-src.sh | 4 +
> 28 files changed, 919 insertions(+)
> create mode 100644 gdb/syscalls/loongarch-linux.xml
> create mode 100644 gdb/syscalls/loongarch-linux.xml.in
Hi all,
The changes are almost generated through scripts, however I think it
is necessary to make sure that the patches have been approved by the
Global Maintainers before pushing them, I am looking forward to your
reply.
Thanks,
Tiezhu
Tom de Vries <tdevries@suse.de> writes: > From: linux <linux@mxqpro.fritz.box> > > On debian 12, aarch64-linux I run into: > ... > (gdb) list .^M > No symbol table is loaded. Use the "file" command.^M > (gdb) FAIL: gdb.base/list-nodebug.exp: first 'list .' > ... > > The test-case expects some debug info, but none for main. Instead, there's no > debug info at all. > > Fix this by adding another source file to the test-case, and compiling it with > debug info. > > Tested on aarch64-linux. > > PR testsuite/31290 > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31290 > --- > gdb/testsuite/gdb.base/list-nodebug-2.c | 24 ++++++++++++++++++++++++ > gdb/testsuite/gdb.base/list-nodebug.c | 7 +++++-- > gdb/testsuite/gdb.base/list-nodebug.exp | 10 +++++++--- > 3 files changed, 36 insertions(+), 5 deletions(-) > create mode 100644 gdb/testsuite/gdb.base/list-nodebug-2.c > > diff --git a/gdb/testsuite/gdb.base/list-nodebug-2.c b/gdb/testsuite/gdb.base/list-nodebug-2.c > new file mode 100644 > index 00000000000..861e6149071 > --- /dev/null > +++ b/gdb/testsuite/gdb.base/list-nodebug-2.c > @@ -0,0 +1,24 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 2024 Free Software Foundation, Inc. > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 3 of the License, or > + (at your option) any later version. > + > + This program is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program. If not, see <http://www.gnu.org/licenses/>. */ > + > +extern int foo (void); > + > +int > +foo (void) > +{ > + return 0; > +} > diff --git a/gdb/testsuite/gdb.base/list-nodebug.c b/gdb/testsuite/gdb.base/list-nodebug.c > index 078517c011e..d4ae6787310 100644 > --- a/gdb/testsuite/gdb.base/list-nodebug.c > +++ b/gdb/testsuite/gdb.base/list-nodebug.c > @@ -15,7 +15,10 @@ > You should have received a copy of the GNU General Public License > along with this program. If not, see <http://www.gnu.org/licenses/>. */ > > -int main () > +extern int foo (void); > + > +int > +main (void) > { > - return 0; > + return foo (); > } > diff --git a/gdb/testsuite/gdb.base/list-nodebug.exp b/gdb/testsuite/gdb.base/list-nodebug.exp > index 08de05423af..274e7de0391 100644 > --- a/gdb/testsuite/gdb.base/list-nodebug.exp > +++ b/gdb/testsuite/gdb.base/list-nodebug.exp > @@ -16,13 +16,17 @@ > # Test that using the command "list" in a file with no debug information > # will not crash GDB and will give reasonable output. > > -standard_testfile .c > +standard_testfile .c -2.c > > -if {[prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \ > - {nodebug}]} { > +if {[build_executable_from_specs "failed to prepare" \ > + $testfile {} \ > + $srcfile {nodebug} \ > + $srcfile2 {debug}]} { > return -1 > } > > +clean_restart $testfile What you have is fine, but it might be nicer to write: if {[prepare_for_testing_full "failed to prepare" \ [list $testfile {} \ $srcfile {nodebug} \ $srcfile2 {debug}]]} { return -1 } and void adding the call to clean_restart. With that change: Approved-By: Andrew Burgess <aburgess@redhat.com> Thanks, Andrew > + > if {![runto_main]} { > untested "couldn't run to main" > return > > base-commit: 04d7f8a5bc3a2b4be65583b10ef100bc567faca1 > -- > 2.35.3
Am Donnerstag, 29. Februar 2024 um 14:26:16 MEZ hat Guinevere Larsen <blarsen@redhat.com> Folgendes geschrieben:
> On 09/02/2024 20:45, Hannes Domani wrote:
> > Currently it's not possible to call functions if an argument is a
> > pointer to an array:
> > ```
> > (gdb) l f
> > 1 int f (int (*x)[2])
> > 2 {
> > 3 return x[0][1];
> > 4 }
> > 5
> > 6 int main()
> > 7 {
> > 8 int a[2][2] = {{0, 1}, {2, 3}};
> > 9 return f (a);
> > 10 }
> > (gdb) p f(a)
> > Cannot resolve function f to any overloaded instance
> > ```
> >
> > This happens because types_equal doesn't handle array types, so the
> > function is never even considered as a possibility.
> >
> > With array type handling added, by comparing element types and array
> > bounds, the same works:
> > ```
> > (gdb) p f(a)
> > $1 = 1
> > ```
> >
> > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=15398
> > Co-Authored-By: Keith Seitz <keiths@redhat.com>
>
> Hi!
>
> This patch fixes the problem (for both gcc and clang) and introduces no
> regressions.
>
> My one nitpick is that I think this code fits more naturally right after
> the pointer check, instead of the last check in the function, but feel
> free to disagree. Either way: Reviewed-By: Guinevere Larsen
> <blarsen@redhat.com>
Either position is fine for me.
Hannes
Tom Tromey <tromey@adacore.com> writes: > This patch arranges to set __file__ when source'ing a Python script. > This fixes a problem that was introduced by the "source" rewrite, and > then pointed out by Lancelot Six. LGTM. Approved-By: Andrew Burgess <aburgess@redhat.com> Thanks, Andrew > --- > gdb/python/python.c | 73 +++++++++++++++++++++++++---- > gdb/testsuite/gdb.python/source2.py | 3 ++ > 2 files changed, 66 insertions(+), 10 deletions(-) > > diff --git a/gdb/python/python.c b/gdb/python/python.c > index 57f6bd571d5..e2ac315f9f5 100644 > --- a/gdb/python/python.c > +++ b/gdb/python/python.c > @@ -288,12 +288,13 @@ gdbpy_check_quit_flag (const struct extension_language_defn *extlang) > > /* Evaluate a Python command like PyRun_SimpleString, but takes a > Python start symbol, and does not automatically print the stack on > - errors. FILENAME is used to set the file name in error > - messages. */ > + errors. FILENAME is used to set the file name in error messages; > + NULL means that this is evaluating a string, not the contents of a > + file. */ > > static int > eval_python_command (const char *command, int start_symbol, > - const char *filename = "<string>") > + const char *filename = nullptr) > { > PyObject *m, *d; > > @@ -305,17 +306,69 @@ eval_python_command (const char *command, int start_symbol, > if (d == NULL) > return -1; > > + bool file_set = false; > + if (filename != nullptr) > + { > + gdbpy_ref<> file = host_string_to_python_string ("__file__"); > + if (file == nullptr) > + return -1; > + > + /* PyDict_GetItemWithError returns a borrowed reference. */ > + PyObject *found = PyDict_GetItemWithError (d, file.get ()); > + if (found == nullptr) > + { > + if (PyErr_Occurred ()) > + return -1; > + > + gdbpy_ref<> filename_obj = host_string_to_python_string (filename); > + if (filename_obj == nullptr) > + return -1; > + > + if (PyDict_SetItem (d, file.get (), filename_obj.get ()) < 0) > + return -1; > + if (PyDict_SetItemString (d, "__cached__", Py_None) < 0) > + return -1; > + > + file_set = true; > + } > + } > + > /* Use this API because it is in Python 3.2. */ > - gdbpy_ref<> code (Py_CompileStringExFlags (command, filename, start_symbol, > + gdbpy_ref<> code (Py_CompileStringExFlags (command, > + filename == nullptr > + ? "<string>" > + : filename, > + start_symbol, > nullptr, -1)); > - if (code == nullptr) > - return -1; > > - gdbpy_ref<> result (PyEval_EvalCode (code.get (), d, d)); > - if (result == nullptr) > - return -1; > + int result = -1; > + if (code != nullptr) > + { > + gdbpy_ref<> eval_result (PyEval_EvalCode (code.get (), d, d)); > + if (eval_result != nullptr) > + result = 0; > + } > + > + if (file_set) > + { > + /* If there's already an exception occurring, preserve it and > + restore it before returning from this function. */ > + std::optional<gdbpy_err_fetch> save_error; > + if (result < 0) > + save_error.emplace (); > + > + /* CPython also just ignores errors here. These should be > + expected to be exceedingly rare anyway. */ > + if (PyDict_DelItemString (d, "__file__") < 0) > + PyErr_Clear (); > + if (PyDict_DelItemString (d, "__cached__") < 0) > + PyErr_Clear (); > > - return 0; > + if (save_error.has_value ()) > + save_error->restore (); > + } > + > + return result; > } > > /* Implementation of the gdb "python-interactive" command. */ > diff --git a/gdb/testsuite/gdb.python/source2.py b/gdb/testsuite/gdb.python/source2.py > index 60d59d9056e..79dc1c26524 100644 > --- a/gdb/testsuite/gdb.python/source2.py > +++ b/gdb/testsuite/gdb.python/source2.py > @@ -15,4 +15,7 @@ > # You should have received a copy of the GNU General Public License > # along with this program. If not, see <http://www.gnu.org/licenses/>. > > +# Make sure __file__ is defined. > +assert type(__file__) == str > + > print("y%ss" % "e") > -- > 2.43.0
Tom Tromey <tromey@adacore.com> writes: > When certain DAP tests are run in a certain order, dejagnu will throw > an exception during shutdown. After adding many logging statements, I > tracked this down to kill_wait_spawned_process not clearing the > 'fileid' board_info entry, causing dejagnu to try to wait for the > process a second time -- and fail. > > Tom de Vries then pointed out a second instance of this, which I > tracked down to the same problem occurring when spawning gdbserver. > > This version of the patch fixes this by adding a new proc that can be > used to clean up board_info after waiting for a process to exit. I'm > not sure why this problem hasn't affected the test suite in the past. > > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31435 I took a look at this and I think this solution seems to make sense. Approved-By: Andrew Burgess <aburgess@redhat.com> Thanks, Andrew > --- > gdb/testsuite/lib/gdb.exp | 15 +++++++++++++++ > gdb/testsuite/lib/gdbserver-support.exp | 1 + > 2 files changed, 16 insertions(+) > > diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp > index fe4ac7d2719..4d27b60ef49 100644 > --- a/gdb/testsuite/lib/gdb.exp > +++ b/gdb/testsuite/lib/gdb.exp > @@ -3310,6 +3310,20 @@ proc with_spawn_id { spawn_id body } { > } > } > > +# DejaGNU records spawn ids in a global array and tries to wait for > +# them when exiting. Sometimes this caused problems if gdb's test > +# suite has already waited for the particular spawn id. And, dejagnu > +# only seems to allow a single spawn id per "machine". This proc can > +# be used to clean up after a spawn id has been closed. > +proc clean_up_spawn_id {host id} { > + global board_info > + set name [board_info $host name] > + if {[info exists board_info($name,fileid)] > + && $board_info($name,fileid) == $id} { > + unset -nocomplain board_info($name,fileid) > + } > +} > + > # Select the largest timeout from all the timeouts: > # - the local "timeout" variable of the scope two levels above, > # - the global "timeout" variable, > @@ -6194,6 +6208,7 @@ proc kill_wait_spawned_process { proc_spawn_id } { > # wait for the PID in the background. That's fine because we > # don't care about the exit status. */ > wait -nowait -i $proc_spawn_id > + clean_up_spawn_id target $proc_spawn_id > } > > # Returns the process id corresponding to the given spawn id. > diff --git a/gdb/testsuite/lib/gdbserver-support.exp b/gdb/testsuite/lib/gdbserver-support.exp > index 8bcf4fbbb01..e8ab057647d 100644 > --- a/gdb/testsuite/lib/gdbserver-support.exp > +++ b/gdb/testsuite/lib/gdbserver-support.exp > @@ -433,6 +433,7 @@ proc close_gdbserver {} { > # -nowait makes expect tell Tcl to wait for the process in the > # background. > catch "wait -nowait -i $server_spawn_id" > + clean_up_spawn_id target $server_spawn_id > unset server_spawn_id > } > > -- > 2.43.0
From: linux <linux@mxqpro.fritz.box> On debian 12, aarch64-linux I run into: ... (gdb) list .^M No symbol table is loaded. Use the "file" command.^M (gdb) FAIL: gdb.base/list-nodebug.exp: first 'list .' ... The test-case expects some debug info, but none for main. Instead, there's no debug info at all. Fix this by adding another source file to the test-case, and compiling it with debug info. Tested on aarch64-linux. PR testsuite/31290 Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31290 --- gdb/testsuite/gdb.base/list-nodebug-2.c | 24 ++++++++++++++++++++++++ gdb/testsuite/gdb.base/list-nodebug.c | 7 +++++-- gdb/testsuite/gdb.base/list-nodebug.exp | 10 +++++++--- 3 files changed, 36 insertions(+), 5 deletions(-) create mode 100644 gdb/testsuite/gdb.base/list-nodebug-2.c diff --git a/gdb/testsuite/gdb.base/list-nodebug-2.c b/gdb/testsuite/gdb.base/list-nodebug-2.c new file mode 100644 index 00000000000..861e6149071 --- /dev/null +++ b/gdb/testsuite/gdb.base/list-nodebug-2.c @@ -0,0 +1,24 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2024 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +extern int foo (void); + +int +foo (void) +{ + return 0; +} diff --git a/gdb/testsuite/gdb.base/list-nodebug.c b/gdb/testsuite/gdb.base/list-nodebug.c index 078517c011e..d4ae6787310 100644 --- a/gdb/testsuite/gdb.base/list-nodebug.c +++ b/gdb/testsuite/gdb.base/list-nodebug.c @@ -15,7 +15,10 @@ You should have received a copy of the GNU General Public License along with this program. If not, see <http://www.gnu.org/licenses/>. */ -int main () +extern int foo (void); + +int +main (void) { - return 0; + return foo (); } diff --git a/gdb/testsuite/gdb.base/list-nodebug.exp b/gdb/testsuite/gdb.base/list-nodebug.exp index 08de05423af..274e7de0391 100644 --- a/gdb/testsuite/gdb.base/list-nodebug.exp +++ b/gdb/testsuite/gdb.base/list-nodebug.exp @@ -16,13 +16,17 @@ # Test that using the command "list" in a file with no debug information # will not crash GDB and will give reasonable output. -standard_testfile .c +standard_testfile .c -2.c -if {[prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \ - {nodebug}]} { +if {[build_executable_from_specs "failed to prepare" \ + $testfile {} \ + $srcfile {nodebug} \ + $srcfile2 {debug}]} { return -1 } +clean_restart $testfile + if {![runto_main]} { untested "couldn't run to main" return base-commit: 04d7f8a5bc3a2b4be65583b10ef100bc567faca1 -- 2.35.3