Bug 501194 - Fix ML_(check_macho_and_get_rw_loads) so that it is correct for any...

diff --git a/NEWS b/NEWS
index 304ce1c0b79c17345e87abe05cf8359ef0994bcf..0fcbc5d0e2ea40fadba3ca4941758abe9ec762fe 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -56,6 +56,7 @@ are not entered into bugzilla tend to get forgotten about or ignored.
 498492  none/tests/amd64/lzcnt64 crashes on FreeBSD compiled with clang
 499183  FreeBSD: differences in avx-vmovq output
 499212  mmap() with MAP_ALIGNED() returns unaligned pointer
+501194  Fix ML_(check_macho_and_get_rw_loads) so that it is correct for any number of segment commands
 
 
 To see details of a given bug, visit

diff --git a/coregrind/m_debuginfo/debuginfo.c b/coregrind/m_debuginfo/debuginfo.c
index 97d0f35c059bd118d5869bd31583ccf3bd3b7995..612833a997935af39f5ee7cab809f9df6e73d096 100644 (file)
--- a/coregrind/m_debuginfo/debuginfo.c
+++ b/coregrind/m_debuginfo/debuginfo.c
@@ -1135,16 +1135,6 @@ ULong VG_(di_notify_mmap)( Addr a, Bool allow_SkFileV, Int use_fd )
 
    DebugInfo* di;
    Int        actual_fd, oflags;
-#if defined(VGO_darwin)
-   SysRes     preadres;
-   // @todo PJF make this dynamic
-   // that probably means reading the sizeofcmds from the mach_header then
-   // allocating enough space for it
-   // and then one day maybe doing something for fat binaries
-   HChar      buf4k[4096];
-#else
-   Bool       elf_ok;
-#endif
 #if defined(VGO_freebsd)
    static Bool first_fixed_file = True;
 #endif
@@ -1321,11 +1311,6 @@ ULong VG_(di_notify_mmap)( Addr a, Bool allow_SkFileV, Int use_fd )
    }
 #endif
 
-#if defined(VGO_darwin)
-   /* Peer at the first few bytes of the file, to see if it is an ELF */
-   /* object file. Ignore the file if we do not have read permission. */
-   VG_(memset)(buf4k, 0, sizeof(buf4k));
-#endif
 
    oflags = VKI_O_RDONLY;
 #  if defined(VKI_O_LARGEFILE)
@@ -1350,35 +1335,17 @@ ULong VG_(di_notify_mmap)( Addr a, Bool allow_SkFileV, Int use_fd )
       actual_fd = use_fd;
    }
 
-#if defined(VGO_darwin)
-   preadres = VG_(pread)( actual_fd, buf4k, sizeof(buf4k), 0 );
-   if (use_fd == -1) {
-      VG_(close)( actual_fd );
-   }
-
-   if (sr_isError(preadres)) {
-      DebugInfo fake_di;
-      VG_(memset)(&fake_di, 0, sizeof(fake_di));
-      fake_di.fsm.filename = ML_(dinfo_strdup)("di.debuginfo.nmm", filename);
-      ML_(symerr)(&fake_di, True, "can't read file to inspect Mach-O headers");
-      return 0;
-   }
-   if (sr_Res(preadres) == 0)
-      return 0;
-   vg_assert(sr_Res(preadres) > 0 && sr_Res(preadres) <= sizeof(buf4k) );
-
    expected_rw_load_count = 0;
 
-   if (!ML_(check_macho_and_get_rw_loads)( buf4k, (SizeT)sr_Res(preadres), &expected_rw_load_count ))
+#if defined(VGO_darwin)
+   if (!ML_(check_macho_and_get_rw_loads)( actual_fd, &expected_rw_load_count ))
       return 0;
 #endif
 
    /* We're only interested in mappings of object files. */
 #  if defined(VGO_linux) || defined(VGO_solaris) || defined(VGO_freebsd)
 
