[v2,(w/,ChangeLog)] Fix disabling of solib probes when LD_AUDITing

Message ID 1ehh33uyr5gra7_h9krnpznz2jqsjw_4593wafg.zlb1mgays0ef@mail.bob131.so
State New, archived
Headers

Commit Message

George Barrett Dec. 15, 2019, 12:12 a.m. UTC
  The SVR4 solib event handler determines whether an event is related to a
non-base link namespace by comparing the event's debug struct address
to the debug struct address of the initial program image. However, this
can fail when using LD_AUDIT as audit libraries are loaded before the
loader has initialised the initial program image's debug struct. When
the event handler fails to find the debug struct, the probe-based
debugger interface is disabled and a warning is flagged to the user.

This commit adds a fallback test to help determine whether an event is
for a foreign link namespace when the debug struct isn't available.

gdb/ChangeLog:
2019-12-15  George Barrett  <bob@bob131.so>

	* solib-svr4.c (svr4_handle_solib_event): Add fallback link
	namespace test for when the debug struct isn't available.
---
 gdb/solib-svr4.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)
  

Comments

Simon Marchi Dec. 15, 2019, 2:30 a.m. UTC | #1
On 2019-12-14 7:12 p.m., George Barrett wrote:
> The SVR4 solib event handler determines whether an event is related to a
> non-base link namespace by comparing the event's debug struct address
> to the debug struct address of the initial program image. However, this
> can fail when using LD_AUDIT as audit libraries are loaded before the
> loader has initialised the initial program image's debug struct. When
> the event handler fails to find the debug struct, the probe-based
> debugger interface is disabled and a warning is flagged to the user.
> 
> This commit adds a fallback test to help determine whether an event is
> for a foreign link namespace when the debug struct isn't available.

Hi George,

The patch makes sense to me, as far as I understand it.

> gdb/ChangeLog:
> 2019-12-15  George Barrett  <bob@bob131.so>
> 
> 	* solib-svr4.c (svr4_handle_solib_event): Add fallback link
> 	namespace test for when the debug struct isn't available.
> ---
>  gdb/solib-svr4.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
> index de765576d0..f0c7769ac2 100644
> --- a/gdb/solib-svr4.c
> +++ b/gdb/solib-svr4.c
> @@ -1942,7 +1942,27 @@ svr4_handle_solib_event (void)
>      /* Always locate the debug struct, in case it moved.  */
>      info->debug_base = 0;
>      if (locate_base (info) == 0)
> -      return;
> +      {
> +	/* It's possible for the reloc_complete probe to be triggered before
> +	   the linker has set the DT_DEBUG pointer (for example, when the
> +	   linker has finished relocating an LD_AUDIT library or its
> +	   dependencies).  Since we can't yet handle libraries from other link
> +	   namespaces, we don't lose anything by ignoring them here.  */
> +	struct value *link_map_id_val;
> +	try
> +	  {
> +	    link_map_id_val = pa->prob->evaluate_argument (0, frame);
> +	  }
> +	catch (const gdb_exception_error)

Catch the exception by reference:

  catch (const gdb_exception_error &)

I can push your patch with this fixed, if you agree (and others think the patch is
fine too).

Simon
  
George Barrett Dec. 15, 2019, 2:33 a.m. UTC | #2
On Sat, Dec 14, 2019 at 09:30:43PM -0500, Simon Marchi wrote:
> Catch the exception by reference:
>
>   catch (const gdb_exception_error &)
>
> I can push your patch with this fixed, if you agree (and others think the
> patch is fine too).

Yes, that's fine by me.

Thanks
  
Simon Marchi Dec. 15, 2019, 2:34 a.m. UTC | #3
On 2019-12-14 9:33 p.m., George Barrett wrote:
> On Sat, Dec 14, 2019 at 09:30:43PM -0500, Simon Marchi wrote:
>> Catch the exception by reference:
>>
>>   catch (const gdb_exception_error &)
>>
>> I can push your patch with this fixed, if you agree (and others think the
>> patch is fine too).
> 
> Yes, that's fine by me.
> 
> Thanks
> 

Ok, I'd like to leave a bit of time for others to comment, so please
ping in a week or so if there hasn't been any other activity, I'll merge
it then.

Simon
  
George Barrett Dec. 21, 2019, 3:25 p.m. UTC | #4
On Sat, Dec 14, 2019 at 09:34:30PM -0500, Simon Marchi wrote:
> Ok, I'd like to leave a bit of time for others to comment, so please
> ping in a week or so if there hasn't been any other activity, I'll merge
> it then.

Pinging, as requested.
  
Simon Marchi Dec. 21, 2019, 9:03 p.m. UTC | #5
On 2019-12-21 10:25 a.m., George Barrett wrote:
> On Sat, Dec 14, 2019 at 09:34:30PM -0500, Simon Marchi wrote:
>> Ok, I'd like to leave a bit of time for others to comment, so please
>> ping in a week or so if there hasn't been any other activity, I'll merge
>> it then.
> 
> Pinging, as requested.
> 

Thanks, I have pushed it.

Simon
  

Patch

diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index de765576d0..f0c7769ac2 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -1942,7 +1942,27 @@  svr4_handle_solib_event (void)
     /* Always locate the debug struct, in case it moved.  */
     info->debug_base = 0;
     if (locate_base (info) == 0)
-      return;
+      {
+	/* It's possible for the reloc_complete probe to be triggered before
+	   the linker has set the DT_DEBUG pointer (for example, when the
+	   linker has finished relocating an LD_AUDIT library or its
+	   dependencies).  Since we can't yet handle libraries from other link
+	   namespaces, we don't lose anything by ignoring them here.  */
+	struct value *link_map_id_val;
+	try
+	  {
+	    link_map_id_val = pa->prob->evaluate_argument (0, frame);
+	  }
+	catch (const gdb_exception_error)
+	  {
+	    link_map_id_val = NULL;
+	  }
+	/* glibc and illumos' libc both define LM_ID_BASE as zero.  */
+	if (link_map_id_val != NULL && value_as_long (link_map_id_val) != 0)
+	  action = DO_NOTHING;
+	else
+	  return;
+      }
 
     /* GDB does not currently support libraries loaded via dlmopen
        into namespaces other than the initial one.  We must ignore