[LLDB][ThreadELFCore] Set all the properties of ELFLinuxSigInfo to a non build dependent size

[Jlalond]

On #110065 the changes to LinuxSigInfo Struct introduced some variables that will differ in size on 32b or 64b. I've rectified this by setting them all to build independent types.

[Jlalond]

@tuliom I don't have a way to verify this, would you mind assisting?

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Jacob Lalonde (Jlalond)

Changes

On #110065 the changes to LinuxSigInfo Struct introduced some variables that will differ in size on 32b or 64b. I've rectified this by setting them all to build independent types.

---

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

1 Files Affected:

• (modified) lldb/source/Plugins/Process/elf-core/ThreadElfCore.h (+5-4)

diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h
index 4ebbaadebe9f90..e60ee86e5f5a03 100644
--- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h
+++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h
@@ -83,10 +83,10 @@ struct ELFLinuxSigInfo {
   int32_t si_errno;
   int32_t si_code;
   // Copied from siginfo_t so we don't have to include signal.h on non 'Nix
-  // builds.
+  // builds. Slight modifications to ensure no 32b vs 64b differences.
   struct {
-    lldb::addr_t si_addr;  /* faulting insn/memory ref. */
-    short int si_addr_lsb; /* Valid LSB of the reported address.  */
+    lldb::addr_t si_addr; /* faulting insn/memory ref. */
+    int16_t si_addr_lsb;  /* Valid LSB of the reported address.  */
     union {
       /* used when si_code=SEGV_BNDERR */
       struct {
@@ -98,7 +98,8 @@ struct ELFLinuxSigInfo {
     } bounds;
   } sigfault;
 
-  enum { eUnspecified, eNT_SIGINFO } note_type;
+  enum SigInfoNoteType : uint8_t { eUnspecified, eNT_SIGINFO };
+  SigInfoNoteType note_type;
 
   ELFLinuxSigInfo();
 

[tuliom]

This didn't help. The size on 32-bits continue to be 44.
I think the difference is not really caused the size of the properties on 32 bits, but the implicit padding on 32/64 bits.

Look at the code generated here: https://godbolt.org/z/j5WxTTess

By commenting the pads or changing their size you can see the impact on the size of the struct and on the address of each of its member.

[Jlalond]

@tuliom Experimented on godbolt a bit, it looks like alignas(8) will handle our padding to 56 on 32 and 64 https://godbolt.org/z/ssqeeEs71

[tuliom]

The build succeeded!

The source comment at line 82 caught my attention, though.
It may imply that data layout is important in this struct, i.e. it should match siginfo_t returned by the OS. But this change is not guaranteeing that.

Actually, the order of the first 3 integers do change on different systems.

With that said, this LGTM considering that data layout doesn't need to match the OS' siginfo_t.

[Jlalond]

The build succeeded!

The source comment at line 82 caught my attention, though. It may imply that data layout is important in this struct, i.e. it should match siginfo_t returned by the OS. But this change is not guaranteeing that.

Actually, the order of the first 3 integers do change on different systems.

With that said, this LGTM considering that data layout doesn't need to match the OS' siginfo_t.

I'll look into fixing the comment, but I'm going to merge to unbreak everyone's builds!