-   expected_rw_load_count = 0;
-
-   elf_ok = ML_(check_elf_and_get_rw_loads) ( actual_fd, filename, &expected_rw_load_count, use_fd == -1 );
+   Bool elf_ok = ML_(check_elf_and_get_rw_loads) ( actual_fd, filename, &expected_rw_load_count, use_fd == -1 );
 
    if (use_fd == -1) {
       VG_(close)( actual_fd );

diff --git a/coregrind/m_debuginfo/priv_readmacho.h b/coregrind/m_debuginfo/priv_readmacho.h
index b5172878ce4445bbb29f8a436d05ed101bd8d137..0cbdad90f219b0722ea485c54a467ea42fe74121 100644 (file)
--- a/coregrind/m_debuginfo/priv_readmacho.h
+++ b/coregrind/m_debuginfo/priv_readmacho.h
@@ -35,7 +35,7 @@
 
 /* Identify a Mach-O object file by peering at the first few bytes of
    it. Also count the number of RW segements. */
-extern Bool ML_(check_macho_and_get_rw_loads)( const void* buf, SizeT size, Int* rw_loads );
+extern Bool ML_(check_macho_and_get_rw_loads)( Int fd, Int* rw_loads );
 
 /* The central function for reading Mach-O debug info.  For the
    object/exe specified by the DebugInfo, find Mach-O sections, then read

diff --git a/coregrind/m_debuginfo/readmacho.c b/coregrind/m_debuginfo/readmacho.c
index e93bae79416b2325bac4015de8cb0d1c2d02500a..691ec94353722e13aea15c3d10e21aa70fd12603 100644 (file)
--- a/coregrind/m_debuginfo/readmacho.c
+++ b/coregrind/m_debuginfo/readmacho.c
@@ -93,7 +93,7 @@
        memory that falls entirely inside the primary image.
 */
 
-Bool ML_(check_macho_and_get_rw_loads)( const void* buf, SizeT szB, Int* rw_loads )
+Bool ML_(check_macho_and_get_rw_loads)( Int fd, Int* rw_loads )
 {
    /* (JRS: the Mach-O headers might not be in this mapped data,
       because we only mapped a page for this initial check,
@@ -108,27 +108,35 @@ Bool ML_(check_macho_and_get_rw_loads)( const void* buf, SizeT szB, Int* rw_load
       can to establish whether or not we're looking at something
       sane. */
 
-   /* @todo PJF change function signature to pass in file handle
-        Read MACH_HEADER to determine sizeofcommands
-       Allocate a dynamic buffer for the commands. */
+   HChar macho_header[sizeof(struct MACH_HEADER)];
+   SysRes preadres = VG_(pread)( fd, macho_header, sizeof(struct MACH_HEADER), 0 );
 
-   const struct fat_header*  fh_be = buf;
-   const struct MACH_HEADER* mh    = buf;
+   if (sr_isError(preadres) || sr_Res(preadres) < sizeof(struct MACH_HEADER)) {
+      return False;
+   }
 
-   vg_assert(buf);
+   const struct fat_header*  fh_be = (const struct fat_header*)macho_header;
+   const struct MACH_HEADER* mh    = (const struct MACH_HEADER*)macho_header;
+
+   vg_assert(fh_be);
+   vg_assert(mh);
    vg_assert(rw_loads);
-   if (szB < sizeof(struct fat_header))
-      return False;
+   STATIC_ASSERT(sizeof(struct fat_header) <= sizeof(struct MACH_HEADER));
    if (VG_(ntohl)(fh_be->magic) == FAT_MAGIC) {
       // @todo PJF not yet handled, previous behaviour was to assume that the count is 1
       *rw_loads = 1;
       return True;
    }
 
-   if (szB < sizeof(struct MACH_HEADER))
-      return False;
    if (mh->magic == MAGIC) {
-      const struct load_command* lc = (const struct load_command*)((const char*)buf + sizeof(struct MACH_HEADER));
+      HChar* macho_load_commands = ML_(dinfo_zalloc)("di.readmacho.macho_load_commands", mh->sizeofcmds);
+      preadres = VG_(pread)( fd, macho_load_commands, mh->sizeofcmds, sizeof(struct MACH_HEADER) );
+      if (sr_isError(preadres) || sr_Res(preadres) < mh->sizeofcmds) {
+         ML_(dinfo_free)(macho_load_commands);
+         return False;
+      }
+
+      const struct load_command* lc = (const struct load_command*)macho_load_commands;
       for (unsigned int i = 0U; i < mh->ncmds; ++i) {
          if (lc->cmd == LC_SEGMENT_CMD) {
             const struct SEGMENT_COMMAND* sc = (const struct SEGMENT_COMMAND*)lc;
@@ -139,6 +147,7 @@ Bool ML_(check_macho_and_get_rw_loads)( const void* buf, SizeT szB, Int* rw_load
          const char* tmp = (const char*)lc + lc->cmdsize;
          lc = (const struct load_command*)tmp;
       }
+      ML_(dinfo_free)(macho_load_commands);
       return True;
    }