From: Andrew Burgess <aburgess@redhat.com>
Date: Mon, 11 Nov 2024 21:45:17 +0000 (+0000)
Subject: gdb: do better in breakpoint_free_objfile
X-Git-Tag: gdb-16-branchpoint~326
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=5066f3680667ec0f2d1745847a2372d85973a1e7;p=thirdparty%2Fbinutils-gdb.git

gdb: do better in breakpoint_free_objfile

The breakpoint_free_objfile function is called from the objfile
destructor, and has the job of removing references to the soon to be
deleted objfile from all breakpoint locations.

The current implementation of breakpoint_free_objfile seems to miss
lots of possible objfile references within bp_location.  Currently we
only check if bp_location::symtab is associated with the objfile in
question, but there's bp_location::section and bp_location::probe,
both of which might reference the soon to be deleted objfile.

Additionally bp_location::symbol and bp_location::msymbol if set will
surely be related to the objfile and should also be cleaned up.

I'm not aware that this causes any problems, but it doesn't seem like
a good idea to retain pointers to deleted state, so I propose that we
improve breakpoint_free_objfile to set these pointers back to nullptr.

In the future I plan to investigate the possibility of merging the
functionality of breakpoint_free_objfile into
disable_breakpoints_in_freed_objfile which is called via the
gdb::observers::free_objfile event.  However, I already have a patch series
in progress which touches this area of GDB, and I'd like to avoid
conflicting with that earlier series:

  https://inbox.sourceware.org/gdb-patches/cover.1724948606.git.aburgess@redhat.com

Once this patch, and that earlier series have landed then I'll see if
I can merge breakpoint_free_objfile, but I don't think that this needs
to block this patch.

There should be no user visible changes after this commit.
---

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index c849a998e79..0ad169a98b0 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -14688,8 +14688,30 @@ void
 breakpoint_free_objfile (struct objfile *objfile)
 {
   for (bp_location *loc : all_bp_locations ())
-    if (loc->symtab != NULL && loc->symtab->compunit ()->objfile () == objfile)
-      loc->symtab = NULL;
+    {
+      if (loc->symtab != nullptr
+	  && loc->symtab->compunit ()->objfile () == objfile)
+	{
+	  loc->symtab = nullptr;
+	  loc->symbol = nullptr;
+	  loc->msymbol = nullptr;
+	}
+
+      if (loc->section != nullptr
+	  && loc->section->objfile == objfile)
+	{
+	  /* If symtab was set then it should have already been cleared.
+	     But if bp_location::msymbol was set then the symbol and symtab
+	     might already have been nullptr.  */
+	  gdb_assert (loc->symtab == nullptr);
+	  loc->section = nullptr;
+	  loc->symbol = nullptr;
+	  loc->msymbol = nullptr;
+	}
+
+      if (loc->probe.objfile == objfile)
+	loc->probe = bound_probe ();
+    }
 }
 
 /* Chain containing all defined "enable breakpoint" subcommands.  */