diff mbox series

[2/2,gdb/tui] Only handle code sections in tui_find_backward_disassembly_start_address

Message ID 20230905150339.6452-2-tdevries@suse.de
State Deferred
Headers
Series [1/2,gdb/tui] Fix segfault in tui_find_disassembly_address |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed

Commit Message

Tom de Vries Sept. 5, 2023, 3:03 p.m. UTC 
  After adding a unit test in gdb/tui/tui-disasm.c excercising
tui_find_disassembly_address, I decided to try to extend it using addresses
around section borders.

The new test was very slow (when using gdb as inferior, as is done in
gdb.gdb/unittest.exp), due to disassembling entire non-code sections.

Fix this this by limiting tui_find_backward_disassembly_start_address to
SEC_CODE sections.

FWIW, compared to other self-tests it's still somewhat slow:
...
(gdb) maint selftest
  ...
Command execution time: 1.535391 (cpu), 1.571246 (wall)
(gdb) maint selftest tui-disasm
  ...
Command execution time: 0.482022 (cpu), 0.482028 (wall)
...
This is for calling gdb_print_insn ~550 times.

Tested on x86_64-linux.
---
 gdb/tui/tui-disasm.c | 50 +++++++++++++++++++++++++++++++++++++-------
 1 file changed, 43 insertions(+), 7 deletions(-)

Comments

Kevin Buettner Sept. 27, 2023, 4:15 p.m. UTC | #1 
On Tue,  5 Sep 2023 17:03:39 +0200
Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> wrote:

> Fix this this by limiting tui_find_backward_disassembly_start_address to
> SEC_CODE sections.

What will happen when a user attempts to use the TUI to look at / debug
dynamically generated code?

Kevin
Tom de Vries Sept. 28, 2023, 6:23 p.m. UTC | #2 
On 9/27/23 18:15, Kevin Buettner wrote:
> On Tue,  5 Sep 2023 17:03:39 +0200
> Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> wrote:
> 
>> Fix this this by limiting tui_find_backward_disassembly_start_address to
>> SEC_CODE sections.
> 
> What will happen when a user attempts to use the TUI to look at / debug
> dynamically generated code?
> 

Hi, and thanks for the review(s).

Hm, I don't know, good question.

Maybe it will be possible to detect these cases and expand the search, 
and by default do a more narrow search.

I'm not familiar with the dynamically generated code scenarios and 
support in gdb, so I'm dropping this for now.

Thanks,
- Tom
diff mbox series

Patch

diff --git a/gdb/tui/tui-disasm.c b/gdb/tui/tui-disasm.c
index 03c78aa1291..c31ab5b0680 100644
--- a/gdb/tui/tui-disasm.c
+++ b/gdb/tui/tui-disasm.c
@@ -46,6 +46,8 @@ 
 
 #include "gdb_curses.h"
 
+#include <unordered_set>
+
 struct tui_asm_line
 {
   CORE_ADDR addr;
@@ -164,6 +166,15 @@  tui_disassemble (struct gdbarch *gdbarch,
 static CORE_ADDR
 tui_find_backward_disassembly_start_address (CORE_ADDR addr)
 {
+  struct obj_section *section = find_pc_section (addr);
+  /* Don't handle addresses not in a known section.  */
+  if (section == nullptr)
+    return addr;
+
+  /* Only handle sections with only code.  */
+  if ((section->the_bfd_section->flags & SEC_CODE) == 0)
+    return addr;
+
   struct bound_minimal_symbol msym, msym_prev;
 
   msym = lookup_minimal_symbol_by_pc_section (addr - 1, nullptr,
@@ -174,13 +185,8 @@  tui_find_backward_disassembly_start_address (CORE_ADDR addr)
   else if (msym_prev.minsym != nullptr)
     return msym_prev.value_address ();
 
-  /* Find the section that ADDR is in, and look for the start of the
-     section.  */
-  struct obj_section *section = find_pc_section (addr);
-  if (section != NULL)
-    return section->addr ();
-
-  return addr;
+  /* Use the start of the section.  */
+  return section->addr ();
 }
 
 /* Find the disassembly address that corresponds to FROM lines above
@@ -545,6 +551,36 @@  run_tests ()
 	 being passed a PC for which gdb_print_insn throws a MEMORY_ERROR.  */
       SELF_CHECK (tui_find_disassembly_address (gdbarch, 0, 1) == 0);
       SELF_CHECK (tui_find_disassembly_address (gdbarch, 0, -1) == 0);
+
+      /* Poke around the edges of sections.  */
+      gdbarch_iterate_over_objfiles_in_search_order
+	(target_gdbarch (),
+	 [gdbarch] (objfile *obj)
+	 {
+	   std::unordered_set<CORE_ADDR> visited;
+
+	   /* Already done above.  */
+	   visited.insert (0);
+
+	   for (obj_section *osect : obj->sections ())
+	     {
+	       CORE_ADDR first_addr = osect->addr ();
+	       CORE_ADDR last_addr = osect->endaddr () - 1;
+
+	       for (auto addr_ : { first_addr, last_addr })
+		 for (int offset = -1; offset <= 1; ++offset)
+		   {
+		     CORE_ADDR addr = addr_ + offset;
+		     if (visited.find (addr) != visited.end ())
+		       continue;
+
+		     tui_find_disassembly_address (gdbarch, addr, 1);
+		     tui_find_disassembly_address (gdbarch, addr, -1);
+		     visited.insert (addr);
+		   }
+	     }
+	   return false;
+	 }, nullptr);
     }
 }