jingham created this revision. jingham added reviewers: JDevlieghere, jasonmolenda. Herald added a subscriber: kristof.beyls. Herald added a project: All. jingham requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits.
The ARM64 ABI does not require restoring the memory return register value (x8) on exit from a function, so lldb can't reliably fetch memory return values. We were still trying and that caused a bunch of tests in TestReturnValue to fail. I changed the ABI code to not try to fetch return values for memory returns. I also changed the tests to assert that for all the cases of memory returns we return no value on Darwin arm64, since we shouldn't be guessing... Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D121348 Files: lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp lldb/test/API/functionalities/return-value/TestReturnValue.py Index: lldb/test/API/functionalities/return-value/TestReturnValue.py =================================================================== --- lldb/test/API/functionalities/return-value/TestReturnValue.py +++ lldb/test/API/functionalities/return-value/TestReturnValue.py @@ -22,10 +22,19 @@ return (self.getArchitecture() in ["aarch64", "arm"] and self.getPlatform() in ["freebsd", "linux"]) - # ABIMacOSX_arm(64) can't fetch simple values inside a structure - def affected_by_radar_34562999(self): + # ABIMacOSX_arm(64) doesn't restore the storage value for memory returns on function + # exit, so lldb shouldn't attempt to fetch memory for those return types, as there is + # no easy way to guarantee that they will be correct. This is calculable, but I don't + # want to have to encode the arm64 ABI here, so I just listed the ones that we shouldn't + # be reporting a value for. + darwin_arm_no_return_values = ["return_five_int", "return_one_int_one_double_one_int", + "return_one_short_one_double_one_short", "return_vector_size_float32_32", + "return_ext_vector_size_float32_8"] + def should_report_return_value(self, func_name): arch = self.getArchitecture().lower() - return arch in ['arm64', 'arm64e', 'armv7', 'armv7k'] and self.platformIsDarwin() + if arch in ['arm64', 'arm64e', 'armv7', 'armv7k'] and self.platformIsDarwin(): + return not func_name in self.darwin_arm_no_return_values + return True @expectedFailureAll(oslist=["freebsd"], archs=["i386"], bugnumber="llvm.org/pr48376") @@ -128,14 +137,13 @@ #self.assertEqual(in_float, return_float) - if not self.affected_by_radar_34562999() and not self.affected_by_pr44132(): + if not self.affected_by_pr44132(): self.return_and_test_struct_value("return_one_int") self.return_and_test_struct_value("return_two_int") self.return_and_test_struct_value("return_three_int") self.return_and_test_struct_value("return_four_int") if not self.affected_by_pr33042(): self.return_and_test_struct_value("return_five_int") - self.return_and_test_struct_value("return_two_double") self.return_and_test_struct_value("return_one_double_two_float") self.return_and_test_struct_value("return_one_int_one_float_one_int") @@ -169,7 +177,6 @@ archs=["i386"]) @expectedFailureAll(compiler=["gcc"], archs=["x86_64", "i386"]) @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24778") - @expectedFailureDarwin(archs=["arm64"]) # <rdar://problem/33976032> ABIMacOSX_arm64 doesn't get structs this big correctly def test_vector_values(self): self.build() exe = self.getBuildArtifact("a.out") @@ -185,7 +192,6 @@ None, None, self.get_process_working_directory()) self.assertEqual(len(lldbutil.get_threads_stopped_at_breakpoint( self.process, main_bktp)), 1) - self.return_and_test_struct_value("return_vector_size_float32_8") self.return_and_test_struct_value("return_vector_size_float32_16") if not self.affected_by_pr44132(): @@ -269,6 +275,9 @@ frame = thread.GetFrameAtIndex(0) ret_value = thread.GetStopReturnValue() + if not self.should_report_return_value(func_name): + self.assertFalse(ret_value.IsValid(), "Shouldn't have gotten a value") + return self.assertTrue(ret_value.IsValid()) Index: lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp =================================================================== --- lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp +++ lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp @@ -590,9 +590,10 @@ } else { const RegisterInfo *reg_info = nullptr; 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->GetRegisterInfoByName("x8", 0); + // The Darwin arm64 ABI doesn't write the return location back to x8 + // before returning from the function the way the x86_64 ABI does. So + // we can't reconstruct stack based returns on exit from the function: + return false; } else { // We are assuming we are stopped at the first instruction in a function // and that the ABI is being respected so all parameters appear where
Index: lldb/test/API/functionalities/return-value/TestReturnValue.py =================================================================== --- lldb/test/API/functionalities/return-value/TestReturnValue.py +++ lldb/test/API/functionalities/return-value/TestReturnValue.py @@ -22,10 +22,19 @@ return (self.getArchitecture() in ["aarch64", "arm"] and self.getPlatform() in ["freebsd", "linux"]) - # ABIMacOSX_arm(64) can't fetch simple values inside a structure - def affected_by_radar_34562999(self): + # ABIMacOSX_arm(64) doesn't restore the storage value for memory returns on function + # exit, so lldb shouldn't attempt to fetch memory for those return types, as there is + # no easy way to guarantee that they will be correct. This is calculable, but I don't + # want to have to encode the arm64 ABI here, so I just listed the ones that we shouldn't + # be reporting a value for. + darwin_arm_no_return_values = ["return_five_int", "return_one_int_one_double_one_int", + "return_one_short_one_double_one_short", "return_vector_size_float32_32", + "return_ext_vector_size_float32_8"] + def should_report_return_value(self, func_name): arch = self.getArchitecture().lower() - return arch in ['arm64', 'arm64e', 'armv7', 'armv7k'] and self.platformIsDarwin() + if arch in ['arm64', 'arm64e', 'armv7', 'armv7k'] and self.platformIsDarwin(): + return not func_name in self.darwin_arm_no_return_values + return True @expectedFailureAll(oslist=["freebsd"], archs=["i386"], bugnumber="llvm.org/pr48376") @@ -128,14 +137,13 @@ #self.assertEqual(in_float, return_float) - if not self.affected_by_radar_34562999() and not self.affected_by_pr44132(): + if not self.affected_by_pr44132(): self.return_and_test_struct_value("return_one_int") self.return_and_test_struct_value("return_two_int") self.return_and_test_struct_value("return_three_int") self.return_and_test_struct_value("return_four_int") if not self.affected_by_pr33042(): self.return_and_test_struct_value("return_five_int") - self.return_and_test_struct_value("return_two_double") self.return_and_test_struct_value("return_one_double_two_float") self.return_and_test_struct_value("return_one_int_one_float_one_int") @@ -169,7 +177,6 @@ archs=["i386"]) @expectedFailureAll(compiler=["gcc"], archs=["x86_64", "i386"]) @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24778") - @expectedFailureDarwin(archs=["arm64"]) # <rdar://problem/33976032> ABIMacOSX_arm64 doesn't get structs this big correctly def test_vector_values(self): self.build() exe = self.getBuildArtifact("a.out") @@ -185,7 +192,6 @@ None, None, self.get_process_working_directory()) self.assertEqual(len(lldbutil.get_threads_stopped_at_breakpoint( self.process, main_bktp)), 1) - self.return_and_test_struct_value("return_vector_size_float32_8") self.return_and_test_struct_value("return_vector_size_float32_16") if not self.affected_by_pr44132(): @@ -269,6 +275,9 @@ frame = thread.GetFrameAtIndex(0) ret_value = thread.GetStopReturnValue() + if not self.should_report_return_value(func_name): + self.assertFalse(ret_value.IsValid(), "Shouldn't have gotten a value") + return self.assertTrue(ret_value.IsValid()) Index: lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp =================================================================== --- lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp +++ lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp @@ -590,9 +590,10 @@ } else { const RegisterInfo *reg_info = nullptr; 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->GetRegisterInfoByName("x8", 0); + // The Darwin arm64 ABI doesn't write the return location back to x8 + // before returning from the function the way the x86_64 ABI does. So + // we can't reconstruct stack based returns on exit from the function: + return false; } else { // We are assuming we are stopped at the first instruction in a function // and that the ABI is being respected so all parameters appear where
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits