[Lldb-commits] [PATCH] D147012: [lldb] Support Universal Mach-O binaries with a fat64 header

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfda53ad9374b: [lldb] Support Universal Mach-O binaries with 
a fat64 header (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147012/new/

https://reviews.llvm.org/D147012

Files:
  lldb/packages/Python/lldbsuite/test/make/Makefile.rules
  
lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp
  
lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.h
  lldb/test/API/macosx/universal64/Makefile
  lldb/test/API/macosx/universal64/TestUniversal64.py
  lldb/test/API/macosx/universal64/main.c

Index: lldb/test/API/macosx/universal64/main.c
===
--- /dev/null
+++ lldb/test/API/macosx/universal64/main.c
@@ -0,0 +1,5 @@
+#include 
+
+int foo() { return 0; }
+
+int main(int argc, char **argv) { return foo(); }
Index: lldb/test/API/macosx/universal64/TestUniversal64.py
===
--- /dev/null
+++ lldb/test/API/macosx/universal64/TestUniversal64.py
@@ -0,0 +1,39 @@
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class Universal64TestCase(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+def do_test(self):
+# Get the executable.
+exe = self.getBuildArtifact("fat.out")
+
+# Create a target.
+self.target = self.dbg.CreateTarget(exe)
+
+# Create a breakpoint on main.
+main_bp = self.target.BreakpointCreateByName("main")
+self.assertTrue(main_bp, VALID_BREAKPOINT)
+
+# Make sure the binary and the dSYM are in the image list.
+self.expect("image list ", patterns=['fat.out', 'fat.out.dSYM'])
+
+# The dynamic loader doesn't support fat64 executables so we can't
+# actually launch them here.
+
+@skipUnlessDarwin
+@skipIfDarwinEmbedded
+def test_universal64_executable(self):
+"""Test fat64 universal executable"""
+self.build(debug_info="dsym")
+self.do_test()
+
+@skipUnlessDarwin
+@skipIfDarwinEmbedded
+@skipIf(compiler="clang", compiler_version=['<', '7.0'])
+def test_universal64_dsym(self):
+"""Test fat64 universal dSYM"""
+self.build(debug_info="dsym", dictionary={'FAT64_DSYM': '1'})
+self.do_test()
Index: lldb/test/API/macosx/universal64/Makefile
===
--- /dev/null
+++ lldb/test/API/macosx/universal64/Makefile
@@ -0,0 +1,24 @@
+EXE := fat.out
+
+ifdef FAT64_DSYM
+	DSFLAGS_EXTRAS=-fat64
+endif
+
+include Makefile.rules
+
+all: fat.out
+
+fat.out: fat.arm64.out fat.x86_64.out
+	lipo -fat64 -create -o $@ $^
+
+fat.x86_64.out: fat.x86_64.o
+	$(CC) -isysroot $(SDKROOT) -target x86_64-apple-macosx10.9 -o $@ $<
+
+fat.arm64.out: fat.arm64.o
+	$(CC) -isysroot $(SDKROOT) -target arm64-apple-macosx10.9 -o $@ $<
+
+fat.x86_64.o: main.c
+	$(CC) -isysroot $(SDKROOT) -g -O0 -target x86_64-apple-macosx11 -c -o $@ $<
+
+fat.arm64.o: main.c
+	$(CC) -isysroot $(SDKROOT) -g -O0 -target arm64-apple-macosx11 -c -o $@ $<
Index: lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.h
===
--- lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.h
+++ lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.h
@@ -63,11 +63,46 @@
 
 protected:
   llvm::MachO::fat_header m_header;
-  std::vector m_fat_archs;
+
+  struct FatArch {
+FatArch(llvm::MachO::fat_arch arch) : m_arch(arch), m_is_fat64(false) {}
+FatArch(llvm::MachO::fat_arch_64 arch) : m_arch(arch), m_is_fat64(true) {}
+
+uint32_t GetCPUType() const {
+  return m_is_fat64 ? m_arch.fat_arch_64.cputype : m_arch.fat_arch.cputype;
+}
+
+uint32_t GetCPUSubType() const {
+  return m_is_fat64 ? m_arch.fat_arch_64.cpusubtype
+: m_arch.fat_arch.cpusubtype;
+}
+
+uint64_t GetOffset() const {
+  return m_is_fat64 ? m_arch.fat_arch_64.offset : m_arch.fat_arch.offset;
+}
+
+uint64_t GetSize() const {
+  return m_is_fat64 ? m_arch.fat_arch_64.size : m_arch.fat_arch.size;
+}
+
+uint32_t GetAlign() const {
+  return m_is_fat64 ? m_arch.fat_arch_64.align : m_arch.fat_arch.align;
+}
+
+  private:
+const union Arch {
+  Arch(llvm::MachO::fat_arch arch) : fat_arch(arch) {}
+  Arch(llvm::MachO::fat_arch_64 arch) : fat_arch_64(arch) {}
+  llvm::MachO::fat_arch fat_arch;
+  llvm::MachO::fat_arch_64 fat_arch_64;
+} m_arch;
+const bool m_is_fat64;
+  };
+  std::vector m_fat_archs;
 
   static bool ParseHeader(lldb_private::DataExtractor ,
   

[Lldb-commits] [PATCH] D147012: [lldb] Support Universal Mach-O binaries with a fat64 header

[Adrian Prantl via Phabricator via lldb-commits]

aprantl accepted this revision.
aprantl added a comment.
This revision is now accepted and ready to land.

Thanks for checking!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147012/new/

https://reviews.llvm.org/D147012

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147012: [lldb] Support Universal Mach-O binaries with a fat64 header

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere marked 2 inline comments as done.
JDevlieghere added inline comments.



Comment at: 
lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp:205
+const lldb::offset_t slice_file_offset =
+fat_arch.GetOffset() + file_offset;
+if (fat_arch.GetOffset() < file_size && file_size > slice_file_offset) 
{

JDevlieghere wrote:
> aprantl wrote:
> > Should this use AddOverflow?
> > https://llvm.org/doxygen/MathExtras_8h.html
> Yes, but nothing in `ObjectContainerUniversalMachO` is currently overflow 
> aware. I'll fix up the whole file in a follow-up. 
Actually, it looks like `AddOverflow` only checks for signed overflow 
(`std::enable_if_t< std::is_signed< T >::value, T >`). I assumed it was going 
through APInt under the hood. I think that's probably overkill here. 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147012/new/

https://reviews.llvm.org/D147012

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147012: [lldb] Support Universal Mach-O binaries with a fat64 header

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere marked 2 inline comments as done.
JDevlieghere added inline comments.



Comment at: 
lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp:205
+const lldb::offset_t slice_file_offset =
+fat_arch.GetOffset() + file_offset;
+if (fat_arch.GetOffset() < file_size && file_size > slice_file_offset) 
{

aprantl wrote:
> Should this use AddOverflow?
> https://llvm.org/doxygen/MathExtras_8h.html
Yes, but nothing in `ObjectContainerUniversalMachO` is currently overflow 
aware. I'll fix up the whole file in a follow-up. 



Comment at: lldb/test/API/macosx/universal64/TestUniversal64.py:21
+# Make sure the binary and the dSYM are in the image list.
+self.expect("image list ", patterns=['fat.out', 'fat.out.dSYM'])
+

aprantl wrote:
> would it be easy to verify in the test or the makefile that the dsym indeed 
> has a fat header?
`llvm-objdump` can dump the universal headers but that's currently not a 
dependency of the test suite and I didn't think it was worth adding just for a 
sanity check.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147012/new/

https://reviews.llvm.org/D147012

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147012: [lldb] Support Universal Mach-O binaries with a fat64 header

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: 
lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp:205
+const lldb::offset_t slice_file_offset =
+fat_arch.GetOffset() + file_offset;
+if (fat_arch.GetOffset() < file_size && file_size > slice_file_offset) 
{

Should this use AddOverflow?
https://llvm.org/doxygen/MathExtras_8h.html



Comment at: lldb/test/API/macosx/universal64/TestUniversal64.py:21
+# Make sure the binary and the dSYM are in the image list.
+self.expect("image list ", patterns=['fat.out', 'fat.out.dSYM'])
+

would it be easy to verify in the test or the makefile that the dsym indeed has 
a fat header?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147012/new/

https://reviews.llvm.org/D147012

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147012: [lldb] Support Universal Mach-O binaries with a fat64 header

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: jasonmolenda, pete, aprantl.
Herald added a project: All.
JDevlieghere requested review of this revision.

Support universal Mach-O binaries with a fat64 header. After D146879 
, dsymutil can now generate such binaries 
when the offsets would otherwise overflow the 32-bit offsets in the regular fat 
header.

rdar://107289570


https://reviews.llvm.org/D147012

Files:
  lldb/packages/Python/lldbsuite/test/make/Makefile.rules
  
lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp
  
lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.h
  lldb/test/API/macosx/universal64/Makefile
  lldb/test/API/macosx/universal64/TestUniversal64.py
  lldb/test/API/macosx/universal64/main.c

Index: lldb/test/API/macosx/universal64/main.c
===
--- /dev/null
+++ lldb/test/API/macosx/universal64/main.c
@@ -0,0 +1,5 @@
+#include 
+
+int foo() { return 0; }
+
+int main(int argc, char **argv) { return foo(); }
Index: lldb/test/API/macosx/universal64/TestUniversal64.py
===
--- /dev/null
+++ lldb/test/API/macosx/universal64/TestUniversal64.py
@@ -0,0 +1,39 @@
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class Universal64TestCase(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+def do_test(self):
+# Get the executable.
+exe = self.getBuildArtifact("fat.out")
+
+# Create a target.
+self.target = self.dbg.CreateTarget(exe)
+
+# Create a breakpoint on main.
+main_bp = self.target.BreakpointCreateByName("main")
+self.assertTrue(main_bp, VALID_BREAKPOINT)
+
+# Make sure the binary and the dSYM are in the image list.
+self.expect("image list ", patterns=['fat.out', 'fat.out.dSYM'])
+
+# The dynamic loader doesn't support fat64 executables so we can't
+# actually launch them here.
+
+@skipUnlessDarwin
+@skipIfDarwinEmbedded
+def test_universal64_executable(self):
+"""Test fat64 universal executable"""
+self.build(debug_info="dsym")
+self.do_test()
+
+@skipUnlessDarwin
+@skipIfDarwinEmbedded
+@skipIf(compiler="clang", compiler_version=['<', '7.0'])
+def test_universal64_dsym(self):
+"""Test fat64 universal dSYM"""
+self.build(debug_info="dsym", dictionary={'FAT64_DSYM': '1'})
+self.do_test()
Index: lldb/test/API/macosx/universal64/Makefile
===
--- /dev/null
+++ lldb/test/API/macosx/universal64/Makefile
@@ -0,0 +1,24 @@
+EXE := fat.out
+
+ifdef FAT64_DSYM
+	DSFLAGS_EXTRAS=-fat64
+endif
+
+include Makefile.rules
+
+all: fat.out
+
+fat.out: fat.arm64.out fat.x86_64.out
+	lipo -fat64 -create -o $@ $^
+
+fat.x86_64.out: fat.x86_64.o
+	$(CC) -isysroot $(SDKROOT) -target x86_64-apple-macosx10.9 -o $@ $<
+
+fat.arm64.out: fat.arm64.o
+	$(CC) -isysroot $(SDKROOT) -target arm64-apple-macosx10.9 -o $@ $<
+
+fat.x86_64.o: main.c
+	$(CC) -isysroot $(SDKROOT) -g -O0 -target x86_64-apple-macosx11 -c -o $@ $<
+
+fat.arm64.o: main.c
+	$(CC) -isysroot $(SDKROOT) -g -O0 -target arm64-apple-macosx11 -c -o $@ $<
Index: lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.h
===
--- lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.h
+++ lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.h
@@ -63,11 +63,46 @@
 
 protected:
   llvm::MachO::fat_header m_header;
-  std::vector m_fat_archs;
+
+  struct FatArch {
+FatArch(llvm::MachO::fat_arch arch) : m_arch(arch), m_is_fat64(false) {}
+FatArch(llvm::MachO::fat_arch_64 arch) : m_arch(arch), m_is_fat64(true) {}
+
+uint32_t GetCPUType() const {
+  return m_is_fat64 ? m_arch.fat_arch_64.cputype : m_arch.fat_arch.cputype;
+}
+
+uint32_t GetCPUSubType() const {
+  return m_is_fat64 ? m_arch.fat_arch_64.cpusubtype
+: m_arch.fat_arch.cpusubtype;
+}
+
+uint64_t GetOffset() const {
+  return m_is_fat64 ? m_arch.fat_arch_64.offset : m_arch.fat_arch.offset;
+}
+
+uint64_t GetSize() const {
+  return m_is_fat64 ? m_arch.fat_arch_64.size : m_arch.fat_arch.size;
+}
+
+uint32_t GetAlign() const {
+  return m_is_fat64 ? m_arch.fat_arch_64.align : m_arch.fat_arch.align;
+}
+
+  private:
+const union Arch {
+  Arch(llvm::MachO::fat_arch arch) : fat_arch(arch) {}
+  Arch(llvm::MachO::fat_arch_64 arch) : fat_arch_64(arch) {}
+  llvm::MachO::fat_arch fat_arch;
+  llvm::MachO::fat_arch_64 fat_arch_64;
+} m_arch;
+const bool m_is_fat64;
+  };
+  std::vector m_fat_archs;