[PATCH 1/2] [gdb/tui] Make assert in tui_find_disassembly_address more strict

[PATCH 1/2] [gdb/tui] Make assert in tui_find_disassembly_address more strict

[Tom de Vries]

In tui_find_disassembly_address we find an assert:
...
      if (asm_lines.size () < max_lines)
	{
	  if (!possible_new_low.has_value ())
	    return new_low;

	  /* Take the best possible match we have.  */
	  new_low = *possible_new_low;
	  next_addr = tui_disassemble (gdbarch, asm_lines, new_low, max_lines);
	  last_addr = asm_lines.back ().addr;
	  gdb_assert (asm_lines.size () >= max_lines);
	}
...

The comment right above:
...
      /* If we failed to disassemble the required number of lines then the
	 following walk forward is not going to work, it assumes that
	 ASM_LINES contains exactly MAX_LINES entries.  Instead we should
	 consider falling back to a previous possible start address in
	 POSSIBLE_NEW_LOW.  */
...
claims that the more strict asm_lines.size () == max_line is required.

Update the assert to reflect this, and move it to after the if because it's
supposed to hold in general, not just when entering the if.

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

diff --git a/gdb/tui/tui-disasm.c b/gdb/tui/tui-disasm.c
index 03c78aa1291..e6d4ee6a7ef 100644
--- a/gdb/tui/tui-disasm.c
+++ b/gdb/tui/tui-disasm.c
@@ -268,11 +268,8 @@ tui_find_disassembly_address (struct gdbarch *gdbarch, CORE_ADDR pc, int from)
 	      || (last_addr == pc && asm_lines.size () < max_lines))
 	     && new_low != prev_low);
 
-      /* If we failed to disassemble the required number of lines then the
-	 following walk forward is not going to work, it assumes that
-	 ASM_LINES contains exactly MAX_LINES entries.  Instead we should
-	 consider falling back to a previous possible start address in
-	 POSSIBLE_NEW_LOW.  */
+      /* If we failed to disassemble the required number of lines, try to fall
+	 back to a previous possible start address in POSSIBLE_NEW_LOW.  */
       if (asm_lines.size () < max_lines)
 	{
 	  if (!possible_new_low.has_value ())
@@ -282,9 +279,12 @@ tui_find_disassembly_address (struct gdbarch *gdbarch, CORE_ADDR pc, int from)
 	  new_low = *possible_new_low;
 	  next_addr = tui_disassemble (gdbarch, asm_lines, new_low, max_lines);
 	  last_addr = asm_lines.back ().addr;
-	  gdb_assert (asm_lines.size () >= max_lines);
 	}
 
+      /* The following walk forward assumes that ASM_LINES contains exactly
+	 MAX_LINES entries.  */
+      gdb_assert (asm_lines.size () == max_lines);
+
       /* Scan forward disassembling one instruction at a time until
 	 the last visible instruction of the window matches the pc.
 	 We keep the disassembled instructions in the 'lines' window

base-commit: 17827f6183550bdaaa8c0a1f82482b7bc88bab41
-- 
2.35.3

[PATCH 2/2] [gdb/tui] Fix Wmaybe-uninitialized in tui_find_disassembly_address

[Tom de Vries]

When building gdb with -O2, we run into:
...
gdb/tui/tui-disasm.c: In function ‘CORE_ADDR tui_find_disassembly_address \
  (gdbarch*, CORE_ADDR, int)’:
gdb/tui/tui-disasm.c:293:7: warning: ‘last_addr’ may be used uninitialized \
  in this function [-Wmaybe-uninitialized]
       if (last_addr < pc)
       ^~
...

The warning triggers since commit 72535eb14bd ("[gdb/tui] Fix segfault in
tui_find_disassembly_address").

Fix the warning by ensuring that last_addr is initialized at the point of
use:
...
+      last_addr = asm_lines.back ().addr;
       if (last_addr < pc)
...

Tested on x86_64-linux.
---
 gdb/tui/tui-disasm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/tui/tui-disasm.c b/gdb/tui/tui-disasm.c
index e6d4ee6a7ef..7e953b72ab7 100644
--- a/gdb/tui/tui-disasm.c
+++ b/gdb/tui/tui-disasm.c
@@ -278,7 +278,6 @@ tui_find_disassembly_address (struct gdbarch *gdbarch, CORE_ADDR pc, int from)
 	  /* Take the best possible match we have.  */
 	  new_low = *possible_new_low;
 	  next_addr = tui_disassemble (gdbarch, asm_lines, new_low, max_lines);
-	  last_addr = asm_lines.back ().addr;
 	}
 
       /* The following walk forward assumes that ASM_LINES contains exactly
@@ -290,6 +289,7 @@ tui_find_disassembly_address (struct gdbarch *gdbarch, CORE_ADDR pc, int from)
 	 We keep the disassembled instructions in the 'lines' window
 	 and shift it downward (increasing its addresses).  */
       int pos = max_lines - 1;
+      last_addr = asm_lines.back ().addr;
       if (last_addr < pc)
 	do
 	  {
-- 
2.35.3

Re: [PATCH 1/2] [gdb/tui] Make assert in tui_find_disassembly_address more strict

[Tom de Vries]

On 9/29/23 12:07, Tom de Vries via Gdb-patches wrote:

I somehow lost track of this patch series.

In patchwork, the first was marked as committed, the second as deferred, 
I suppose I made a mistake there.

I've retested and committed both.

Thanks,
- Tom