On 10/17/24 21:09, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
>
> Tom> Creating a registry key using emplace calls new:
> Tom> ...
> Tom> DATA *result = new DATA (std::forward<Args> (args)...);
> Tom> ...
> Tom> which can throw a bad alloc, which will terminate gdb if called from a
> Tom> destructor.
>
> I don't think gdb generally tries to be robust if allocation fails.
>
I suppose that is warned about with the "further debugging may prove
unreliable" message, but I don't see any reason not to attempt to make
things more robust.
> Tom> static struct ada_tasks_pspace_data *
> Tom> -get_ada_tasks_pspace_data (struct program_space *pspace)
> Tom> +get_ada_tasks_pspace_data (struct program_space *pspace, bool create = true)
> Tom> {
>
> In many of these cases, it is fine to just delete the data.
>
I'm not entirely sure what you mean, but having updated the patch to
drop the create parameter, I hope it's no longer relevant.
> Tom> data = ada_tasks_pspace_data_handle.get (pspace);
> Tom> if (data == NULL)
>
> In cases where it is not, the caller can use .get() instead and do
> nothing when the data hasn't been set. IMO this is preferable to adding
> optional 'bool create' flags.
I've updated the patch this way, submitted a v2, not the whole series,
just this patch (
https://sourceware.org/pipermail/gdb-patches/2024-October/212472.html ).
That approach didn't work seamlessly in objfile::~objfile. I could have
exported current_source_key in source.h to make that work, but instead I
chose to introduce a clear_current_source_symtab_and_line variant.
Thanks,
- Tom
@@ -287,18 +287,21 @@ task_to_str (int taskno, const ada_task_info *task_info)
}
/* Return the ada-tasks module's data for the given program space (PSPACE).
- If none is found, add a zero'ed one now.
-
- This function always returns a valid object. */
+ If none is found, add a zero'ed one now, unless !CREATE. */
static struct ada_tasks_pspace_data *
-get_ada_tasks_pspace_data (struct program_space *pspace)
+get_ada_tasks_pspace_data (struct program_space *pspace, bool create = true)
{
struct ada_tasks_pspace_data *data;
data = ada_tasks_pspace_data_handle.get (pspace);
if (data == NULL)
- data = ada_tasks_pspace_data_handle.emplace (pspace);
+ {
+ if (!create)
+ return nullptr;
+
+ data = ada_tasks_pspace_data_handle.emplace (pspace);
+ }
return data;
}
@@ -316,13 +319,18 @@ get_ada_tasks_pspace_data (struct program_space *pspace)
not use tasking. */
static struct ada_tasks_inferior_data *
-get_ada_tasks_inferior_data (struct inferior *inf)
+get_ada_tasks_inferior_data (struct inferior *inf, bool create = true)
{
struct ada_tasks_inferior_data *data;
data = ada_tasks_inferior_data_handle.get (inf);
if (data == NULL)
- data = ada_tasks_inferior_data_handle.emplace (inf);
+ {
+ if (!create)
+ return nullptr;
+
+ data = ada_tasks_inferior_data_handle.emplace (inf);
+ }
return data;
}
@@ -1447,7 +1455,10 @@ ada_task_list_changed (struct inferior *inf)
static void
ada_tasks_invalidate_pspace_data (struct program_space *pspace)
{
- get_ada_tasks_pspace_data (pspace)->initialized_p = 0;
+ auto data = get_ada_tasks_pspace_data (pspace, false);
+ if (data == nullptr)
+ return;
+ data->initialized_p = 0;
}
/* Invalidate the per-inferior data. */
@@ -1455,8 +1466,10 @@ ada_tasks_invalidate_pspace_data (struct program_space *pspace)
static void
ada_tasks_invalidate_inferior_data (struct inferior *inf)
{
- struct ada_tasks_inferior_data *data = get_ada_tasks_inferior_data (inf);
-
+ struct ada_tasks_inferior_data *data
+ = get_ada_tasks_inferior_data (inf, false);
+ if (data == nullptr)
+ return;
data->known_tasks_kind = ADA_TASKS_UNKNOWN;
data->task_list_valid_p = false;
}
@@ -82,17 +82,22 @@ objfile_pspace_info::~objfile_pspace_info ()
xfree (sections);
}
-/* Get the current svr4 data. If none is found yet, add it now. This
- function always returns a valid object. */
+/* Get the current svr4 data. If none is found yet, add it now, unless
+ !CREATE. */
static struct objfile_pspace_info *
-get_objfile_pspace_data (struct program_space *pspace)
+get_objfile_pspace_data (struct program_space *pspace, bool create = true)
{
struct objfile_pspace_info *info;
info = objfiles_pspace_data.get (pspace);
if (info == NULL)
- info = objfiles_pspace_data.emplace (pspace);
+ {
+ if (!create)
+ return nullptr;
+
+ info = objfiles_pspace_data.emplace (pspace);
+ }
return info;
}
@@ -563,14 +568,18 @@ objfile::~objfile ()
{
symtab_and_line cursal
- = get_current_source_symtab_and_line (this->pspace ());
+ = get_current_source_symtab_and_line (this->pspace (), false);
if (cursal.symtab && cursal.symtab->compunit ()->objfile () == this)
clear_current_source_symtab_and_line (this->pspace ());
}
/* Rebuild section map next time we need it. */
- get_objfile_pspace_data (m_pspace)->section_map_dirty = 1;
+ {
+ auto info = get_objfile_pspace_data (m_pspace, false);
+ if (info != nullptr)
+ info->section_map_dirty = 1;
+ }
}
@@ -426,16 +426,21 @@ free_probes_table (struct svr4_info *info)
info->probes_table.reset (nullptr);
}
-/* Get the svr4 data for program space PSPACE. If none is found yet, add it now.
- This function always returns a valid object. */
+/* Get the svr4 data for program space PSPACE. If none is found yet, add it
+ now, unless !CREATE. */
static struct svr4_info *
-get_svr4_info (program_space *pspace)
+get_svr4_info (program_space *pspace, bool create = true)
{
struct svr4_info *info = solib_svr4_pspace_data.get (pspace);
if (info == NULL)
- info = solib_svr4_pspace_data.emplace (pspace);
+ {
+ if (!create)
+ return nullptr;
+
+ info = solib_svr4_pspace_data.emplace (pspace);
+ }
return info;
}
@@ -1631,7 +1636,9 @@ probes_table_htab_remove_objfile_probes (void **slot, void *info)
static void
probes_table_remove_objfile_probes (struct objfile *objfile)
{
- svr4_info *info = get_svr4_info (objfile->pspace ());
+ svr4_info *info = get_svr4_info (objfile->pspace (), false);
+ if (info == nullptr)
+ return;
if (info->probes_table != nullptr)
htab_traverse_noresize (info->probes_table.get (),
probes_table_htab_remove_objfile_probes, objfile);
@@ -219,29 +219,34 @@ get_lines_to_list (void)
}
/* A helper to return the current source location object for PSPACE,
- creating it if it does not exist. */
+ creating it if it does not exist, unless !CREATE. */
static current_source_location *
-get_source_location (program_space *pspace)
+get_source_location (program_space *pspace, bool create = true)
{
current_source_location *loc
= current_source_key.get (pspace);
if (loc == nullptr)
- loc = current_source_key.emplace (pspace);
+ {
+ if (!create)
+ return nullptr;
+
+ loc = current_source_key.emplace (pspace);
+ }
return loc;
}
/* See source.h. */
symtab_and_line
-get_current_source_symtab_and_line (program_space *pspace)
+get_current_source_symtab_and_line (program_space *pspace, bool create)
{
symtab_and_line cursal;
- current_source_location *loc = get_source_location (pspace);
+ current_source_location *loc = get_source_location (pspace, create);
cursal.pspace = pspace;
- cursal.symtab = loc->symtab ();
- cursal.line = loc->line ();
+ cursal.symtab = loc == nullptr ? nullptr : loc->symtab ();
+ cursal.line = loc == nullptr ? 0 : loc->line ();
cursal.pc = 0;
cursal.end = 0;
@@ -300,7 +305,9 @@ set_current_source_symtab_and_line (const symtab_and_line &sal)
void
clear_current_source_symtab_and_line (program_space *pspace)
{
- current_source_location *loc = get_source_location (pspace);
+ current_source_location *loc = current_source_key.get (pspace);
+ if (loc == nullptr)
+ return;
loc->set (nullptr, 0);
}
@@ -108,9 +108,10 @@ extern int get_first_line_listed (void);
extern int get_lines_to_list (void);
/* Return the current source file for listing and next line to list.
+ Don't create a current source location object, unless CREATE.
NOTE: The returned sal pc and end fields are not valid. */
extern symtab_and_line get_current_source_symtab_and_line
- (program_space *pspace);
+ (program_space *pspace, bool create = true);
/* If the current source file for listing is not set, try and get a default.
Usually called before get_current_source_symtab_and_line() is called.
@@ -102,7 +102,7 @@ static struct block_symbol
const domain_search_flags domain);
static void set_main_name (program_space *pspace, const char *name,
- language lang);
+ language lang, bool create = true);
/* Type of the data stored on the program space. */
@@ -1756,7 +1756,7 @@ symtab_all_objfiles_removed (program_space *pspace)
symbol_cache_flush (pspace);
/* Forget everything we know about the main function. */
- set_main_name (pspace, nullptr, language_unknown);
+ set_main_name (pspace, nullptr, language_unknown, false);
}
/* This module's 'free_objfile' observer. */
@@ -6445,15 +6445,18 @@ make_source_files_completion_list (const char *text, const char *word)
/* Return the "main_info" object for the current program space. If
the object has not yet been created, create it and fill in some
- default values. */
+ default values, unless !CREATE. */
static main_info *
-get_main_info (program_space *pspace)
+get_main_info (program_space *pspace, bool create = true)
{
main_info *info = main_progspace_key.get (pspace);
if (info == NULL)
{
+ if (!create)
+ return nullptr;
+
/* It may seem strange to store the main name in the progspace
and also in whatever objfile happens to see a main name in
its debug info. The reason for this is mainly historical:
@@ -6467,9 +6470,12 @@ get_main_info (program_space *pspace)
}
static void
-set_main_name (program_space *pspace, const char *name, enum language lang)
+set_main_name (program_space *pspace, const char *name, enum language lang,
+ bool create)
{
- main_info *info = get_main_info (pspace);
+ main_info *info = get_main_info (pspace, create);
+ if (info == nullptr)
+ return;
if (!info->name_of_main.empty ())
{