[LLDB, FreeBSD, x86] Fix empty register set when trying to get size of register

[aokblast]

The register set information is stored as a singleton in GetRegisterInfo_i386. However, other functions later access this information assuming it is stored in GetSharedRegisterInfoVector. To resolve this inconsistency, we remove the original construction logic and instead initialize the singleton using llvm::call_once within the appropriate function (GetSharedRegisterInfoVector_i386).

[aokblast]

I fell asleep so I will write the test case tomorrow. I put it here to first let more people test it and to seek comments on my method.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: None (aokblast)

Changes

GetSharedRegisterInfoVector is a function to get the singleton of total register info. Also, PrivateGetRegisterCount assumes that we have already filled the object in GetSharedRegisterInfoVector and panic when the object is empty.
However, the actually function possess the register info is GetRegisterInfo_i386. Originally, I plan to solve this by only referencing object in GetSharedRegisterInfoVector. However, RegisterInfos_x86_64.h requires the symbol with name g_register_infos in current scope so that the header can append x86_64 registers after it. As a result, I decide to copy the info to the object in GetSharedRegisterInfoVector.

Also, reorder the header order and provide a test for debugging 32bit binary on 64bit platform.

---

Full diff: https://github.com/llvm/llvm-project/pull/162890.diff

2 Files Affected:

• (modified) lldb/source/Host/freebsd/Host.cpp (+1-2)
• (modified) lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_x86_64.cpp (+4-2)

diff --git a/lldb/source/Host/freebsd/Host.cpp b/lldb/source/Host/freebsd/Host.cpp
index fa7efad466bad..dfdbfea0c3c0a 100644
--- a/lldb/source/Host/freebsd/Host.cpp
+++ b/lldb/source/Host/freebsd/Host.cpp
@@ -18,8 +18,6 @@
 #include <dlfcn.h>
 #include <execinfo.h>
 
-#include "llvm/Object/ELF.h"
-
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/HostInfo.h"
@@ -32,6 +30,7 @@
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/StreamString.h"
 
+#include "llvm/Object/ELF.h"
 #include "llvm/TargetParser/Host.h"
 
 namespace lldb_private {
diff --git a/lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_x86_64.cpp b/lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_x86_64.cpp
index e0f3971c6e272..b55a5c595d3ca 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_x86_64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_x86_64.cpp
@@ -76,8 +76,7 @@ static std::vector<lldb_private::RegisterInfo> &GetSharedRegisterInfoVector() {
 
 static const RegisterInfo *
 GetRegisterInfo_i386(const lldb_private::ArchSpec &arch) {
-  static std::vector<lldb_private::RegisterInfo> g_register_infos(
-      GetSharedRegisterInfoVector());
+  static std::vector<lldb_private::RegisterInfo> g_register_infos;
 
   // Allocate RegisterInfo only once
   if (g_register_infos.empty()) {
@@ -93,6 +92,9 @@ GetRegisterInfo_i386(const lldb_private::ArchSpec &arch) {
 #define UPDATE_REGISTER_INFOS_I386_STRUCT_WITH_X86_64_OFFSETS
 #include "RegisterInfos_x86_64.h"
 #undef UPDATE_REGISTER_INFOS_I386_STRUCT_WITH_X86_64_OFFSETS
+    std::vector<lldb_private::RegisterInfo> &shared_regs =
+        GetSharedRegisterInfoVector();
+    shared_regs = g_register_infos;
   }
 
   return &g_register_infos[0];

[aokblast]

@DavidSpickett Hello:
I think the error is because LLVM CI container does not ship 32bit library. Do you have any idea about this?

[DavidSpickett]

This register handling was questionable to begin with but I think there's a way to make it clear.

Think we should deal with the test separately. I'd be ok accepting the register fix if it was obvious just from reading the code.

[DavidSpickett]

This is certainly some improvement. Register information handling is weird at the best of times, and going any further refactoring this would obscure the bug that you're fixing.

I'm ok accepting this without the test for now given that it only affects 32-bit FreeBSD debug, which is already broken anyway.

When we deal with the test case we'll make sure something reads a few registers to verify this.

LGTM.

Please update the PR description to reflect the current changes before landing.

[aokblast]

This is certainly some improvement. Register information handling is weird at the best of times, and going any further refactoring this would obscure the bug that you're fixing.

Emm, I see Linux one also use a more wired method. They take the reference from singloton function and call it g_register_info like std::vector &g_register_info = singloton(). We need to change the RegisterInfo_x86_64.h to fix it.

I'm ok accepting this without the test for now given that it only affects 32-bit FreeBSD debug, which is already broken anyway.

Thanks for your help!

When we deal with the test case we'll make sure something reads a few registers to verify this.

I have added the test case on my local tree. bb00bc9.
I am still figuring out how to get condition like HAVE-32-BIT-LIB

LGTM.

Please update the PR description to reflect the current changes before landing.

Done.