[v2,ARM] : gdb cannot step across CMSE secure entry function code.

Hello Alan, Simon and Tom.

Thank you very much for reviewing my first patch to GDB and providing me your feedback.
https://sourceware.org/ml/gdb-patches/2019-07/msg00392.html

I have incorporating all your review comments and sending a new patch here.

Hello,

GDB is not able to execute "step" command on function calls of Armv8-M cmse secure entry functions.
Everytime GNU linker come across definition of any cmse secure entry function in object file(s),
it creates two new instructions secure gateway (sg) and original branch destination (b.w),
place those two instructions in ".gnu.sgstubs" section of executable.
Any function calls to these cmse secure entry functions is re-directed through secure gateway (sg)
present in ".gnu.sgstubs" section.

Example:
Following is a function call to cmse secure entry function "foo":
        ...
        bl xxxx <foo>   --->(a)
        ...
        <foo>
        xxxx: push    {r7, lr}

GNU linker on finding out "foo" is a cmse secure entry function, created sg and b.w instructions and
place them in ".gnu.sgstubs" section (marked by c).

The "bl" instruction (marked by a) which is a call to cmse secure entry function is modified by GNU linker
(as marked by b) and call flow is re-directly through secure gateway (sg) in ".gnu.sgstubs" section.
       ...
       bl yyyy <foo>  ---> (b)
       ...
       section .gnu.sgstubs: ---> (c)
       yyyy <foo>
       yyyy: sg   // secure gateway
	     b.w xxxx <__acle_se_foo>  // original_branch_dest
       ...
       0000xxxx <__acle_se_foo>
       xxxx: push    {r7, lr} ---> (d)

On invoking GDB, when the control is at "b" and we pass "step" command, the pc returns "yyyy"
(sg address) which is a trampoline and which does not exist in source code. So GDB jumps
to next line without jumping to "__acle_se_foo" (marked by d).

The above details are published on the Arm website [1], please refer to section 5.4 (Entry functions)
and section 3.4.4 (C level development flow of secure code).

[1] https://developer.arm.com/architectures/cpu-architecture/m-profile/docs/ecm0359818/latest/armv8-m-security-extensions-requirements-on-development-tools-engineering-specification

This patch fixes above problem by returning target pc "xxxx" to GDB on executing "step"
command at "b", so that the control jumps to "__acle_se_foo" (marked by d).

This patch is tested by debugging the CMSE executable using GDB on Aarch32 box.
Regression tested for armv7hl-redhat-linux-gnueabi and found no regressions.

Ok for master? If ok, could someone commit this on my behalf,
I don't have the commit rights.

gdb/ChangeLog:

2019-07-19  Srinath Parvathaneni  <srinath.parvathaneni@arm.com>

	* gdb/arm-tdep.c (arm_skip_cmse_entry):New function. When gdb
	encounters a "step" command on cmse secure entry function (eg:func),
	this function return an address of "__acle_se_<func>" to PC instead
	of secure gateaway (sg) address which is present in ".gnu.sgstubs"
	section.
	(arm_is_sgstubs_section):New function. To check the current section is
	".gnu.sgstubs".
	(arm_skip_stub):Add call to arm_skip_cmse_entry function.
	* gdb/arm-tdep.h (arm_is_sgstubs_section):New function declaration.

gdb/testsuite/ChangeLog:

2019-07-19  Srinath Parvathaneni  <srinath.parvathaneni@arm.com>

	* gdb.arch/arm-cmse-sgstubs.c: New test.
	* gdb.arch/arm-cmse-sgstubs.exp: New file.



###############     Attachment also inlined for ease of reply    ###############

diff --git a/gdb/arm-tdep.h b/gdb/arm-tdep.h
index 23dd40ea8beb1b00289a4cd4e65647399d351580..9482e765a7fc5bb58676096f6b879eae2a7c858e 100644
--- a/gdb/arm-tdep.h
+++ b/gdb/arm-tdep.h
@@ -259,6 +259,7 @@ ULONGEST arm_get_next_pcs_read_memory_unsigned_integer (CORE_ADDR memaddr,
 
 CORE_ADDR arm_get_next_pcs_addr_bits_remove (struct arm_get_next_pcs *self,
 					     CORE_ADDR val);
+bool arm_is_sgstubs_section (struct obj_section *);
 
 int arm_get_next_pcs_is_thumb (struct arm_get_next_pcs *self);
 
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index d244707210628ab045f677c0cbad3d8b0c6d6269..e3b7ab6f096eb91da067d772b8798ffd0737e3d6 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -8211,6 +8211,53 @@ arm_get_longjmp_target (struct frame_info *frame, CORE_ADDR *pc)
   *pc = extract_unsigned_integer (buf, INT_REGISTER_SIZE, byte_order);
   return 1;
 }
