[Bug tdep/31817] New: [gdb/tdep, arm] thumb ld.so and "set auto-solib-add off" don't work well together

[Bug tdep/31817] New: [gdb/tdep, arm] thumb ld.so and "set auto-solib-add off" don't work well together

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31817

            Bug ID: 31817
           Summary: [gdb/tdep, arm] thumb ld.so and "set auto-solib-add
                    off" don't work well together
           Product: gdb
           Version: HEAD
            Status: NEW
          Severity: normal
          Priority: P2
         Component: tdep
          Assignee: unassigned at sourceware dot org
          Reporter: vries at gcc dot gnu.org
  Target Milestone: ---

Consider a hello world test-case:
...
$ gcc hello.c
...

Which runs fine without gdb:
...
$ ./a.out 
hello
...

And runs fine with gdb:
...
$ gdb -q -batch a.out -ex run
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/arm-linux-gnueabihf/libthread_db.so.1".
hello
[Inferior 1 (process 2227847) exited normally]
...

Now, let's try that with "set auto-solib-add off":
...
$ gdb -q -batch -iex "set auto-solib-add off" a.out -ex "run"

Program received signal SIGSEGV, Segmentation fault.
0xf7fd2646 in ?? () from /lib/ld-linux-armhf.so.3
...

So, what happened here?

We get a bit more info like this:
...
$ gdb -q -batch -iex "set auto-solib-add off" a.out \
  -ex starti \
  -ex "maint info break" \
  -ex continue

Program stopped.
0xf7fe1564 in ?? () from /lib/ld-linux-armhf.so.3
Num     Type           Disp Enb Address    What
-1      shlib events   keep y   0xf7fd2660  inf 1
-1.1                        y   0xf7fd2660  inf 1

Program received signal SIGSEGV, Segmentation fault.
0xf7fd2646 in ?? () from /lib/ld-linux-armhf.so.3
...

Only one internal breakpoint was installed, the shlib events one.

Somehow, even in absence of symbol info, the address of
_dl_debug_state@@GLIBC_PRIVATE is found.

But setting a breakpoint on it requires knowing whether this is an arm or thumb
function.

There's a large comment about this in solib-svr4.c:enable_break:
...
         On ARM we need to know whether the ISA of rtld_db_dlactivity (or       
         however it's spelled in your particular system) is ARM or Thumb.       
         That knowledge is encoded in the address, if it's Thumb the low bit    
         is 1.  However, we've stripped that info above and it's not clear      
         what all the consequences are of passing a non-addr_bits_remove'd      
         address to svr4_create_solib_event_breakpoints.  The call to           
         find_pc_section verifies we know about the address and have some       
         hope of computing the right kind of breakpoint to use (via             
         symbol info).  It does mean that GDB needs to be pointed at a          
         non-stripped version of the dynamic linker in order to obtain          
         information it already knows about.  Sigh.  */

      os = find_pc_section (sym_addr);
      if (os != NULL)
...

That code is not triggered however, because at that point sym_addr is still 0.

Reading through enable_break, it becomes clear that we manage to find the
address of the address of _dl_debug_state@@GLIBC_PRIVATE by peeking into bfd
symbols.  I'm not sure it that's allowed for "set auto-solib-add off", or even
whether the bfd symbols should be there.

Anyway, by replicating the same check:
...
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 1d4a50568d7..bbd07dab7e1 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -2452,12 +2452,18 @@ enable_break (struct svr4_info *info, int from_tty)
        }

       if (sym_addr != 0)
-       /* Convert 'sym_addr' from a function pointer to an address.
-          Because we pass tmp_bfd_target instead of the current
-          target, this will always produce an unrelocated value.  */
-       sym_addr = gdbarch_convert_from_func_ptr_addr
-                    (current_inferior ()->arch (), sym_addr,
-                     tmp_bfd_target.get ());
+       {
+         /* Convert 'sym_addr' from a function pointer to an address.
+            Because we pass tmp_bfd_target instead of the current
+            target, this will always produce an unrelocated value.  */
+         sym_addr = gdbarch_convert_from_func_ptr_addr
+           (current_inferior ()->arch (), sym_addr,
+            tmp_bfd_target.get ());
+
+         struct obj_section *os = find_pc_section (sym_addr);
+         if (os == nullptr)
+           sym_addr = 0;
+       }

       if (sym_addr != 0)
        {
...
we get:
...
$ gdb -q -batch -iex "set auto-solib-add off" a.out -ex run
warning: Unable to find dynamic linker breakpoint function.
GDB will be unable to debug shared library initializers
and track explicitly loaded dynamic code.
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/arm-linux-gnueabihf/libthread_db.so.1".
hello
[Inferior 1 (process 2230337) exited normally]
...

Regardless of this PR, and it being fixed or not, we use "set auto-solib-add
off" in the testsuite to work around certain symbol clash issues, and we should
probably try to solve that in another way, because some targets are just not
designed to work well without symbol info for some critical libs.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug tdep/31817] [gdb/tdep, arm] thumb ld.so and "set auto-solib-add off" don't work well together

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31817

