[binutils-gdb] libctf: endianness fixes

[binutils-gdb] libctf: endianness fixes

[Jose E.Marchesi]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=7cee18263c234073bfe88cbc962b1fc68509df82

commit 7cee18263c234073bfe88cbc962b1fc68509df82
Author: Nick Alcock <nick.alcock@oracle.com>
Date:   Wed Jun 19 12:34:56 2019 +0100

    libctf: endianness fixes
    
    Testing of the first code to generate CTF_K_SLICEs on big-endian
    revealed a bunch of new problems in this area.  Most importantly, the
    trick we did earlier to avoid wasting two bytes on padding in the
    ctf_slice_t is best avoided: because it leads to the whole file after
    that point no longer being naturally aligned, all multibyte accesses
    from then on must use memmove() to avoid unaligned access on platforms
    where that is fatal.  In future, this is planned, but for now we are
    still doing direct access in many places, so we must revert to making
    ctf_slice_t properly aligned for storage in an array.
    
    Rather than wasting bytes on padding, we boost the size of cts_offset
    and cts_bits.  This is still a waste of space (we cannot have offsets or
    bits in bitfields > 256) but it cannot be avoided for now, and slices
    are not so common that this will be a serious problem.
    
    A possibly-worse endianness problem fixed at the same time involves
    a codepath used only for foreign-endian, uncompressed CTF files, where
    we were not copying the actual CTF data into the buffer, leading to
    libctf reading only zeroes (or, possibly, uninitialized garbage).
    
    Finally, when we read in a CTF file, we copy the header and work from
    the copy.  We were flipping the endianness of the header copy, and of
    the body of the file buffer, but not of the header in the file buffer
    itself: so if we write the file back out again we end up with an
    unreadable frankenfile with header and body of different endiannesses.
    Fix by flipping both copies of the header.
    
    include/
    	* ctf.h (ctf_slice_t): Make cts_offset and cts_bits unsigned
    	short, so following structures are properly aligned.
    
    libctf/
    	* ctf-open.c (get_vbytes_common): Return the new slice size.
    	(ctf_bufopen): Flip the endianness of the CTF-section header copy.
    	Remember to copy in the CTF data when opening an uncompressed
    	foreign-endian CTF file.  Prune useless variable manipulation.

Diff:
---
 include/ChangeLog |  5 +++++
 include/ctf.h     | 10 +++++++---
 libctf/ChangeLog  |  7 +++++++
 libctf/ctf-open.c | 18 +++++++++++-------
 4 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/include/ChangeLog b/include/ChangeLog
index 8169b6a..81b6670 100644
--- a/include/ChangeLog
+++ b/include/ChangeLog
@@ -1,3 +1,8 @@
+2019-06-19  Nick Alcock <nick.alcock@oracle.com>
+
+	* ctf.h (ctf_slice_t): Make cts_offset and cts_bits unsigned
+	short, so following structures are properly aligned.
+
 2019-06-14  Szabolcs Nagy  <szabolcs.nagy@arm.com>
 
 	* elf/aarch64.h (R_AARCH64_P32_MOVW_PREL_G0): Define.
diff --git a/include/ctf.h b/include/ctf.h
index e99a673..2b35781 100644
--- a/include/ctf.h
+++ b/include/ctf.h
@@ -430,13 +430,17 @@ union
    ctt_type, which must be a type which has an encoding (fp, int, or enum).  We
    also store the referenced type in here, because it is easier to keep the
    ctt_size correct for the slice than to shuffle the size into here and keep
-   the ctt_type where it is for other types.  */
+   the ctt_type where it is for other types.
+
+   In a future version, where we loosen requirements on alignment in the CTF
+   file, the cts_offset and cts_bits will be chars: but for now they must be
+   shorts or everything after a slice will become unaligned.  */
 
 typedef struct ctf_slice
 {
   uint32_t cts_type;
-  unsigned char cts_offset;
-  unsigned char cts_bits;
+  unsigned short cts_offset;
+  unsigned short cts_bits;
 } ctf_slice_t;
 
 typedef struct ctf_array_v1
diff --git a/libctf/ChangeLog b/libctf/ChangeLog
index ad5685b..0d55ea0 100644
--- a/libctf/ChangeLog
+++ b/libctf/ChangeLog
@@ -1,5 +1,12 @@
 2019-06-19  Nick Alcock <nick.alcock@oracle.com>
 
+	* ctf-open.c (get_vbytes_common): Return the new slice size.
+	(ctf_bufopen): Flip the endianness of the CTF-section header copy.
+	Remember to copy in the CTF data when opening an uncompressed
+	foreign-endian CTF file.  Prune useless variable manipulation.
+
+2019-06-19  Nick Alcock <nick.alcock@oracle.com>
+
 	* ctf-open.c (ctf_types): Fail when unidentified type kinds are
 	seen.
 
diff --git a/libctf/ctf-open.c b/libctf/ctf-open.c
index 14f7062..777a6b5 100644
--- a/libctf/ctf-open.c
+++ b/libctf/ctf-open.c
@@ -150,8 +150,7 @@ get_vbytes_common (unsigned short kind, ssize_t size _libctf_unused_,
     case CTF_K_FLOAT:
       return (sizeof (uint32_t));
     case CTF_K_SLICE:
-      return (offsetof (ctf_slice_t, cts_bits) +
-	      sizeof (((ctf_slice_t *)0)->cts_bits));
+      return (sizeof (ctf_slice_t));
     case CTF_K_ENUM:
       return (sizeof (ctf_enum_t) * vlen);
     case CTF_K_FORWARD:
@@ -1208,7 +1207,7 @@ ctf_bufopen (const ctf_sect_t *ctfsect, const ctf_sect_t *symsect,
   const ctf_preamble_t *pp;
   ctf_header_t hp;
   ctf_file_t *fp;
-  void *buf, *base;
+  void *base;
   size_t size, hdrsz;
   int foreign_endian = 0;
   int err;
@@ -1317,6 +1316,7 @@ ctf_bufopen (const ctf_sect_t *ctfsect, const ctf_sect_t *symsect,
       uLongf dstlen;
       const void *src;
       int rc = Z_OK;
+      void *buf;
 
       if ((base = ctf_alloc (size + hdrsz)) == NULL)
 	return (ctf_set_open_errno (errp, ECTF_ZALLOC));
@@ -1349,12 +1349,16 @@ ctf_bufopen (const ctf_sect_t *ctfsect, const ctf_sect_t *symsect,
     {
       if ((base = ctf_alloc (size + hdrsz)) == NULL)
 	return (ctf_set_open_errno (errp, ECTF_ZALLOC));
+      memcpy (base, ctfsect->cts_data, size + hdrsz);
     }
   else
-    {
-      base = (void *) ctfsect->cts_data;
-      buf = (unsigned char *) base + hdrsz;
-    }
+    base = (void *) ctfsect->cts_data;
+
+  /* Flip the endianness of the copy of the header in the section, to avoid
+     ending up with a partially-endian-flipped file.  */
+
+  if (foreign_endian)
+    flip_header ((ctf_header_t *) base);
 
   /* Once we have uncompressed and validated the CTF data buffer, we can
      proceed with allocating a ctf_file_t and initializing it.