From f2c09e5e16d592303b5a1c158cdef28ef08104f0 Mon Sep 17 00:00:00 2001 From: David Spickett Date: Wed, 25 Oct 2023 09:16:58 +0100 Subject: [PATCH] [lldb][AArch64] Invalidate SVG prior to reconfiguring ZA regdef (#66768) This fixes a bug where writing vg during streaming mode could prevent you reading za directly afterwards. vg is invalidated just prior to us reading it in AArch64Reconfigure, but svg was not. This lead to some situations where vg would be updated or cleared and re-read, but svg would not be. This meant it had some undefined value which lead to errors that prevented us reading ZA. Likely we received a lot more data than we were expecting. There are at least 2 ways to get into this situation: * Explicit write by the user to vg. * We have just stopped and need to get the potentially new svg and vg. The first is handled by invalidating svg client side before fetching the new one. This also covers some but not all of the second scenario. For the second, I've made writes to vg invalidate svg by noting this in the register information. Whichever one of those kicks in, we'll get the latest value of svg. The bug may depend on timing, I could not find a consistent way to trigger it. I originally found it when checking whether za is disabled after a vg change, so I've added checks for that to TestZAThreadedDynamic. The SVE VG version of the bug did show up on the buildbot, but not consistently. So it's possible that TestZAThreadedDynamic does in fact cover this, but I haven't run it enough times to know. --- .../Process/Utility/RegisterInfoPOSIX_arm64.cpp | 11 +++++++++++ .../Process/gdb-remote/GDBRemoteRegisterContext.cpp | 5 +++++ .../za_dynamic_resize/TestZAThreadedDynamic.py | 2 ++ 3 files changed, 18 insertions(+) diff --git a/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp b/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp index 60070819cb9269..e6e6c12d0404ae 100644 --- a/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp +++ b/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp @@ -374,6 +374,17 @@ void RegisterInfoPOSIX_arm64::AddRegSetSME() { std::make_pair(sme_regnum, m_dynamic_reg_infos.size()); m_dynamic_reg_sets.push_back(g_reg_set_sme_arm64); m_dynamic_reg_sets.back().registers = m_sme_regnum_collection.data(); + + // When vg is written during streaming mode, svg will also change, as vg and + // svg in this state are both showing the streaming vector length. + // We model this as vg invalidating svg. In non-streaming mode this doesn't + // happen but to keep things simple we will invalidate svg anyway. + // + // This must be added now, rather than when vg is defined because SME is a + // dynamic set that may or may not be present. + static const uint32_t vg_invalidates[] = {sme_regnum + 1 /*svg*/, + LLDB_INVALID_REGNUM}; + m_dynamic_reg_infos[GetRegNumSMESVG()].invalidate_regs = vg_invalidates; } uint32_t RegisterInfoPOSIX_arm64::ConfigureVectorLengthSVE(uint32_t sve_vq) { diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp index b127d3d6213a4a..72280927471f88 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp @@ -783,6 +783,11 @@ void GDBRemoteRegisterContext::AArch64Reconfigure() { std::optional svg_reg_value; const RegisterInfo *svg_reg_info = m_reg_info_sp->GetRegisterInfo("svg"); if (svg_reg_info) { + // When vg is written it is automatically made invalid. Writing vg will also + // change svg if we're in streaming mode but it will not be made invalid + // so do this manually so the following read gets the latest svg value. + SetRegisterIsValid(svg_reg_info, false); + uint32_t svg_reg_num = svg_reg_info->kinds[eRegisterKindLLDB]; uint64_t reg_value = ReadRegisterAsUnsigned(svg_reg_num, fail_value); if (reg_value != fail_value && reg_value <= 32) diff --git a/lldb/test/API/commands/register/register/aarch64_za_register/za_dynamic_resize/TestZAThreadedDynamic.py b/lldb/test/API/commands/register/register/aarch64_za_register/za_dynamic_resize/TestZAThreadedDynamic.py index 65d1071c26b2a3..d2a26ce71bde1d 100644 --- a/lldb/test/API/commands/register/register/aarch64_za_register/za_dynamic_resize/TestZAThreadedDynamic.py +++ b/lldb/test/API/commands/register/register/aarch64_za_register/za_dynamic_resize/TestZAThreadedDynamic.py @@ -125,11 +125,13 @@ def za_test_impl(self, enable_za): self.runCmd("thread select %d" % (idx + 1)) self.check_za_register(4, 2) self.runCmd("register write vg 2") + self.check_disabled_za_register(2) elif stopped_at_line_number == thY_break_line1: self.runCmd("thread select %d" % (idx + 1)) self.check_za_register(2, 3) self.runCmd("register write vg 4") + self.check_disabled_za_register(4) self.runCmd("thread continue 2") self.runCmd("thread continue 3")