--- Comment #1 from Tom de Vries <vries at gcc dot gnu.org> ---
The relevant test-case for this PR is
gdb.base/solib-probes-nosharedlibrary.exp, which does:
- starti
- nosharedlibrary
- break main
- continue

Indeed the patch proposed in comment 1 fixes the test-case.

But, not correctly.

The check should read:
...
         struct obj_section *os = find_pc_section (load_addr + sym_addr);
         if (os == nullptr)
...

Without this correction, the effect of the patch proposed in comment 1 makes
the "shlib events" breakpoint fall back to _start.

With the correction, the patch no longer fixes the test-case.

The first time the breakpoint is set, the check succeeds, and the addresses is
used, the required symbols is found, and all goes well.

The second time, the address is re-used, but the required symbol is no longer
found, and we run into the same problem.

This does fix the test-case:
...
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index f36ce631a08..018a0996776 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -8813,8 +8813,17 @@ arm_breakpoint_kind_from_pc (struct gdbarch *gdbarch,
CORE_ADDR *pcptr)
   arm_gdbarch_tdep *tdep = gdbarch_tdep<arm_gdbarch_tdep> (gdbarch);
   enum bfd_endian byte_order_for_code = gdbarch_byte_order_for_code (gdbarch);

+  static CORE_ADDR cached_addr = 0;
+  static int cached_res = -1;
+
+  if (cached_res != -1 && cached_addr == *pcptr)
+    return cached_res;
+  
+  int res = -1;
   if (arm_pc_is_thumb (gdbarch, *pcptr))
     {
+      res = ARM_BP_KIND_THUMB;
+
       *pcptr = UNMAKE_THUMB_ADDR (*pcptr);

       /* If we have a separate 32-bit breakpoint instruction for Thumb-2,
@@ -8829,15 +8838,20 @@ arm_breakpoint_kind_from_pc (struct gdbarch *gdbarch,
CORE_ADDR *pcptr)

              inst1 = extract_unsigned_integer (buf, 2, byte_order_for_code);
              if (thumb_insn_size (inst1) == 4)
-               return ARM_BP_KIND_THUMB2;
+               res = ARM_BP_KIND_THUMB2;
            }
        }
-
-      return ARM_BP_KIND_THUMB;
     }
   else
-    return ARM_BP_KIND_ARM;
+    res = ARM_BP_KIND_ARM;
+
+  if (cached_res == -1)
+    {
+      cached_addr = *pcptr;
+      cached_res = res;
+    }

+  return res;
 }

 /* Implement the sw_breakpoint_from_kind gdbarch method.  */
...
but it's a bit hacky.  If there's a relocation, this cache should be flushed.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug tdep/31817] [gdb/tdep, arm] thumb ld.so and "set auto-solib-add off" don't work well together

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31817

--- Comment #2 from Tom de Vries <vries at gcc dot gnu.org> ---
Fix in generic code:
...
index a973518ac5f..0c99ccc9568 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -2792,7 +2792,11 @@ breakpoint_kind (const struct bp_location *bl, CORE_ADDR
*addr)
                                                         regcache, addr);
     }
   else
-    return gdbarch_breakpoint_kind_from_pc (bl->gdbarch, addr);
+    {
+      if (bl->target_info.kind != 0)
+       return bl->target_info.kind;
+      return gdbarch_breakpoint_kind_from_pc (bl->gdbarch, addr);
+    }
 }

 /* Rethrow the currently handled exception, if it's a TARGET_CLOSE_ERROR.
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug tdep/31817] [gdb/tdep, arm] thumb ld.so and "set auto-solib-add off" don't work well together

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31817

--- Comment #3 from Tom de Vries <vries at gcc dot gnu.org> ---
https://sourceware.org/pipermail/gdb-patches/2024-June/209756.html

-- 
You are receiving this mail because:
You are on the CC list for the bug.