aprantl updated this revision to Diff 283669.
aprantl added a comment.

Address review feedback.


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

https://reviews.llvm.org/D85049

Files:
  lldb/source/Target/TargetList.cpp
  lldb/test/API/macosx/universal/Makefile
  lldb/test/API/macosx/universal/TestUniversal.py

Index: lldb/test/API/macosx/universal/TestUniversal.py
===================================================================
--- lldb/test/API/macosx/universal/TestUniversal.py
+++ lldb/test/API/macosx/universal/TestUniversal.py
@@ -1,7 +1,3 @@
-"""Test aspects of lldb commands on universal binaries."""
-
-
-
 import unittest2
 import os
 import lldb
@@ -14,6 +10,7 @@
     return "AVX2" in features.split()
 
 class UniversalTestCase(TestBase):
+    """Test aspects of lldb commands on universal binaries."""
 
     NO_DEBUG_INFO_TESTCASE = True
     mydir = TestBase.compute_mydir(__file__)
@@ -39,9 +36,10 @@
 
         # Create a target by the debugger.
         target = self.dbg.CreateTargetWithFileAndTargetTriple(
-            exe, "x86_64-apple-macosx")
+            exe, "x86_64-apple-macosx10.10")
         self.assertTrue(target, VALID_TARGET)
-        self.expect("image list -A -b", substrs=["x86_64 testit"])
+        self.expect("image list -t -b", substrs=["x86_64-apple-macosx10.9.0 testit"])
+        self.expect("target list", substrs=["testit", "arch=x86_64-apple-macosx10.10"])
 
         # Now launch the process, and do not stop at entry point.
         process = target.LaunchSimple(
Index: lldb/test/API/macosx/universal/Makefile
===================================================================
--- lldb/test/API/macosx/universal/Makefile
+++ lldb/test/API/macosx/universal/Makefile
@@ -8,13 +8,13 @@
 	lipo -create -o testit $^
 
 testit.x86_64h: testit.x86_64h.o
-	$(CC) -isysroot $(SDKROOT) -arch x86_64h -o testit.x86_64h $<
+	$(CC) -isysroot $(SDKROOT) -target x86_64h-apple-macosx10.9 -o testit.x86_64h $<
 
 testit.x86_64: testit.x86_64.o
-	$(CC) -isysroot $(SDKROOT) -arch x86_64 -o testit.x86_64 $<
+	$(CC) -isysroot $(SDKROOT) -target x86_64-apple-macosx10.9 -o testit.x86_64 $<
 
 testit.x86_64h.o: main.c
-	$(CC) -isysroot $(SDKROOT) -g -O0 -arch x86_64h -c -o testit.x86_64h.o $<
+	$(CC) -isysroot $(SDKROOT) -g -O0 -target x86_64h-apple-macosx10.9-apple-macosx10.9-apple-macosx10.9-apple-macosx10.9 -c -o testit.x86_64h.o $<
 
 testit.x86_64.o: main.c
-	$(CC) -isysroot $(SDKROOT) -g -O0 -arch x86_64 -c -o testit.x86_64.o $<
+	$(CC) -isysroot $(SDKROOT) -g -O0 -target x86_64-apple-macosx10.9 -c -o testit.x86_64.o $<
Index: lldb/source/Target/TargetList.cpp
===================================================================
--- lldb/source/Target/TargetList.cpp
+++ lldb/source/Target/TargetList.cpp
@@ -104,6 +104,15 @@
   }
 
   bool prefer_platform_arch = false;
+  auto update_platform_arch = [&](const ArchSpec &module_arch) {
+    // If the OS or vendor weren't specified, then adopt the module's
+    // architecture so that the platform matching can be more accurate.
+    if (!platform_arch.TripleOSWasSpecified() ||
+        !platform_arch.TripleVendorWasSpecified()) {
+      prefer_platform_arch = true;
+      platform_arch = module_arch;
+    }
+  };
 
   if (!user_exe_path.empty()) {
     ModuleSpec module_spec(FileSpec(user_exe_path, FileSpec::Style::native));
@@ -155,16 +164,14 @@
           }
         }
       } else if (arch.IsValid()) {
-        // A (valid) architecture was specified.
+        // Fat binary. A (valid) architecture was specified.
         module_spec.GetArchitecture() = arch;
         if (module_specs.FindMatchingModuleSpec(module_spec,
-                                                matching_module_spec)) {
-          prefer_platform_arch = true;
-          platform_arch = matching_module_spec.GetArchitecture();
-        }
+                                                matching_module_spec))
+            update_platform_arch(matching_module_spec.GetArchitecture());
       } else {
-        // No architecture specified, check if there is only one platform for
-        // all of the architectures.
+        // Fat binary. No architecture specified, check if there is
+        // only one platform for all of the architectures.
         PlatformSP host_platform_sp = Platform::GetHostPlatform();
         std::vector<PlatformSP> platforms;
         for (size_t i = 0; i < num_specs; ++i) {
@@ -251,7 +258,7 @@
   // If we have a valid architecture, make sure the current platform is
   // compatible with that architecture.
   if (!prefer_platform_arch && arch.IsValid()) {
-    if (!platform_sp->IsCompatibleArchitecture(arch, false, &platform_arch)) {
+    if (!platform_sp->IsCompatibleArchitecture(arch, false, nullptr)) {
       platform_sp = Platform::GetPlatformForArchitecture(arch, &platform_arch);
       if (!is_dummy_target && platform_sp)
         debugger.GetPlatformList().SetSelectedPlatform(platform_sp);
@@ -260,8 +267,7 @@
     // If "arch" isn't valid, yet "platform_arch" is, it means we have an
     // executable file with a single architecture which should be used.
     ArchSpec fixed_platform_arch;
-    if (!platform_sp->IsCompatibleArchitecture(platform_arch, false,
-                                               &fixed_platform_arch)) {
+    if (!platform_sp->IsCompatibleArchitecture(platform_arch, false, nullptr)) {
       platform_sp = Platform::GetPlatformForArchitecture(platform_arch,
                                                          &fixed_platform_arch);
       if (!is_dummy_target && platform_sp)
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to