[llvm] r333797 - [MachO] Fixing ub in MachO BinaryFormat

[Chris Bieneman via llvm-commits]

Author: cbieneman
Date: Fri Jun  1 15:07:36 2018
New Revision: 333797

URL: http://llvm.org/viewvc/llvm-project?rev=333797&view=rev
Log:
[MachO] Fixing ub in MachO BinaryFormat

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.

Added:
    llvm/trunk/unittests/BinaryFormat/MachOTest.cpp
Modified:
    llvm/trunk/include/llvm/BinaryFormat/MachO.h
    llvm/trunk/unittests/BinaryFormat/CMakeLists.txt

Modified: llvm/trunk/include/llvm/BinaryFormat/MachO.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/BinaryFormat/MachO.h?rev=333797&r1=333796&r2=333797&view=diff
==============================================================================
--- llvm/trunk/include/llvm/BinaryFormat/MachO.h (original)
+++ llvm/trunk/include/llvm/BinaryFormat/MachO.h Fri Jun  1 15:07:36 2018
@@ -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

Modified: llvm/trunk/unittests/BinaryFormat/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/BinaryFormat/CMakeLists.txt?rev=333797&r1=333796&r2=333797&view=diff
==============================================================================
--- llvm/trunk/unittests/BinaryFormat/CMakeLists.txt (original)
+++ llvm/trunk/unittests/BinaryFormat/CMakeLists.txt Fri Jun  1 15:07:36 2018
@@ -4,6 +4,7 @@ set(LLVM_LINK_COMPONENTS
 
 add_llvm_unittest(BinaryFormatTests
   DwarfTest.cpp
+  MachOTest.cpp
   TestFileMagic.cpp
   )
 

Added: llvm/trunk/unittests/BinaryFormat/MachOTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/BinaryFormat/MachOTest.cpp?rev=333797&view=auto
==============================================================================
--- llvm/trunk/unittests/BinaryFormat/MachOTest.cpp (added)
+++ llvm/trunk/unittests/BinaryFormat/MachOTest.cpp Fri Jun  1 15:07:36 2018
@@ -0,0 +1,43 @@
+//===- 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) {
+  const 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};
+
+  const mach_header *Header =
+      reinterpret_cast<const mach_header *>(Valid32BitMachO);
+  ASSERT_EQ(Header->magic, MH_MAGIC);
+  const unsigned char *Current = Valid32BitMachO + sizeof(mach_header);
+  const unsigned char *BufferEnd =
+      Valid32BitMachO + sizeof(mach_header) + Header->sizeofcmds;
+  while (Current < BufferEnd) {
+    const macho_load_command *LC =
+        reinterpret_cast<const macho_load_command *>(Current);
+    ASSERT_EQ(LC->load_command_data.cmd, LC_SEGMENT);
+    Current += LC->load_command_data.cmdsize;
+  }
+}