return-object-by-reference ("non trivial") xfail on arm64 in TestTrivialABI.py

jasonmolenda · jasonmolenda · commit 6d64162a2d0d · 2019-12-11T12:00:16.000-08:00
I don't think this test case can be handled correctly on AAPCS64.
The ABI says that the caller passes the address of the return object
in x8.  x8 is a caller-spilled (aka "volatile") register, and the
function is not required to preserve x8 or to copy the address back
into x8 on function exit like the SysV x86_64 ABI does with rax.
(from aapcs64: "there is no requirement for the callee to preserve the
value stored in x8")

From my quick reading of ABISysV_arm64, I worry that it may actually be
using the value in x8 at function exit, assuming it still has the
address of the return object -

    if (is_return_value) {
      // We are assuming we are decoding this immediately after returning from
      // a function call and that the address of the structure is in x8
      reg_info = reg_ctx-&gt;GetRegisterInfoByName("x8", 0);

This will work on trivial test programs / examples, but if the function
does another function call, or overwrites x8 as a scratch register, lldb
will provide incorrect values to the user.

ABIMacOSX_arm64 doesn't do this, but it also doesn't flag the value
as unavailable so we're providing incorrect values to the user all
the time.  I expect my fix will be to make ABIMacOSX_arm64 flag
the return value as unretrievable, unless I've misread the ABI.
diff --git a/lldb/packages/Python/lldbsuite/test/lang/cpp/trivial_abi/TestTrivialABI.py b/lldb/packages/Python/lldbsuite/test/lang/cpp/trivial_abi/TestTrivialABI.py
@@ -31,6 +31,7 @@ def test_call_trivial(self):
     @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr36870")
     @expectedFailureAll(archs=["aarch64"], oslist=["linux"],
                         bugnumber="llvm.org/pr44161")
+    @expectedFailureAll(archs=["arm64", "arm64e"], bugnumber="<rdar://problem/57844240>")
     def test_call_nontrivial(self):
         """Test that we can print a variable & call a function on the same class w/o the trivial ABI marker."""
         self.build()
diff --git a/lldb/packages/Python/lldbsuite/test/lang/cpp/trivial_abi/main.cpp b/lldb/packages/Python/lldbsuite/test/lang/cpp/trivial_abi/main.cpp
@@ -29,7 +29,7 @@ main()
   outVal = takeTrivial(inVal);
 
   S_NotTrivial inNotVal, outNotVal;
-  outNotVal = takeNotTrivial(outNotVal);
+  outNotVal = takeNotTrivial(inNotVal);
 
   return 0; // Set another for return value
 }
diff --git a/lldb/source/Plugins/ABI/MacOSX-arm64/ABIMacOSX_arm64.cpp b/lldb/source/Plugins/ABI/MacOSX-arm64/ABIMacOSX_arm64.cpp
@@ -2020,6 +2020,8 @@ bool ABIMacOSX_arm64::CreateDefaultUnwindPlan(UnwindPlan &unwind_plan) {
 // registers x19 through x28 and sp are callee preserved. v8-v15 are non-
 // volatile (and specifically only the lower 8 bytes of these regs), the rest
 // of the fp/SIMD registers are volatile.
+//
+// v. https://github.com/ARM-software/software-standards/blob/master/abi/aapcs64/
 
 // We treat x29 as callee preserved also, else the unwinder won't try to
 // retrieve fp saves.

Original file line number	Diff line number	Diff line change
`@@ -29,7 +29,7 @@ main()`
`29`	`29`	`outVal = takeTrivial(inVal);`
`30`	`30`
`31`	`31`	`S_NotTrivial inNotVal, outNotVal;`
`32`		`- outNotVal = takeNotTrivial(outNotVal);`
	`32`	`+ outNotVal = takeNotTrivial(inNotVal);`
`33`	`33`
`34`	`34`	`return 0; // Set another for return value`
`35`	`35`	`}`