[rfa] Detect __aeabi_read_tp even without symbols

[rfa] Detect __aeabi_read_tp even without symbols

[Ulrich Weigand]

Hello,

even with the exception unwinder, there are still a couple of extra failures
on ARM when libc symbol information is missing.

One set of failures is related to the skip_prologue_function routine, which
is used to detect helper routines that are called *during* a function prologue,
so that the prologue parser should not stop when encountering a call to one
of these special routines (as opposed to regular function calls).

However, skip_prologue_function works by identifying the routine by *name*.
If no symbol information is present for libc, this may not work.

One case where this problem happens is in calls to __aeabi_read_tp early
in the prologue of certain glibc routines (like abort).  This causes a
number of test case failures.

However, the __aeabi_read_tp implementation in glibc is actually easy to
recognize even in the absence of a function name: its *contents* are just
two ARM instructions, which are hard-coded as assembler in glibc and seem
unlikely to change (they just forward to the kernel-provided code in the
vector page).

The following patch uses this idea to work around the issue.  This fixes
these failures when running without libc symbol info:
FAIL: gdb.base/corefile.exp: print func2::coremaker_local
FAIL: gdb.base/corefile.exp: backtrace in corefile.exp
FAIL: gdb.base/relativedebug.exp: pause found in backtrace

Tested on armv7l-linux-gnueabi with no regressions.

OK for mainline?

Bye,
Ulrich


ChangeLog:

	* arm-tdep.c (thumb_analyze_prologue): Skip in-prologue calls to glibc
	__aeabi_read_tp implementation even if no symbols are available.


Index: gdb/arm-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.c,v
retrieving revision 1.310
diff -u -p -r1.310 arm-tdep.c
--- gdb/arm-tdep.c	12 Oct 2010 08:46:15 -0000	1.310
+++ gdb/arm-tdep.c	15 Oct 2010 14:11:40 -0000
@@ -822,7 +826,23 @@ thumb_analyze_prologue (struct gdbarch *
 	      if (bit (inst2, 12) == 0)
 		nextpc = nextpc & 0xfffffffc;
 
-	      if (!skip_prologue_function (nextpc))
+	      if (skip_prologue_function (nextpc))
+		;
+	      /* If we run against a stripped glibc, skip_prologue_function
+		 might not have been able to identify the special functions
+		 by name.  Check for one important case, __aeabi_read_tp,
+		 by comparing the *code* against the default implementation
+		 (this is hand-written ARM assembler in glibc, therefore we
+		 need to check for BLX here).  */
+	      else if (bit (inst2, 12) == 0
+		       && read_memory_unsigned_integer (nextpc, 4,
+							byte_order_for_code)
+			   == 0xe3e00a0f /* mov r0, #0xffff0fff */
+		       && read_memory_unsigned_integer (nextpc + 4, 4,
+							byte_order_for_code)
+			   == 0xe240f01f) /* sub pc, r0, #31 */
+		;
+	      else
 		break;
 	    }
 
-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfa] Detect __aeabi_read_tp even without symbols

[Daniel Jacobowitz]

On Wed, Oct 20, 2010 at 02:02:31AM +0200, Ulrich Weigand wrote:
> OK for mainline?

The approach looks good to me.  Two comments on the implementation:
the explanation is just as valid in the ARM case as the Thumb case,
and we should not do this read if we know the name of the target
function and it is different from __aeabi_read_tp.  Both of these can
be accomplished by putting the check in skip_prologue_function
instead.

-- 
Daniel Jacobowitz
CodeSourcery

[rfa v2] Detect __aeabi_read_tp even without symbols

[Ulrich Weigand]

Dan Jacobowitz wrote:
> The approach looks good to me.  Two comments on the implementation:
> the explanation is just as valid in the ARM case as the Thumb case,
> and we should not do this read if we know the name of the target
> function and it is different from __aeabi_read_tp.  Both of these can
> be accomplished by putting the check in skip_prologue_function
> instead.

This version implements the suggestion to move the check into
skip_prologue_function itself (which requires passing some
additional data into that function).

[ As an aside, the new implementation also avoids the empty-
statement construct that Mark disliked ... ]

Re-tested with same results on armv7l-linux-gnueabi.

OK for mainline?

Bye,
Ulrich


ChangeLog:

	* arm-tdep.c (skip_prologue_function): Add GDBARCH and IS_THUMB
	arguments.  Skip in-prologue calls to glibc __aeabi_read_tp
	implementation even if no symbols are available.
	(thumb_analyze_prologue): Update call to skip_prologue_function.
	(arm_analyze_prologue): Likewise.


Index: gdb/arm-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.c,v
retrieving revision 1.313
diff -u -p -r1.313 arm-tdep.c
--- gdb/arm-tdep.c	18 Nov 2010 16:38:20 -0000	1.313
+++ gdb/arm-tdep.c	1 Dec 2010 13:52:02 -0000
@@ -449,39 +449,55 @@ arm_smash_text_address (struct gdbarch *
 }
 
 /* Return 1 if PC is the start of a compiler helper function which
-   can be safely ignored during prologue skipping.  */
+   can be safely ignored during prologue skipping.  IS_THUMB is true
+   if the function is known to be a Thumb function due to the way it
+   is being called.  */
 static int
