Re: [PATCH] [gdb/testsuite] Fix gdb.base/inline-frame-cycle-unwind.exp for s390x (alternative)

[Andrew Burgess <aburgess@redhat.com>]

Andrew Burgess <aburgess@redhat.com> writes:

> Tom de Vries <tdevries@suse.de> writes:
>
>> With test-case gdb.base/inline-frame-cycle-unwind.exp on s390x-linux, I run
>> into:
>> ...
>>  (gdb) bt^M
>>  #0  inline_func () at inline-frame-cycle-unwind.c:49^M
>>  #1  normal_func () at inline-frame-cycle-unwind.c:32^M
>>  #2  0x000000000100065c in inline_func () at inline-frame-cycle-unwind.c:45^M
>>  #3  normal_func () at inline-frame-cycle-unwind.c:32^M
>>  Backtrace stopped: previous frame identical to this frame (corrupt stack?)^M
>>  (gdb) FAIL: $exp: bt: cycle at level 5: backtrace when the unwind is broken \
>>    at frame 5
>> ...
>>
>> In contrast, on x86_64-linux, I get:
>> ...
>>  (gdb) bt^M
>>  #0  inline_func () at inline-frame-cycle-unwind.c:49^M
>>  #1  normal_func () at inline-frame-cycle-unwind.c:32^M
>>  #2  0x0000000000401157 in inline_func () at inline-frame-cycle-unwind.c:45^M
>>  #3  normal_func () at inline-frame-cycle-unwind.c:32^M
>>  #4  0x0000000000401157 in inline_func () at inline-frame-cycle-unwind.c:45^M
>>  #5  normal_func () at inline-frame-cycle-unwind.c:32^M
>>  Backtrace stopped: previous frame identical to this frame (corrupt stack?)^M
>>  (gdb) PASS: $exp: bt: cycle at level 5: backtrace when the unwind is broken \
>>    at frame 5
>> ...

I took a look at this issue.  I'm running out of time today, so I
thought I'd just dump what I've seen so far.

Here's a table I made for x86-64:

  | Level |    $frame_base |            $sp |    $code | Type   |
  |-------+----------------+----------------+----------+--------+
  |     0 | 0x7fffffffa560 | 0x7fffffffa550 | 0x401128 | inline |
  |     1 | 0x7fffffffa560 | 0x7fffffffa550 | 0x401106 | normal |
  |     2 | 0x7fffffffa570 | 0x7fffffffa560 | 0x401128 | inline |
  |     3 | 0x7fffffffa570 | 0x7fffffffa560 | 0x401106 | normal |
  |     4 | 0x7fffffffa580 | 0x7fffffffa570 | 0x401128 | inline |
  |     5 | 0x7fffffffa580 | 0x7fffffffa570 | 0x401106 | normal |
  |     6 | 0x7fffffffa590 | 0x7fffffffa580 | 0x401189 | normal |
  |-------+----------------+----------------+----------+--------+

The $frame_base and $code are what we see in the frame-id, and $sp is
the stack pointer ($sp) register in each frame.

Notice the relationship between $frame_base and $sp as we move up the
stack; as we move past each inline/normal frame pair the previous
$frame_base becomes the $sp value.  This makes sense to me.

As a result of this the difference between $frame_base values is the
same as the difference between $frame_base and $sp values.

Here's the same table for s390:

  | Level |   $frame_base |           $sp |     $code | Type   |
  |-------+---------------+---------------+-----------+--------+
  |     0 | 0x3ffffff7898 | 0x3ffffff7758 | 0x1000620 | inline |
  |     1 | 0x3ffffff7898 | 0x3ffffff7758 | 0x10005f0 | normal |
  |     2 | 0x3ffffff7938 | 0x3ffffff77f8 | 0x1000620 | inline |
  |     3 | 0x3ffffff7938 | 0x3ffffff77f8 | 0x10005f0 | normal |
  |     4 | 0x3ffffff79d8 | 0x3ffffff7898 | 0x1000620 | inline |
  |     5 | 0x3ffffff79d8 | 0x3ffffff7898 | 0x10005f0 | normal |
  |     6 | 0x3ffffff7a78 | 0x3ffffff7938 | 0x1000698 | normal |
  |-------+---------------+---------------+-----------+--------+

There are a couple of things here that I find are unexpected, but are
probably a result of me not understanding the s390 ABI well enough.

First, and this is not relevant to the test failure, but the inline and
normal (non-inline) frames don't have the same $code address.  I double
checked the disassembly for frame #0, and it does appear like the
function has been inlined, but maybe I'm wrong on this.  This means that
GDB manages to find some address within the function and claims this is
the "start" of the inlined function.  Weird.  But like I said, I don't
think this is really an issue for this test.