+/* A call to cmse secure entry function "foo" at "a" is modified by
+     GNU ld as "b".
+     a) bl xxxx <foo>
+
+     <foo>
+     xxxx:
+
+     b) bl yyyy <__acle_se_foo>
+
+     section .gnu.sgstubs:
+     <foo>
+     yyyy: sg   // secure gateway
+	   b.w xxxx <__acle_se_foo>  // original_branch_dest
+
+     <__acle_se_foo>
+     xxxx:
+
+  When the control at "b", the pc contains "yyyy" (sg address) which is a
+  trampoline and does not exist in source code.  This function returns the
+  target pc "xxxx".  For more details please refer to section 5.4
+  (Entry functions) and section 3.4.4 (C level development flow of secure code)
+  of "armv8-m-security-extensions-requirements-on-development-tools-engineering-specification"
+  document on www.developer.arm.com.  */
+
+static CORE_ADDR
+arm_skip_cmse_entry (CORE_ADDR pc, const char *name, struct objfile *objfile)
+{
+  struct bound_minimal_symbol minsym;
+  int target_len = strlen (name) + strlen ("__acle_se_") + 1;
+  char *target_name  = (char *) alloca (target_len);
+  xsnprintf (target_name, target_len, "%s%s", "__acle_se_",name);
+  minsym = lookup_minimal_symbol (target_name, NULL, objfile);
+  if (minsym.minsym != nullptr)
+    return BMSYMBOL_VALUE_ADDRESS (minsym);
+  return 0;
+}
+
+/* Return true when sec points to ".gnu.sgstubs" section.  */
+bool
+arm_is_sgstubs_section (struct obj_section *sec)
+{
+ if (sec != nullptr && sec->the_bfd_section != nullptr
+     && sec->the_bfd_section->name != nullptr
+     && streq (sec->the_bfd_section->name,".gnu.sgstubs"))
+   return true;
+ return false;
+}
 
 /* Recognize GCC and GNU ld's trampolines.  If we are in a trampoline,
    return the target PC.  Otherwise return 0.  */
@@ -8221,6 +8268,7 @@ arm_skip_stub (struct frame_info *frame, CORE_ADDR pc)
   const char *name;
   int namelen;
   CORE_ADDR start_addr;
+  struct obj_section *section;
 
   /* Find the starting address and name of the function containing the PC.  */
   if (find_pc_partial_function (pc, &name, &start_addr, NULL) == 0)
@@ -8290,6 +8338,10 @@ arm_skip_stub (struct frame_info *frame, CORE_ADDR pc)
 	return 0;
     }
 
+  section = find_pc_section (pc);
+  /* checks whether address pc holds belows to ".gnu.sgstubs" section.  */
+  if (arm_is_sgstubs_section (section))
+    return arm_skip_cmse_entry (pc, name, section->objfile);
   return 0;			/* not a stub */
 }
 
diff --git a/gdb/testsuite/gdb.arch/arm-cmse-sgstubs.c b/gdb/testsuite/gdb.arch/arm-cmse-sgstubs.c
new file mode 100644
index 0000000000000000000000000000000000000000..7f3b40f20c67abfdd2410614e7ee29ae77d37966
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/arm-cmse-sgstubs.c
@@ -0,0 +1,29 @@
+#include <stdio.h>
+extern void func();
+void __acle_se_func ()
+{
+  printf("__acle_se_func\n");
+}
+
+/* The following code is written using asm so that the instructions in function
+ * "func" will be placed in .gnu.sgstubs section of the executable.  */
+asm ("\t.section .gnu.sgstubs,\"ax\",%progbits\n"
+     "\t.global func\n"
+     "\t.type func, %function\n"
+     "func:\n"
+     "\tnop @sg\n"
+     "\tb __acle_se_func @b.w");
+
+void fun1 ()
+{
+  printf("In fun1\n");
+}
+
+int main (void)
+{
+  func();
+  fun1();
+  __acle_se_func();
+  func();
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.arch/arm-cmse-sgstubs.exp b/gdb/testsuite/gdb.arch/arm-cmse-sgstubs.exp
new file mode 100644
index 0000000000000000000000000000000000000000..3416e887d9ebe5ebc52336eff15ba83a6d16df21
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/arm-cmse-sgstubs.exp
@@ -0,0 +1,60 @@
+# Copyright 2019 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite.
+
+if { ![istarget "arm*-*-*"]} {
+     return 1
+}
+
+standard_testfile
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile ]} {
+    return -1
+}
+
+if ![runto_main] {
+   untested "could not run to main"
+   return -1
+}
+
+set test "branch to func from main"
+gdb_test "si" "0x.*" "$test"
+
+set test "next instruction in func"
+gdb_test "ni" "0x.*" "$test"
+
+set test "branch to __acle_se_func from func"
+gdb_test "ni" "__acle_se_func ().*" "${test}"
+
+set test "next in __acle_se_func function"
+gdb_test "next" "5	  .*" "$test"
+
+set test "next in __acle_se_func function outputs __acle_se_func"
+gdb_test "next" "__acle_se_func.*" "$test"
+
+set test "next in __acle_se_func function controls returns to main"
+gdb_test "next" "main ().*" "$test"
+
+set test "next in main outputs In fun1"
+gdb_test "next" "In fun1.*" "$test"
+
+set test "next in main outputs __acle_se_func"
+gdb_test "next" "__acle_se_func.*" "$test"
+
+set test "control jumps to __acle_se_func from main via func"
+gdb_test "step" "__acle_se_func ().*" "${test}"
+
+set test "next in __acle_se_func function via func"
+gdb_test "next" "__acle_se_func.*" "$test"
  

[v2,ARM] : gdb cannot step across CMSE secure entry function code.

Commit Message

Comments

Patch