Fixing layout of elf-core file related structures
authorPavel Labath <labath@google.com>
Fri, 22 Jul 2016 12:18:45 +0000 (12:18 +0000)
committerPavel Labath <labath@google.com>
Fri, 22 Jul 2016 12:18:45 +0000 (12:18 +0000)
Summary:
The binary layout of prstatus and prpsinfo was wrong.
Some of the member variables where not aligned properly
and others where with a wrong type (e.g. the time related
stuff in prstatus).

I used the structs defined in bfd in binutils to see what the layout
of the elf-core format in these section is.
(https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=bfd/hosts/x86-64linux.h;h=4e420a1f2081dd3b51f5d6b7a8e4093580f5cdb5;hb=master)
Note: those structures are only for x86 64 bit elf-core files

This shouldn't have any impact on the functionality, because
lldb actually uses only a few of the member variables of those structs
and they are with a correct type and alignment.

I found this while trying to add/fix the support for
i386 core files (https://llvm.org/bugs/show_bug.cgi?id=26947)

Reviewers: labath

Subscribers: lldb-commits

Differential Revision: https://reviews.llvm.org/D22628
Author: Dimitar Vlahovski <dvlahovski@google.com>

llvm-svn: 276406

lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
lldb/source/Plugins/Process/elf-core/ThreadElfCore.h

index e4cfa68..1445026 100644 (file)
@@ -228,8 +228,8 @@ ELFLinuxPrStatus::Parse(DataExtractor &data, ArchSpec &arch)
     {
         case ArchSpec::eCore_s390x_generic:
         case ArchSpec::eCore_x86_64_x86_64:
-            len = data.ExtractBytes(0, ELFLINUXPRSTATUS64_SIZE, byteorder, this);
-            return len == ELFLINUXPRSTATUS64_SIZE;
+            len = data.ExtractBytes(0, sizeof(ELFLinuxPrStatus), byteorder, this);
+            return len == sizeof(ELFLinuxPrStatus);
         default:
             return false;
     }
@@ -252,8 +252,8 @@ ELFLinuxPrPsInfo::Parse(DataExtractor &data, ArchSpec &arch)
     {
         case ArchSpec::eCore_s390x_generic:
         case ArchSpec::eCore_x86_64_x86_64:
-            len = data.ExtractBytes(0, ELFLINUXPRPSINFO64_SIZE, byteorder, this);
-            return len == ELFLINUXPRPSINFO64_SIZE;
+            len = data.ExtractBytes(0, sizeof(ELFLinuxPrPsInfo), byteorder, this);
+            return len == sizeof(ELFLinuxPrPsInfo);
         default:
             return false;
     }
index b4e9901..f8694c8 100644 (file)
 
 struct compat_timeval
 {
-    int64_t tv_sec;
-    int32_t tv_usec;
+    alignas(8) uint64_t tv_sec;
+    alignas(8) uint64_t tv_usec;
 };
 
 // PRSTATUS structure's size differs based on architecture.
 // Currently parsing done only for x86-64 architecture by
 // simply reading data from the buffer.
-// The following macros are used to specify the size.
-// Calculating size using sizeof() wont work because of padding.
-#define ELFLINUXPRSTATUS64_SIZE (112)
-#define ELFLINUXPRPSINFO64_SIZE (132)
 
 #undef si_signo
 #undef si_code
@@ -39,24 +35,24 @@ struct compat_timeval
 
 struct ELFLinuxPrStatus
 {
-    int32_t         si_signo;
-    int32_t         si_code;
-    int32_t         si_errno;
+    int32_t si_signo;
+    int32_t si_code;
+    int32_t si_errno;
 
-    int16_t         pr_cursig;
+    int16_t pr_cursig;
 
-    uint64_t        pr_sigpend;
-    uint64_t        pr_sighold;
+    alignas(8) uint64_t pr_sigpend;
+    alignas(8) uint64_t pr_sighold;
 
-    uint32_t        pr_pid;
-    uint32_t        pr_ppid;
-    uint32_t        pr_pgrp;
-    uint32_t        pr_sid;
+    uint32_t pr_pid;
+    uint32_t pr_ppid;
+    uint32_t pr_pgrp;
+    uint32_t pr_sid;
 
-    compat_timeval  pr_utime;
-    compat_timeval  pr_stime;
-    compat_timeval  pr_cutime;
-    compat_timeval  pr_cstime;
+    compat_timeval pr_utime;
+    compat_timeval pr_stime;
+    compat_timeval pr_cutime;
+    compat_timeval pr_cstime;
 
     ELFLinuxPrStatus();
 
@@ -70,28 +66,30 @@ struct ELFLinuxPrStatus
         {
             case lldb_private::ArchSpec::eCore_s390x_generic:
             case lldb_private::ArchSpec::eCore_x86_64_x86_64:
-                return ELFLINUXPRSTATUS64_SIZE;
+                return sizeof(ELFLinuxPrStatus);
             default:
                 return 0;
         }
     }
 };
 
+static_assert(sizeof(ELFLinuxPrStatus) == 112, "sizeof ELFLinuxPrStatus is not correct!");
+
 struct ELFLinuxPrPsInfo
 {
-    char        pr_state;
-    char        pr_sname;
-    char        pr_zomb;
-    char        pr_nice;
-    uint64_t    pr_flag;
-    uint32_t    pr_uid;
-    uint32_t    pr_gid;
-    int32_t     pr_pid;
-    int32_t     pr_ppid;
-    int32_t     pr_pgrp;
-    int32_t     pr_sid;
-    char        pr_fname[16];
-    char        pr_psargs[80];
+    char pr_state;
+    char pr_sname;
+    char pr_zomb;
+    char pr_nice;
+    alignas(8) uint64_t pr_flag;
+    uint32_t pr_uid;
+    uint32_t pr_gid;
+    int32_t pr_pid;
+    int32_t pr_ppid;
+    int32_t pr_pgrp;
+    int32_t pr_sid;
+    char pr_fname[16];
+    char pr_psargs[80];
 
     ELFLinuxPrPsInfo();
 
@@ -105,13 +103,15 @@ struct ELFLinuxPrPsInfo
         {
             case lldb_private::ArchSpec::eCore_s390x_generic:
             case lldb_private::ArchSpec::eCore_x86_64_x86_64:
-                return ELFLINUXPRPSINFO64_SIZE;
+                return sizeof(ELFLinuxPrPsInfo);
             default:
                 return 0;
         }
     }
 };
 
+static_assert(sizeof(ELFLinuxPrPsInfo) == 136, "sizeof ELFLinuxPrPsInfo is not correct!");
+
 struct ThreadData
 {
     lldb_private::DataExtractor gpregset;