Menu
▾
▴
[Valgrind-developers] [valgrind] Bug 501194 - Fix ML_(check_macho_and_get_rw_loads) so that it is correct for any number of segment c
|
From: Paul F. <pa...@so...> - 2025-03-07 22:12:46
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=6999f18906f0b16d407eda3a9975cb3845dedcfe commit 6999f18906f0b16d407eda3a9975cb3845dedcfe Author: Paul Floyd <pj...@wa...> Date: Fri Mar 7 23:12:13 2025 +0100 Bug 501194 - Fix ML_(check_macho_and_get_rw_loads) so that it is correct for any number of segment commands Diff: --- NEWS | 1 + coregrind/m_debuginfo/debuginfo.c | 39 +++------------------------------- coregrind/m_debuginfo/priv_readmacho.h | 2 +- coregrind/m_debuginfo/readmacho.c | 33 +++++++++++++++++----------- 4 files changed, 26 insertions(+), 49 deletions(-) diff --git a/NEWS b/NEWS index 304ce1c0b7..0fcbc5d0e2 100644 --- 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 97d0f35c05..612833a997 100644 --- 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 b5172878ce..0cbdad90f2 100644 --- 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 e93bae7941..691ec94353 100644 --- 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; } |
