[PATCH] [lld] [mach-o] implement minimal __unwind_info support

kledzik at apple.com kledzik at apple.com
Tue Sep 9 13:34:42 PDT 2014


A few nits, but over all a great start.

================
Comment at: lib/ReaderWriter/MachO/CompactUnwindPass.cpp:100-102
@@ +99,5 @@
+
+    ulittle32_t *headerEntries = (ulittle32_t *)contents.data();
+    // version
+    headerEntries[0] = 1;
+    // commonEncodingsArraySectionOffset
----------------
This style (ulittle32_t) won't scale if we ever have a big endian architecture.  That is why I use the style elsewhere of:
    write32(headerEntries[0], _swap, 1);

================
Comment at: lib/ReaderWriter/MachO/CompactUnwindPass.cpp:229-234
@@ +228,8 @@
+  mach_o::ArchHandler &_archHandler;
+  std::vector<uint8_t> contents;
+  uint32_t commonEncodingsOffset;
+  uint32_t personalityArrayOffset;
+  uint32_t topLevelIndexOffset;
+  uint32_t lsdaIndexOffset;
+  uint32_t firstPageOffset;
+};
----------------
These ivars need a leading underscore.

================
Comment at: lib/ReaderWriter/MachO/CompactUnwindPass.hpp:1-31
@@ +1,31 @@
+//===- lib/ReaderWriter/MachO/CompactUnwindPass.hpp -----------------------===//
+//
+//                             The LLVM Linker
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLD_READER_WRITER_MACHO_COMPACT_UNWIND_PASS_H
+#define LLD_READER_WRITER_MACHO_COMPACT_UNWIND_PASS_H
+
+#include "lld/Core/DefinedAtom.h"
+#include "lld/Core/SharedLibraryAtom.h"
+#include "lld/Core/File.h"
+#include "lld/Core/Reference.h"
+#include "lld/Core/Pass.h"
+
+#include "MachOPasses.h"
+#include "ReferenceKinds.h"
+#include "StubAtoms.hpp"
+
+namespace lld {
+namespace mach_o {
+
+
+} // namespace mach_o
+} // namespace lld
+
+
+#endif // LLD_READER_WRITER_MACHO_GOT_PASS_H
----------------
Why does this file exist?

================
Comment at: lib/ReaderWriter/MachO/MachOLinkingContext.cpp:273-280
@@ -272,2 +272,10 @@
 
+bool MachOLinkingContext::needsCompactUnwindPass() const {
+  switch (_outputMachOType) {
+  case MH_EXECUTE:
+    return true;
+  default:
+    return false;
+  }
+}
 
----------------
The should also be true for MH_DYLIB and MH_BUNDLE.  It should also call archHandler. needsCompactUnwind().

================
Comment at: lib/ReaderWriter/MachO/MachONormalizedFileFromAtoms.cpp:452
@@ -447,3 +451,3 @@
                                                                uint64_t &addr) {
-  seg->address = addr;
+  seg->address = _imageBase = addr;
   // Walks sections starting at end to calculate padding for start.
----------------
_imageBase should be initialized in the constructor from a new method MachOLinkingContext::baseAddress().

Then in assignAddressesToSections(), the FIXME for the initial value for "address" can be changed to be _imageBase.

================
Comment at: test/mach-o/unwind-info-simple.yaml:1
@@ +1,2 @@
+# RUN: lld -flavor darwin -arch x86_64 %s -o %t -e _main
+# RUN: llvm-objdump -unwind-info %t | FileCheck %s
----------------
Should name this test case with x86_64 in the name, since will need similar test case for other arches

================
Comment at: test/mach-o/unwind-info-simple.yaml:68-71
@@ +67,6 @@
+
+  - name:            dyld_stub_binder
+    scope:           global
+    type:            data
+    content:         [ 00 ]
+shared-library-atoms:
----------------
You can add %p/Inputs/libSystem.yaml to the link line, then you don't need to define dyld_stub_binder

http://reviews.llvm.org/D5261






More information about the llvm-commits mailing list