-skip_prologue_function (CORE_ADDR pc)
+skip_prologue_function (struct gdbarch *gdbarch, CORE_ADDR pc, int is_thumb)
 {
+  enum bfd_endian byte_order_for_code = gdbarch_byte_order_for_code (gdbarch);
   struct minimal_symbol *msym;
-  const char *name;
 
   msym = lookup_minimal_symbol_by_pc (pc);
-  if (msym == NULL || SYMBOL_VALUE_ADDRESS (msym) != pc)
-    return 0;
-
-  name = SYMBOL_LINKAGE_NAME (msym);
-  if (name == NULL)
-    return 0;
-
-  /* The GNU linker's Thumb call stub to foo is named
-     __foo_from_thumb.  */
-  if (strstr (name, "_from_thumb") != NULL)
-    name += 2;
-
-  /* On soft-float targets, __truncdfsf2 is called to convert promoted
-     arguments to their argument types in non-prototyped
-     functions.  */
-  if (strncmp (name, "__truncdfsf2", strlen ("__truncdfsf2")) == 0)
-    return 1;
-  if (strncmp (name, "__aeabi_d2f", strlen ("__aeabi_d2f")) == 0)
-    return 1;
+  if (msym != NULL
+      && SYMBOL_VALUE_ADDRESS (msym) == pc
+      && SYMBOL_LINKAGE_NAME (msym) != NULL)
+    {
+      const char *name = SYMBOL_LINKAGE_NAME (msym);
+
+      /* The GNU linker's Thumb call stub to foo is named
+	 __foo_from_thumb.  */
+      if (strstr (name, "_from_thumb") != NULL)
+	name += 2;
+
+      /* On soft-float targets, __truncdfsf2 is called to convert promoted
+	 arguments to their argument types in non-prototyped
+	 functions.  */
+      if (strncmp (name, "__truncdfsf2", strlen ("__truncdfsf2")) == 0)
+	return 1;
+      if (strncmp (name, "__aeabi_d2f", strlen ("__aeabi_d2f")) == 0)
+	return 1;
 
-  /* Internal functions related to thread-local storage.  */
-  if (strncmp (name, "__tls_get_addr", strlen ("__tls_get_addr")) == 0)
-    return 1;
-  if (strncmp (name, "__aeabi_read_tp", strlen ("__aeabi_read_tp")) == 0)
-    return 1;
+      /* Internal functions related to thread-local storage.  */
+      if (strncmp (name, "__tls_get_addr", strlen ("__tls_get_addr")) == 0)
+	return 1;
+      if (strncmp (name, "__aeabi_read_tp", strlen ("__aeabi_read_tp")) == 0)
+	return 1;
+    }
+  else
+    {
+      /* If we run against a stripped glibc, we may be unable to identify
+	 special functions by name.  Check for one important case,
+	 __aeabi_read_tp, by comparing the *code* against the default
+	 implementation (this is hand-written ARM assembler in glibc).  */
+
+      if (!is_thumb
+	  && read_memory_unsigned_integer (pc, 4, byte_order_for_code)
+	     == 0xe3e00a0f /* mov r0, #0xffff0fff */
+	  && read_memory_unsigned_integer (pc + 4, 4, byte_order_for_code)
+	     == 0xe240f01f) /* sub pc, r0, #31 */
+	return 1;
+    }
 
   return 0;
 }
@@ -825,7 +841,8 @@ thumb_analyze_prologue (struct gdbarch *
 	      if (bit (inst2, 12) == 0)
 		nextpc = nextpc & 0xfffffffc;
 
-	      if (!skip_prologue_function (nextpc))
+	      if (!skip_prologue_function (gdbarch, nextpc,
+					   bit (inst2, 12) != 0))
 		break;
 	    }
 
@@ -1602,7 +1619,7 @@ arm_analyze_prologue (struct gdbarch *gd
 	     the stack.  */
 	  CORE_ADDR dest = BranchDest (current_pc, insn);
 
-	  if (skip_prologue_function (dest))
+	  if (skip_prologue_function (gdbarch, dest, 0))
 	    continue;
 	  else
 	    break;

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfa v2] Detect __aeabi_read_tp even without symbols

[Daniel Jacobowitz]

On Wed, Dec 01, 2010 at 05:05:17PM +0100, Ulrich Weigand wrote:
> Dan Jacobowitz wrote:
> > The approach looks good to me.  Two comments on the implementation:
> > the explanation is just as valid in the ARM case as the Thumb case,
> > and we should not do this read if we know the name of the target
> > function and it is different from __aeabi_read_tp.  Both of these can
> > be accomplished by putting the check in skip_prologue_function
> > instead.
> 
> This version implements the suggestion to move the check into
> skip_prologue_function itself (which requires passing some
> additional data into that function).
> 
> [ As an aside, the new implementation also avoids the empty-
> statement construct that Mark disliked ... ]
> 
> Re-tested with same results on armv7l-linux-gnueabi.
> 
> OK for mainline?

