From f88e9ada1edacd949d4d0785d15323dc68c51341 Mon Sep 17 00:00:00 2001 From: Chris Bieneman Date: Sun, 3 Jun 2018 20:33:42 +0000 Subject: [PATCH] Re-land: [MachO] Fixing ub in MachO BinaryFormat This re-lands r333797 with a fix for big endian systems. Original commit message: This isn't encountered anywhere inside LLVM, so I wrote a test case to expose the issue and verify that it is fixed. The basic problem is that the macho_load_command union contains all load comamnd structs. Load command structs in 32-bit macho files can be 32-bit aligned instead of 64-bit aligned. There are some strange circumstances in which this can be exposed in a 64-bit macho if the load commands are invalid or if a 32-bit aligned load command is used. In the past we've worked around this type of problem with changes like r264232. llvm-svn: 333854 --- include/llvm/BinaryFormat/MachO.h | 4 ++- unittests/BinaryFormat/CMakeLists.txt | 1 + unittests/BinaryFormat/MachOTest.cpp | 47 +++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 unittests/BinaryFormat/MachOTest.cpp diff --git a/include/llvm/BinaryFormat/MachO.h b/include/llvm/BinaryFormat/MachO.h index 060fbe162ad..2eacc6d8cc0 100644 --- a/include/llvm/BinaryFormat/MachO.h +++ b/include/llvm/BinaryFormat/MachO.h @@ -1973,9 +1973,11 @@ const uint32_t PPC_THREAD_STATE_COUNT = // Define a union of all load command structs #define LOAD_COMMAND_STRUCT(LCStruct) LCStruct LCStruct##_data; -union macho_load_command { +LLVM_PACKED_START +union LLVM_ALIGNAS(4) macho_load_command { #include "llvm/BinaryFormat/MachO.def" }; +LLVM_PACKED_END } // end namespace MachO } // end namespace llvm diff --git a/unittests/BinaryFormat/CMakeLists.txt b/unittests/BinaryFormat/CMakeLists.txt index 631936795b6..95c672e35be 100644 --- a/unittests/BinaryFormat/CMakeLists.txt +++ b/unittests/BinaryFormat/CMakeLists.txt @@ -4,6 +4,7 @@ set(LLVM_LINK_COMPONENTS add_llvm_unittest(BinaryFormatTests DwarfTest.cpp + MachOTest.cpp TestFileMagic.cpp ) diff --git a/unittests/BinaryFormat/MachOTest.cpp b/unittests/BinaryFormat/MachOTest.cpp new file mode 100644 index 00000000000..da8e9e8c690 --- /dev/null +++ b/unittests/BinaryFormat/MachOTest.cpp @@ -0,0 +1,47 @@ +//===- unittest/BinaryFormat/MachOTest.cpp - MachO support tests ----------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "llvm/BinaryFormat/MachO.h" +#include "gtest/gtest.h" + +using namespace llvm; +using namespace llvm::MachO; + +TEST(MachOTest, UnalignedLC) { + unsigned char Valid32BitMachO[] = { + 0xCE, 0xFA, 0xED, 0xFE, 0x07, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, + 0x02, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x70, 0x00, 0x00, 0x00, + 0x85, 0x80, 0x21, 0x01, 0x01, 0x00, 0x00, 0x00, 0x38, 0x00, 0x00, 0x00, + 0x5F, 0x5F, 0x50, 0x41, 0x47, 0x45, 0x5A, 0x45, 0x52, 0x4F, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x01, 0x00, 0x00, 0x00, 0x38, 0x00, 0x00, 0x00, 0x5F, 0x5F, 0x4C, 0x49, + 0x4E, 0x4B, 0x45, 0x44, 0x49, 0x54, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x40, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x30, 0x00, 0x00, + 0x8C, 0x0B, 0x00, 0x00, 0x07, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; + + mach_header *Header = + reinterpret_cast(Valid32BitMachO); + if (!sys::IsLittleEndianHost) + swapStruct(*Header); + ASSERT_EQ(Header->magic, MH_MAGIC); + unsigned char *Current = Valid32BitMachO + sizeof(mach_header); + unsigned char *BufferEnd = + Valid32BitMachO + sizeof(mach_header) + Header->sizeofcmds; + while (Current < BufferEnd) { + macho_load_command *LC = + reinterpret_cast(Current); + if (!sys::IsLittleEndianHost) + swapStruct(LC->load_command_data); + ASSERT_EQ(LC->load_command_data.cmd, LC_SEGMENT); + Current += LC->load_command_data.cmdsize; + } +}