Second, the difference between $frame_base values is 0xa0, and the
difference between $sp values is also 0xa0.  This is reassuring, each
recursive frame moves 0xa0 bytes down the stack.

But the difference between $sp and $frame_base values is 0x140 bytes.
This was a little unexpected.

But, I guess, this is fine.  The DWARF spec doesn't (as far as I know)
claim that the frame base has to mean anything specific, it's just an
address that's unique to a frame, and from which we can offset to find
frame local data.

However, the test tries to calculate the frame #5 $frame_base address by
taking the difference between frame #2 and frame #0 $sp value, and
adding this difference to the frame #5 $sp value.  For most
architectures this works fine.  But clearly this isn't going to work for
s390.

I don't have time right now, but tomorrow I'll double check the two
weird things above, I'd like to confirm these really are how the DWARF
defines things.  But assuming it is, then I think your proposed solution
is the right way to go; though I think the commit message needs updating
to reflect the above.

Having spent some time now running and debugging this test, I think one
neat thing about the way it was originally written, is that the Python
script could be used stand-alone, so long as the test program was
stopped in the right place you could source the Python script and it
would do the right thing.

You proposed patch changes this slightly so that the .exp script is
needed to setup some of the frame-id data.  That's absolutely fine, but
can make the test harder to run outside DejaGNU; if the stack addresses
change the user needs to figure out the updated values.  It's a really
minor thing, but how about the patch below (still needs a commit
message), but this does more of the work in the Python script again.

Thanks,
Andrew

--

diff --git i/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.py w/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.py
index 80cfb864f21..974d40d9219 100644
--- i/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.py
+++ w/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.py
@@ -13,6 +13,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+import re
 import gdb
 from gdb.unwinder import Unwinder
 
@@ -21,11 +22,8 @@ from gdb.unwinder import Unwinder
 # was written for.
 stop_at_level = None
 
-# Set this to the stack frame size of frames 1, 3, and 5.  These
-# frames will all have the same stack frame size as they are the same
-# function called recursively.
-stack_adjust = None
-
+# List of FrameId instances, one for each stack frame.
+frame_ids = []
 
 class FrameId(object):
     def __init__(self, sp, pc):
@@ -49,16 +47,13 @@ class TestUnwinder(Unwinder):
         if stop_at_level is None or pending_frame.level() != stop_at_level:
             return None
 
-        if stack_adjust is None:
-            raise gdb.GdbError("invalid stack_adjust")
+        if len(frame_ids) < stop_at_level:
+            raise gdb.GdbError("not enough parsed frame-ids")
 
         if stop_at_level not in [1, 3, 5]:
             raise gdb.GdbError("invalid stop_at_level")
 
-        sp_desc = pending_frame.architecture().registers().find("sp")
-        sp = pending_frame.read_register(sp_desc) + stack_adjust
-        pc = (gdb.lookup_symbol("normal_func"))[0].value().address
-        unwinder = pending_frame.create_unwind_info(FrameId(sp, pc))
+        unwinder = pending_frame.create_unwind_info(frame_ids[stop_at_level])
 
         for reg in pending_frame.architecture().registers("general"):
             val = pending_frame.read_register(reg)
@@ -76,11 +71,22 @@ gdb.unwinder.register_unwinder(None, TestUnwinder(), True)
 #
 #   main -> normal_func -> inline_func -> normal_func -> inline_func -> normal_func -> inline_func
 #
-# Compute the stack frame size of normal_func, which has inline_func
-# inlined within it.
-f0 = gdb.newest_frame()
-f1 = f0.older()
-f2 = f1.older()
-f0_sp = f0.read_register("sp")
-f2_sp = f2.read_register("sp")
-stack_adjust = f2_sp - f0_sp
+# Iterate through frames 0 to 5, parse their frame-id and store it
+# into the global FRAME_IDS list.
+for i in range(6):
+    # Select the specific frame.  Use to_string to suppress output.
+    gdb.execute(f"frame {i}", to_string=True)
+
+    # Get the frame-id in a verbose text form.
+    output = gdb.execute("maint print frame-id", to_string=True)
+
+    # Parse the frame-id in OUTPUT, find the stack and code addresses.
+    match = re.search(r"stack=(0x[0-9a-fA-F]+).*?code=(0x[0-9a-fA-F]+)", output)
+
+    if match:
+        # Create the FrameId object.
+        sp_addr = int(match.group(1), 16)
+        pc_addr = int(match.group(2), 16)
+        frame_ids.append(FrameId(sp_addr, pc_addr))
+    else:
+        raise gdb.GdbError("Could not parse frame-id for frame #%d" % i)