Looks good to me.

-- 
Daniel Jacobowitz
CodeSourcery

[ping] Re: [rfa v2] Detect __aeabi_read_tp even without symbols

[Ulrich Weigand]

Dan Jacobowitz wrote:
> On Wed, Dec 01, 2010 at 05:05:17PM +0100, Ulrich Weigand wrote:
> > Dan Jacobowitz wrote:
> > > The approach looks good to me.  Two comments on the implementation:
> > > the explanation is just as valid in the ARM case as the Thumb case,
> > > and we should not do this read if we know the name of the target
> > > function and it is different from __aeabi_read_tp.  Both of these can
> > > be accomplished by putting the check in skip_prologue_function
> > > instead.
> > 
> > This version implements the suggestion to move the check into
> > skip_prologue_function itself (which requires passing some
> > additional data into that function).
> > 
> > [ As an aside, the new implementation also avoids the empty-
> > statement construct that Mark disliked ... ]
> > 
> > Re-tested with same results on armv7l-linux-gnueabi.
> > 
> > OK for mainline?
> 
> Looks good to me.

Thanks, Dan!

Richard, is the patch OK for mainline now?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfa v2] Detect __aeabi_read_tp even without symbols

[Ulrich Weigand]

> 	* arm-tdep.c (skip_prologue_function): Add GDBARCH and IS_THUMB
> 	arguments.  Skip in-prologue calls to glibc __aeabi_read_tp
> 	implementation even if no symbols are available.
> 	(thumb_analyze_prologue): Update call to skip_prologue_function.
> 	(arm_analyze_prologue): Likewise.

I've checked this in as well.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfa] Detect __aeabi_read_tp even without symbols

[Mark Kettenis]

> Date: Wed, 20 Oct 2010 02:02:31 +0200 (CEST)
> From: "Ulrich Weigand" <uweigand@de.ibm.com>
> 
> Hello,
> 
> even with the exception unwinder, there are still a couple of extra failures
> on ARM when libc symbol information is missing.
> 
> One set of failures is related to the skip_prologue_function routine, which
> is used to detect helper routines that are called *during* a function prologue,
> so that the prologue parser should not stop when encountering a call to one
> of these special routines (as opposed to regular function calls).
> 
> However, skip_prologue_function works by identifying the routine by *name*.
> If no symbol information is present for libc, this may not work.
> 
> One case where this problem happens is in calls to __aeabi_read_tp early
> in the prologue of certain glibc routines (like abort).  This causes a
> number of test case failures.
> 
> However, the __aeabi_read_tp implementation in glibc is actually easy to
> recognize even in the absence of a function name: its *contents* are just
> two ARM instructions, which are hard-coded as assembler in glibc and seem
> unlikely to change (they just forward to the kernel-provided code in the
> vector page).
> 
> The following patch uses this idea to work around the issue.  This fixes
> these failures when running without libc symbol info:
> FAIL: gdb.base/corefile.exp: print func2::coremaker_local
> FAIL: gdb.base/corefile.exp: backtrace in corefile.exp
> FAIL: gdb.base/relativedebug.exp: pause found in backtrace
> 
> Tested on armv7l-linux-gnueabi with no regressions.
> 
> OK for mainline?
> 
> Bye,
> Ulrich
> 
> 
> ChangeLog:
> 
> 	* arm-tdep.c (thumb_analyze_prologue): Skip in-prologue calls to glibc
> 	__aeabi_read_tp implementation even if no symbols are available.
> 
> 
> Index: gdb/arm-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/arm-tdep.c,v
> retrieving revision 1.310
> diff -u -p -r1.310 arm-tdep.c
> --- gdb/arm-tdep.c	12 Oct 2010 08:46:15 -0000	1.310
> +++ gdb/arm-tdep.c	15 Oct 2010 14:11:40 -0000
> @@ -822,7 +826,23 @@ thumb_analyze_prologue (struct gdbarch *
>  	      if (bit (inst2, 12) == 0)
>  		nextpc = nextpc & 0xfffffffc;
>  
> -	      if (!skip_prologue_function (nextpc))
> +	      if (skip_prologue_function (nextpc))
> +		;
> +	      /* If we run against a stripped glibc, skip_prologue_function
> +		 might not have been able to identify the special functions
> +		 by name.  Check for one important case, __aeabi_read_tp,
> +		 by comparing the *code* against the default implementation
> +		 (this is hand-written ARM assembler in glibc, therefore we
> +		 need to check for BLX here).  */
> +	      else if (bit (inst2, 12) == 0
> +		       && read_memory_unsigned_integer (nextpc, 4,
> +							byte_order_for_code)
> +			   == 0xe3e00a0f /* mov r0, #0xffff0fff */
> +		       && read_memory_unsigned_integer (nextpc + 4, 4,
> +							byte_order_for_code)
> +			   == 0xe240f01f) /* sub pc, r0, #31 */
> +		;
> +	      else
>  		break;
>  	    }

I must say that constructs like

  if (foo)
    ;
  else
    ...

confuse the hell out of me.