From de7985c3cca1358b21b49a9872455e2032f48ee3 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Fri, 11 Aug 2017 12:11:28 +0100 Subject: [PATCH] More gdb/skip.c C++ification - Make skiplist_entry a class with private data members. - Move all construction logic to the ctor. - Make skip_file_p etc be methods of skiplist_entry. - Use std::list for the skip entries chain. Make the list own its elements. - Get rid of the ALL_SKIPLIST_ENTRIES/ALL_SKIPLIST_ENTRIES_SAFE macros, use range-for / iterators instead. - function_name_is_marked_for_skip 'function_sal' argument must be non-NULL, so make it a reference instead. All skiplist_entry invariants are now controlled by skiplist_entry methods/internals. Some gdb_asserts disappear for being redundant. gdb/ChangeLog: 2017-08-11 Pedro Alves * infrun.c (process_event_stop_test): Adjust function_name_is_marked_for_skip call. * skip.c: Include . (skiplist_entry): Make it a class with private fields, and getters/setters. (skiplist_entry_chain): Delete. (skiplist_entries): New. (skiplist_entry_count): Delete. (highest_skiplist_entry_num): New. (ALL_SKIPLIST_ENTRIES, ALL_SKIPLIST_ENTRIES_SAFE): Delete. (add_skiplist_entry): Delete. (skiplist_entry::skiplist_entry): New. (skiplist_entry::add_entry): New. (skip_file_command, skip_function): Adjust. (compile_skip_regexp): Delete. (skip_command): Don't compile regexp here. Adjust to use skiplist_entry::add_entry. (skip_info): Adjust to use range-for and getters. (skip_enable_command, skip_disable_command): Adjust to use range-for and setters. (skip_delete_command): Adjust to use std::list. (add_skiplist_entry): Delete. (skip_file_p): Delete, refactored as ... (skiplist_entry::do_skip_file_p): ... this new method. (skip_gfile_p): Delete, refactored as ... (skiplist_entry::do_gskip_file_p): ... this new method. (skip_function_p, skip_rfunction_p): Delete, refactored as ... (skiplist_entry::skip_function_p): ... this new method. (function_name_is_marked_for_skip): Now returns bool, and takes the function sal by const reference. Adjust to use range-for and skiplist_entry methods. (_initialize_step_skip): Remove references to skiplist_entry_chain, skiplist_entry_count. * skip.h (function_name_is_marked_for_skip): Now returns bool, and takes the function sal by const reference. --- gdb/ChangeLog | 38 +++++ gdb/infrun.c | 2 +- gdb/skip.c | 433 +++++++++++++++++++++++--------------------------- gdb/skip.h | 8 +- 4 files changed, 243 insertions(+), 238 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 75a1ea80f5f..c5882914272 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,41 @@ +2017-08-11 Pedro Alves + + * infrun.c (process_event_stop_test): Adjust + function_name_is_marked_for_skip call. + * skip.c: Include . + (skiplist_entry): Make it a class with private fields, and + getters/setters. + (skiplist_entry_chain): Delete. + (skiplist_entries): New. + (skiplist_entry_count): Delete. + (highest_skiplist_entry_num): New. + (ALL_SKIPLIST_ENTRIES, ALL_SKIPLIST_ENTRIES_SAFE): Delete. + (add_skiplist_entry): Delete. + (skiplist_entry::skiplist_entry): New. + (skiplist_entry::add_entry): New. + (skip_file_command, skip_function): Adjust. + (compile_skip_regexp): Delete. + (skip_command): Don't compile regexp here. Adjust to use + skiplist_entry::add_entry. + (skip_info): Adjust to use range-for and getters. + (skip_enable_command, skip_disable_command): Adjust to use + range-for and setters. + (skip_delete_command): Adjust to use std::list. + (add_skiplist_entry): Delete. + (skip_file_p): Delete, refactored as ... + (skiplist_entry::do_skip_file_p): ... this new method. + (skip_gfile_p): Delete, refactored as ... + (skiplist_entry::do_gskip_file_p): ... this new method. + (skip_function_p, skip_rfunction_p): Delete, refactored as ... + (skiplist_entry::skip_function_p): ... this new method. + (function_name_is_marked_for_skip): Now returns bool, and takes + the function sal by const reference. Adjust to use range-for and + skiplist_entry methods. + (_initialize_step_skip): Remove references to + skiplist_entry_chain, skiplist_entry_count. + * skip.h (function_name_is_marked_for_skip): Now returns bool, and + takes the function sal by const reference. + 2017-08-11 Yao Qi * dwarf2-frame.c (clear_pointer_cleanup): Remove. diff --git a/gdb/infrun.c b/gdb/infrun.c index 8f966e2c449..d6723fde85c 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -6788,7 +6788,7 @@ process_event_stop_test (struct execution_control_state *ecs) tmp_sal = find_pc_line (ecs->stop_func_start, 0); if (tmp_sal.line != 0 && !function_name_is_marked_for_skip (ecs->stop_func_name, - &tmp_sal)) + tmp_sal)) { if (execution_direction == EXEC_REVERSE) handle_step_into_function_backward (gdbarch, ecs); diff --git a/gdb/skip.c b/gdb/skip.c index 6ab6c91ba7c..9f772486cef 100644 --- a/gdb/skip.c +++ b/gdb/skip.c @@ -35,74 +35,129 @@ #include "fnmatch.h" #include "gdb_regex.h" #include "common/gdb_optional.h" +#include -struct skiplist_entry +class skiplist_entry { - skiplist_entry (bool file_is_glob_, std::string &&file_, - bool function_is_regexp_, std::string &&function_) - : number (-1), - file_is_glob (file_is_glob_), - file (std::move (file_)), - function_is_regexp (function_is_regexp_), - function (std::move (function_)), - enabled (true), - next (NULL) - { - } - - int number; +public: + /* Create a skiplist_entry object and add it to the chain. */ + static void add_entry (bool file_is_glob, + std::string &&file, + bool function_is_regexp, + std::string &&function); + + /* Return true if the skip entry has a file or glob-style file + pattern that matches FUNCTION_SAL. */ + bool skip_file_p (const symtab_and_line &function_sal) const; + + /* Return true if the skip entry has a function or function regexp + that matches FUNCTION_NAME. */ + bool skip_function_p (const char *function_name) const; + + /* Getters. */ + int number () const { return m_number; }; + bool enabled () const { return m_enabled; }; + bool file_is_glob () const { return m_file_is_glob; } + const std::string &file () const { return m_file; } + const std::string &function () const { return m_function; } + bool function_is_regexp () const { return m_function_is_regexp; } + + /* Setters. */ + void enable () { m_enabled = true; }; + void disable () { m_enabled = false; }; + + /* Disable copy. */ + skiplist_entry (const skiplist_entry &) = delete; + void operator= (const skiplist_entry &) = delete; + +private: + /* Key that grants access to the constructor. */ + struct private_key {}; +public: + /* Public so we can construct with container::emplace_back. Since + it requires a private class key, it can't be called from outside. + Use the add_entry static factory method to construct instead. */ + skiplist_entry (bool file_is_glob, std::string &&file, + bool function_is_regexp, std::string &&function, + private_key); + +private: + /* Return true if we're stopped at a file to be skipped. */ + bool do_skip_file_p (const symtab_and_line &function_sal) const; + + /* Return true if we're stopped at a globbed file to be skipped. */ + bool do_skip_gfile_p (const symtab_and_line &function_sal) const; + +private: /* data */ + int m_number = -1; /* True if FILE is a glob-style pattern. Otherwise it is the plain file name (possibly with directories). */ - bool file_is_glob; + bool m_file_is_glob; /* The name of the file or empty if no name. */ - std::string file; + std::string m_file; /* True if FUNCTION is a regexp. Otherwise it is a plain function name (possibly with arguments, for C++). */ - bool function_is_regexp; + bool m_function_is_regexp; /* The name of the function or empty if no name. */ - std::string function; + std::string m_function; /* If this is a function regexp, the compiled form. */ - gdb::optional compiled_function_regexp; + gdb::optional m_compiled_function_regexp; - bool enabled; - - struct skiplist_entry *next; + /* Enabled/disabled state. */ + bool m_enabled = true; }; -static void add_skiplist_entry (std::unique_ptr &&e); +static std::list skiplist_entries; +static int highest_skiplist_entry_num = 0; + +skiplist_entry::skiplist_entry (bool file_is_glob, + std::string &&file, + bool function_is_regexp, + std::string &&function, + private_key) + : m_file_is_glob (file_is_glob), + m_file (std::move (file)), + m_function_is_regexp (function_is_regexp), + m_function (std::move (function)) +{ + gdb_assert (!m_file.empty () || !m_function.empty ()); -static struct skiplist_entry *skiplist_entry_chain; -static int skiplist_entry_count; + if (m_file_is_glob) + gdb_assert (!m_file.empty ()); -#define ALL_SKIPLIST_ENTRIES(E) \ - for (E = skiplist_entry_chain; E; E = E->next) + if (m_function_is_regexp) + { + gdb_assert (!m_function.empty ()); -#define ALL_SKIPLIST_ENTRIES_SAFE(E,TMP) \ - for (E = skiplist_entry_chain; \ - E ? (TMP = E->next, 1) : 0; \ - E = TMP) + int flags = REG_NOSUB; +#ifdef REG_EXTENDED + flags |= REG_EXTENDED; +#endif -/* Create a skip object. */ + gdb_assert (!m_function.empty ()); + m_compiled_function_regexp.emplace (m_function.c_str (), flags, + _("regexp")); + } +} -static std::unique_ptr -make_skip_entry (bool file_is_glob, std::string &&file, - bool function_is_regexp, std::string &&function) +void +skiplist_entry::add_entry (bool file_is_glob, std::string &&file, + bool function_is_regexp, std::string &&function) { - gdb_assert (!file.empty () || !function.empty ()); - if (file_is_glob) - gdb_assert (!file.empty ()); - if (function_is_regexp) - gdb_assert (!function.empty ()); - - return std::unique_ptr - (new skiplist_entry (file_is_glob, std::move (file), - function_is_regexp, std::move (function))); + skiplist_entries.emplace_back (file_is_glob, + std::move (file), + function_is_regexp, + std::move (function), + private_key {}); + + /* Incremented after push_back, in case push_back throws. */ + skiplist_entries.back ().m_number = ++highest_skiplist_entry_num; } static void @@ -126,8 +181,8 @@ skip_file_command (char *arg, int from_tty) else filename = arg; - add_skiplist_entry (make_skip_entry (false, std::string (filename), false, - std::string ())); + skiplist_entry::add_entry (false, std::string (filename), + false, std::string ()); printf_filtered (_("File %s will be skipped when stepping.\n"), filename); } @@ -138,8 +193,7 @@ skip_file_command (char *arg, int from_tty) static void skip_function (const char *name) { - add_skiplist_entry (make_skip_entry (false, std::string (), false, - std::string (name))); + skiplist_entry::add_entry (false, std::string (), false, std::string (name)); printf_filtered (_("Function %s will be skipped when stepping.\n"), name); } @@ -169,23 +223,6 @@ skip_function_command (char *arg, int from_tty) skip_function (arg); } -/* Compile the regexp in E. - An error is thrown if there's an error. - MESSAGE is used as a prefix of the error message. */ - -static void -compile_skip_regexp (struct skiplist_entry *e, const char *message) -{ - int flags = REG_NOSUB; - -#ifdef REG_EXTENDED - flags |= REG_EXTENDED; -#endif - - gdb_assert (e->function_is_regexp && !e->function.empty ()); - e->compiled_function_regexp.emplace (e->function.c_str (), flags, message); -} - /* Process "skip ..." that does not match "skip file" or "skip function". */ static void @@ -273,18 +310,15 @@ skip_command (char *arg, int from_tty) entry_file = file; else if (gfile != NULL) entry_file = gfile; + std::string entry_function; if (function != NULL) entry_function = function; else if (rfunction != NULL) entry_function = rfunction; - std::unique_ptr e - = make_skip_entry (gfile != NULL, std::move (entry_file), - rfunction != NULL, std::move (entry_function)); - if (rfunction != NULL) - compile_skip_regexp (e.get (), _("regexp")); - add_skiplist_entry (std::move (e)); + skiplist_entry::add_entry (gfile != NULL, std::move (entry_file), + rfunction != NULL, std::move (entry_function)); /* I18N concerns drive some of the choices here (we can't piece together the output too much). OTOH we want to keep this simple. Therefore the @@ -321,7 +355,6 @@ skip_command (char *arg, int from_tty) static void skip_info (char *arg, int from_tty) { - struct skiplist_entry *e; int num_printable_entries = 0; struct value_print_options opts; @@ -329,8 +362,8 @@ skip_info (char *arg, int from_tty) /* Count the number of rows in the table and see if we need space for a 64-bit address anywhere. */ - ALL_SKIPLIST_ENTRIES (e) - if (arg == NULL || number_is_in_list (arg, e->number)) + for (const skiplist_entry &e : skiplist_entries) + if (arg == NULL || number_is_in_list (arg, e.number ())) num_printable_entries++; if (num_printable_entries == 0) @@ -355,37 +388,36 @@ skip_info (char *arg, int from_tty) current_uiout->table_header (40, ui_noalign, "function", "Function"); /* 6 */ current_uiout->table_body (); - ALL_SKIPLIST_ENTRIES (e) + for (const skiplist_entry &e : skiplist_entries) { - QUIT; - if (arg != NULL && !number_is_in_list (arg, e->number)) + if (arg != NULL && !number_is_in_list (arg, e.number ())) continue; ui_out_emit_tuple tuple_emitter (current_uiout, "blklst-entry"); - current_uiout->field_int ("number", e->number); /* 1 */ + current_uiout->field_int ("number", e.number ()); /* 1 */ - if (e->enabled) + if (e.enabled ()) current_uiout->field_string ("enabled", "y"); /* 2 */ else current_uiout->field_string ("enabled", "n"); /* 2 */ - if (e->file_is_glob) + if (e.file_is_glob ()) current_uiout->field_string ("regexp", "y"); /* 3 */ else current_uiout->field_string ("regexp", "n"); /* 3 */ current_uiout->field_string ("file", - e->file.empty () ? "" - : e->file.c_str ()); /* 4 */ - if (e->function_is_regexp) + e.file ().empty () ? "" + : e.file ().c_str ()); /* 4 */ + if (e.function_is_regexp ()) current_uiout->field_string ("regexp", "y"); /* 5 */ else current_uiout->field_string ("regexp", "n"); /* 5 */ current_uiout->field_string ("function", - e->function.empty () ? "" - : e->function.c_str ()); /* 6 */ + e.function ().empty () ? "" + : e.function ().c_str ()); /* 6 */ current_uiout->text ("\n"); } @@ -394,14 +426,13 @@ skip_info (char *arg, int from_tty) static void skip_enable_command (char *arg, int from_tty) { - struct skiplist_entry *e; - int found = 0; + bool found = false; - ALL_SKIPLIST_ENTRIES (e) - if (arg == NULL || number_is_in_list (arg, e->number)) + for (skiplist_entry &e : skiplist_entries) + if (arg == NULL || number_is_in_list (arg, e.number ())) { - e->enabled = true; - found = 1; + e.enable (); + found = true; } if (!found) @@ -411,14 +442,13 @@ skip_enable_command (char *arg, int from_tty) static void skip_disable_command (char *arg, int from_tty) { - struct skiplist_entry *e; - int found = 0; + bool found = false; - ALL_SKIPLIST_ENTRIES (e) - if (arg == NULL || number_is_in_list (arg, e->number)) + for (skiplist_entry &e : skiplist_entries) + if (arg == NULL || number_is_in_list (arg, e.number ())) { - e->enabled = false; - found = 1; + e.disable (); + found = true; } if (!found) @@ -428,104 +458,62 @@ skip_disable_command (char *arg, int from_tty) static void skip_delete_command (char *arg, int from_tty) { - struct skiplist_entry *e, *temp, *b_prev; - int found = 0; + bool found = false; - b_prev = 0; - ALL_SKIPLIST_ENTRIES_SAFE (e, temp) - if (arg == NULL || number_is_in_list (arg, e->number)) - { - if (b_prev != NULL) - b_prev->next = e->next; - else - skiplist_entry_chain = e->next; + for (auto it = skiplist_entries.begin (), + end = skiplist_entries.end (); + it != end;) + { + const skiplist_entry &e = *it; - delete e; - found = 1; - } - else - { - b_prev = e; - } + if (arg == NULL || number_is_in_list (arg, e.number ())) + { + it = skiplist_entries.erase (it); + found = true; + } + else + ++it; + } if (!found) error (_("No skiplist entries found with number %s."), arg); } -/* Add the given skiplist entry to our list, and set the entry's number. */ - -static void -add_skiplist_entry (std::unique_ptr &&e) -{ - struct skiplist_entry *e1; - - e->number = ++skiplist_entry_count; - - /* Add to the end of the chain so that the list of - skiplist entries will be in numerical order. */ - - e1 = skiplist_entry_chain; - if (e1 == NULL) - skiplist_entry_chain = e.release (); - else - { - while (e1->next) - e1 = e1->next; - e1->next = e.release (); - } -} - -/* Return non-zero if we're stopped at a file to be skipped. */ - -static int -skip_file_p (struct skiplist_entry *e, - const struct symtab_and_line *function_sal) +bool +skiplist_entry::do_skip_file_p (const symtab_and_line &function_sal) const { - gdb_assert (!e->file.empty () && !e->file_is_glob); - - if (function_sal->symtab == NULL) - return 0; - /* Check first sole SYMTAB->FILENAME. It may not be a substring of symtab_to_fullname as it may contain "./" etc. */ - if (compare_filenames_for_search (function_sal->symtab->filename, - e->file.c_str ())) - return 1; + if (compare_filenames_for_search (function_sal.symtab->filename, + m_file.c_str ())) + return true; /* Before we invoke realpath, which can get expensive when many files are involved, do a quick comparison of the basenames. */ if (!basenames_may_differ - && filename_cmp (lbasename (function_sal->symtab->filename), - lbasename (e->file.c_str ())) != 0) - return 0; + && filename_cmp (lbasename (function_sal.symtab->filename), + lbasename (m_file.c_str ())) != 0) + return false; /* Note: symtab_to_fullname caches its result, thus we don't have to. */ { - const char *fullname = symtab_to_fullname (function_sal->symtab); + const char *fullname = symtab_to_fullname (function_sal.symtab); - if (compare_filenames_for_search (fullname, e->file.c_str ())) - return 1; + if (compare_filenames_for_search (fullname, m_file.c_str ())) + return true; } - return 0; + return false; } -/* Return non-zero if we're stopped at a globbed file to be skipped. */ - -static int -skip_gfile_p (struct skiplist_entry *e, - const struct symtab_and_line *function_sal) +bool +skiplist_entry::do_skip_gfile_p (const symtab_and_line &function_sal) const { - gdb_assert (!e->file.empty () && e->file_is_glob); - - if (function_sal->symtab == NULL) - return 0; - /* Check first sole SYMTAB->FILENAME. It may not be a substring of symtab_to_fullname as it may contain "./" etc. */ - if (gdb_filename_fnmatch (e->file.c_str (), function_sal->symtab->filename, + if (gdb_filename_fnmatch (m_file.c_str (), function_sal.symtab->filename, FNM_FILE_NAME | FNM_NOESCAPE) == 0) - return 1; + return true; /* Before we invoke symtab_to_fullname, which is expensive, do a quick comparison of the basenames. @@ -533,101 +521,83 @@ skip_gfile_p (struct skiplist_entry *e, If the basename of the glob pattern is something like "*.c" then this isn't much of a win. Oh well. */ if (!basenames_may_differ - && gdb_filename_fnmatch (lbasename (e->file.c_str ()), - lbasename (function_sal->symtab->filename), + && gdb_filename_fnmatch (lbasename (m_file.c_str ()), + lbasename (function_sal.symtab->filename), FNM_FILE_NAME | FNM_NOESCAPE) != 0) - return 0; + return false; /* Note: symtab_to_fullname caches its result, thus we don't have to. */ { - const char *fullname = symtab_to_fullname (function_sal->symtab); + const char *fullname = symtab_to_fullname (function_sal.symtab); - if (compare_glob_filenames_for_search (fullname, e->file.c_str ())) - return 1; + if (compare_glob_filenames_for_search (fullname, m_file.c_str ())) + return true; } - return 0; + return false; } -/* Return non-zero if we're stopped at a function to be skipped. */ - -static int -skip_function_p (struct skiplist_entry *e, const char *function_name) +bool +skiplist_entry::skip_file_p (const symtab_and_line &function_sal) const { - gdb_assert (!e->function.empty () && !e->function_is_regexp); - return strcmp_iw (function_name, e->function.c_str ()) == 0; -} + if (m_file.empty ()) + return false; -/* Return non-zero if we're stopped at a function regexp to be skipped. */ + if (function_sal.symtab == NULL) + return false; + + if (m_file_is_glob) + return do_skip_gfile_p (function_sal); + else + return do_skip_file_p (function_sal); +} -static int -skip_rfunction_p (struct skiplist_entry *e, const char *function_name) +bool +skiplist_entry::skip_function_p (const char *function_name) const { - gdb_assert (!e->function.empty () && e->function_is_regexp - && e->compiled_function_regexp); - return (e->compiled_function_regexp->exec (function_name, 0, NULL, 0) - == 0); + if (m_function.empty ()) + return false; + + if (m_function_is_regexp) + { + gdb_assert (m_compiled_function_regexp); + return (m_compiled_function_regexp->exec (function_name, 0, NULL, 0) + == 0); + } + else + return strcmp_iw (function_name, m_function.c_str ()) == 0; } /* See skip.h. */ -int +bool function_name_is_marked_for_skip (const char *function_name, - const struct symtab_and_line *function_sal) + const symtab_and_line &function_sal) { - struct skiplist_entry *e; - if (function_name == NULL) - return 0; + return false; - ALL_SKIPLIST_ENTRIES (e) + for (const skiplist_entry &e : skiplist_entries) { - int skip_by_file = 0; - int skip_by_function = 0; - - if (!e->enabled) + if (!e.enabled ()) continue; - if (!e->file.empty ()) - { - if (e->file_is_glob) - { - if (skip_gfile_p (e, function_sal)) - skip_by_file = 1; - } - else - { - if (skip_file_p (e, function_sal)) - skip_by_file = 1; - } - } - if (!e->function.empty ()) - { - if (e->function_is_regexp) - { - if (skip_rfunction_p (e, function_name)) - skip_by_function = 1; - } - else - { - if (skip_function_p (e, function_name)) - skip_by_function = 1; - } - } + bool skip_by_file = e.skip_file_p (function_sal); + bool skip_by_function = e.skip_function_p (function_name); /* If both file and function must match, make sure we don't errantly exit if only one of them match. */ - if (!e->file.empty () && !e->function.empty ()) + if (!e.file ().empty () && !e.function ().empty ()) { if (skip_by_file && skip_by_function) - return 1; + return true; } /* Only one of file/function is specified. */ else if (skip_by_file || skip_by_function) - return 1; + return true; } - return 0; + return false; } /* Provide a prototype to silence -Wmissing-prototypes. */ @@ -639,9 +609,6 @@ _initialize_step_skip (void) static struct cmd_list_element *skiplist = NULL; struct cmd_list_element *c; - skiplist_entry_chain = 0; - skiplist_entry_count = 0; - add_prefix_cmd ("skip", class_breakpoint, skip_command, _("\ Ignore a function while stepping.\n\ \n\ diff --git a/gdb/skip.h b/gdb/skip.h index dbc92d809eb..4e1b544dd74 100644 --- a/gdb/skip.h +++ b/gdb/skip.h @@ -20,9 +20,9 @@ struct symtab_and_line; -/* Returns 1 if the given FUNCTION_NAME is marked for skip and shouldn't be - stepped into. Otherwise, returns 0. */ -int function_name_is_marked_for_skip (const char *function_name, - const struct symtab_and_line *function_sal); +/* Returns true if the given FUNCTION_NAME is marked for skip and + shouldn't be stepped into. Otherwise, returns false. */ +bool function_name_is_marked_for_skip (const char *function_name, + const symtab_and_line &function_sal); #endif /* !defined (SKIP_H) */ -- 2.30.2