labath created this revision.
labath added reviewers: jasonmolenda, aprantl, clayborg.
Herald added subscribers: kristof.beyls, mgorny.
Herald added a reviewer: javed.absar.

Before this patch we were unable to write cross-platform MachO tests
because the parsing code did not compile on other platforms. The reason
for that was that ObjectFileMachO depended on
RegisterContextDarwin_arm(64)? (presumably for core file parsing) and
the two Register Context classes uses constants from the system headers
(KERN_SUCCESS, KERN_INVALID_ARGUMENT).

As far as I can tell, these two files don't actually interact with the
darwin kernel -- they are used only in ObjectFileMachO and MacOSX-Kernel
process plugin (even though it has "kernel" in the name, this one
communicates with it via network packets and not syscalls). So there is
no good reason to use os-specific error codes here. In fact, the
RegisterContextDarwin_i386/x86_64 classes and the code which uses them
already use 0/-1 constants for these purposes, so changing the arm
context to do the same makes the code more consistent.

This is the only change necessary (apart from build system glue) to make
ObjectFileMachO work on other platforms. To demonstrate that, I remove
REQUIRES:darwin from our (only) cross-platform mach-o test.


https://reviews.llvm.org/D46934

Files:
  lit/Modules/lc_version_min.yaml
  source/Initialization/CMakeLists.txt
  source/Initialization/SystemInitializerCommon.cpp
  source/Plugins/Process/Utility/RegisterContextDarwin_arm.cpp
  source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp

Index: source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp
===================================================================
--- source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp
+++ source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp
@@ -8,15 +8,8 @@
 //
 //===----------------------------------------------------------------------===//
 
-#if defined(__APPLE__)
-
 #include "RegisterContextDarwin_arm64.h"
 
-// C Includes
-#include <mach/mach_types.h>
-#include <mach/thread_act.h>
-#include <sys/sysctl.h>
-
 // C++ Includes
 // Other libraries and framework includes
 #include "lldb/Core/RegisterValue.h"
@@ -221,7 +214,7 @@
   int set = GPRRegSet;
   if (!RegisterSetIsCached(set)) {
     SetError(set, Write, -1);
-    return KERN_INVALID_ARGUMENT;
+    return -1;
   }
   SetError(set, Write, DoWriteGPR(GetThreadID(), set, gpr));
   SetError(set, Read, -1);
@@ -232,7 +225,7 @@
   int set = FPURegSet;
   if (!RegisterSetIsCached(set)) {
     SetError(set, Write, -1);
-    return KERN_INVALID_ARGUMENT;
+    return -1;
   }
   SetError(set, Write, DoWriteFPU(GetThreadID(), set, fpu));
   SetError(set, Read, -1);
@@ -243,7 +236,7 @@
   int set = EXCRegSet;
   if (!RegisterSetIsCached(set)) {
     SetError(set, Write, -1);
-    return KERN_INVALID_ARGUMENT;
+    return -1;
   }
   SetError(set, Write, DoWriteEXC(GetThreadID(), set, exc));
   SetError(set, Read, -1);
@@ -254,7 +247,7 @@
   int set = DBGRegSet;
   if (!RegisterSetIsCached(set)) {
     SetError(set, Write, -1);
-    return KERN_INVALID_ARGUMENT;
+    return -1;
   }
   SetError(set, Write, DoWriteDBG(GetThreadID(), set, dbg));
   SetError(set, Read, -1);
@@ -274,7 +267,7 @@
   default:
     break;
   }
-  return KERN_INVALID_ARGUMENT;
+  return -1;
 }
 
 int RegisterContextDarwin_arm64::WriteRegisterSet(uint32_t set) {
@@ -293,7 +286,7 @@
       break;
     }
   }
-  return KERN_INVALID_ARGUMENT;
+  return -1;
 }
 
 void RegisterContextDarwin_arm64::LogDBGRegisters(Log *log, const DBG &dbg) {
@@ -313,7 +306,7 @@
   if (set == -1)
     return false;
 
-  if (ReadRegisterSet(set, false) != KERN_SUCCESS)
+  if (ReadRegisterSet(set, false) != 0)
     return false;
 
   switch (reg) {
@@ -545,7 +538,7 @@
   if (set == -1)
     return false;
 
-  if (ReadRegisterSet(set, false) != KERN_SUCCESS)
+  if (ReadRegisterSet(set, false) != 0)
     return false;
 
   switch (reg) {
@@ -642,14 +635,14 @@
   default:
     return false;
   }
-  return WriteRegisterSet(set) == KERN_SUCCESS;
+  return WriteRegisterSet(set) == 0;
 }
 
 bool RegisterContextDarwin_arm64::ReadAllRegisterValues(
     lldb::DataBufferSP &data_sp) {
   data_sp.reset(new DataBufferHeap(REG_CONTEXT_SIZE, 0));
-  if (data_sp && ReadGPR(false) == KERN_SUCCESS &&
-      ReadFPU(false) == KERN_SUCCESS && ReadEXC(false) == KERN_SUCCESS) {
+  if (data_sp && ReadGPR(false) == 0 && ReadFPU(false) == 0 &&
+      ReadEXC(false) == 0) {
     uint8_t *dst = data_sp->GetBytes();
     ::memcpy(dst, &gpr, sizeof(gpr));
     dst += sizeof(gpr);
@@ -675,11 +668,11 @@
 
     ::memcpy(&exc, src, sizeof(exc));
     uint32_t success_count = 0;
-    if (WriteGPR() == KERN_SUCCESS)
+    if (WriteGPR() == 0)
       ++success_count;
-    if (WriteFPU() == KERN_SUCCESS)
+    if (WriteFPU() == 0)
       ++success_count;
-    if (WriteEXC() == KERN_SUCCESS)
+    if (WriteEXC() == 0)
       ++success_count;
     return success_count == 3;
   }
@@ -980,7 +973,7 @@
   // Read the debug state
   int kret = ReadDBG(false);
 
-  if (kret == KERN_SUCCESS) {
+  if (kret == 0) {
     // Check to make sure we have the needed hardware support
     uint32_t i = 0;
 
@@ -1007,7 +1000,7 @@
       //            ("RegisterContextDarwin_arm64::EnableHardwareWatchpoint()
       //            WriteDBG() => 0x%8.8x.", kret);
 
-      if (kret == KERN_SUCCESS)
+      if (kret == 0)
         return i;
     } else {
       //            if (log) log->Printf
@@ -1023,7 +1016,7 @@
   int kret = ReadDBG(false);
 
   const uint32_t num_hw_points = NumSupportedHardwareWatchpoints();
-  if (kret == KERN_SUCCESS) {
+  if (kret == 0) {
     if (hw_index < num_hw_points) {
       dbg.wcr[hw_index] = 0;
       //            if (log) log->Printf
@@ -1037,11 +1030,9 @@
 
       kret = WriteDBG();
 
-      if (kret == KERN_SUCCESS)
+      if (kret == 0)
         return true;
     }
   }
   return false;
 }
-
-#endif
Index: source/Plugins/Process/Utility/RegisterContextDarwin_arm.cpp
===================================================================
--- source/Plugins/Process/Utility/RegisterContextDarwin_arm.cpp
+++ source/Plugins/Process/Utility/RegisterContextDarwin_arm.cpp
@@ -7,14 +7,8 @@
 //
 //===----------------------------------------------------------------------===//
 
-#if defined(__APPLE__)
-
 #include "RegisterContextDarwin_arm.h"
 
-// C Includes
-#include <mach/mach_types.h>
-#include <mach/thread_act.h>
-
 // C++ Includes
 // Other libraries and framework includes
 #include "lldb/Core/RegisterValue.h"
@@ -1040,7 +1034,7 @@
   int set = GPRRegSet;
   if (!RegisterSetIsCached(set)) {
     SetError(set, Write, -1);
-    return KERN_INVALID_ARGUMENT;
+    return -1;
   }
   SetError(set, Write, DoWriteGPR(GetThreadID(), set, gpr));
   SetError(set, Read, -1);
@@ -1051,7 +1045,7 @@
   int set = FPURegSet;
   if (!RegisterSetIsCached(set)) {
     SetError(set, Write, -1);
-    return KERN_INVALID_ARGUMENT;
+    return -1;
   }
   SetError(set, Write, DoWriteFPU(GetThreadID(), set, fpu));
   SetError(set, Read, -1);
@@ -1062,7 +1056,7 @@
   int set = EXCRegSet;
   if (!RegisterSetIsCached(set)) {
     SetError(set, Write, -1);
-    return KERN_INVALID_ARGUMENT;
+    return -1;
   }
   SetError(set, Write, DoWriteEXC(GetThreadID(), set, exc));
   SetError(set, Read, -1);
@@ -1073,7 +1067,7 @@
   int set = DBGRegSet;
   if (!RegisterSetIsCached(set)) {
     SetError(set, Write, -1);
-    return KERN_INVALID_ARGUMENT;
+    return -1;
   }
   SetError(set, Write, DoWriteDBG(GetThreadID(), set, dbg));
   SetError(set, Read, -1);
@@ -1095,7 +1089,7 @@
   default:
     break;
   }
-  return KERN_INVALID_ARGUMENT;
+  return -1;
 }
 
 int RegisterContextDarwin_arm::WriteRegisterSet(uint32_t set) {
@@ -1116,7 +1110,7 @@
       break;
     }
   }
-  return KERN_INVALID_ARGUMENT;
+  return -1;
 }
 
 void RegisterContextDarwin_arm::LogDBGRegisters(Log *log, const DBG &dbg) {
@@ -1136,7 +1130,7 @@
   if (set == -1)
     return false;
 
-  if (ReadRegisterSet(set, false) != KERN_SUCCESS)
+  if (ReadRegisterSet(set, false) != 0)
     return false;
 
   switch (reg) {
@@ -1224,7 +1218,7 @@
   if (set == -1)
     return false;
 
-  if (ReadRegisterSet(set, false) != KERN_SUCCESS)
+  if (ReadRegisterSet(set, false) != 0)
     return false;
 
   switch (reg) {
@@ -1300,14 +1294,14 @@
   default:
     return false;
   }
-  return WriteRegisterSet(set) == KERN_SUCCESS;
+  return WriteRegisterSet(set) == 0;
 }
 
 bool RegisterContextDarwin_arm::ReadAllRegisterValues(
     lldb::DataBufferSP &data_sp) {
   data_sp.reset(new DataBufferHeap(REG_CONTEXT_SIZE, 0));
-  if (data_sp && ReadGPR(false) == KERN_SUCCESS &&
-      ReadFPU(false) == KERN_SUCCESS && ReadEXC(false) == KERN_SUCCESS) {
+  if (data_sp && ReadGPR(false) == 0 && ReadFPU(false) == 0 &&
+      ReadEXC(false) == 0) {
     uint8_t *dst = data_sp->GetBytes();
     ::memcpy(dst, &gpr, sizeof(gpr));
     dst += sizeof(gpr);
@@ -1333,11 +1327,11 @@
 
     ::memcpy(&exc, src, sizeof(exc));
     uint32_t success_count = 0;
-    if (WriteGPR() == KERN_SUCCESS)
+    if (WriteGPR() == 0)
       ++success_count;
-    if (WriteFPU() == KERN_SUCCESS)
+    if (WriteFPU() == 0)
       ++success_count;
-    if (WriteEXC() == KERN_SUCCESS)
+    if (WriteEXC() == 0)
       ++success_count;
     return success_count == 3;
   }
@@ -1543,7 +1537,7 @@
 
   int kret = ReadDBG(false);
 
-  if (kret == KERN_SUCCESS) {
+  if (kret == 0) {
     const uint32_t num_hw_breakpoints = NumSupportedHardwareBreakpoints();
     uint32_t i;
     for (i = 0; i < num_hw_breakpoints; ++i) {
@@ -1601,7 +1595,7 @@
       //            ("RegisterContextDarwin_arm::EnableHardwareBreakpoint()
       //            WriteDBG() => 0x%8.8x.", kret);
 
-      if (kret == KERN_SUCCESS)
+      if (kret == 0)
         return i;
     }
     //        else
@@ -1620,7 +1614,7 @@
   int kret = ReadDBG(false);
 
   const uint32_t num_hw_points = NumSupportedHardwareBreakpoints();
-  if (kret == KERN_SUCCESS) {
+  if (kret == 0) {
     if (hw_index < num_hw_points) {
       dbg.bcr[hw_index] = 0;
       //            if (log) log->Printf
@@ -1634,7 +1628,7 @@
 
       kret = WriteDBG();
 
-      if (kret == KERN_SUCCESS)
+      if (kret == 0)
         return true;
     }
   }
@@ -1704,7 +1698,7 @@
   // Read the debug state
   int kret = ReadDBG(false);
 
-  if (kret == KERN_SUCCESS) {
+  if (kret == 0) {
     // Check to make sure we have the needed hardware support
     uint32_t i = 0;
 
@@ -1731,7 +1725,7 @@
       //            ("RegisterContextDarwin_arm::EnableHardwareWatchpoint()
       //            WriteDBG() => 0x%8.8x.", kret);
 
-      if (kret == KERN_SUCCESS)
+      if (kret == 0)
         return i;
     } else {
       //            if (log) log->Printf
@@ -1746,7 +1740,7 @@
   int kret = ReadDBG(false);
 
   const uint32_t num_hw_points = NumSupportedHardwareWatchpoints();
-  if (kret == KERN_SUCCESS) {
+  if (kret == 0) {
     if (hw_index < num_hw_points) {
       dbg.wcr[hw_index] = 0;
       //            if (log) log->Printf
@@ -1760,11 +1754,9 @@
 
       kret = WriteDBG();
 
-      if (kret == KERN_SUCCESS)
+      if (kret == 0)
         return true;
     }
   }
   return false;
 }
-
-#endif
Index: source/Initialization/SystemInitializerCommon.cpp
===================================================================
--- source/Initialization/SystemInitializerCommon.cpp
+++ source/Initialization/SystemInitializerCommon.cpp
@@ -21,10 +21,7 @@
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Timer.h"
-
-#if defined(__APPLE__)
 #include "Plugins/ObjectFile/Mach-O/ObjectFileMachO.h"
-#endif
 
 #if defined(__linux__) || defined(__FreeBSD__) || defined(__NetBSD__)
 #include "Plugins/Process/POSIX/ProcessPOSIXLog.h"
@@ -82,6 +79,7 @@
   // Initialize plug-ins
   ObjectContainerBSDArchive::Initialize();
   ObjectFileELF::Initialize();
+  ObjectFileMachO::Initialize();
   ObjectFilePECOFF::Initialize();
 
   EmulateInstructionARM::Initialize();
@@ -93,9 +91,6 @@
   //----------------------------------------------------------------------
   ObjectContainerUniversalMachO::Initialize();
 
-#if defined(__APPLE__)
-  ObjectFileMachO::Initialize();
-#endif
 #if defined(__linux__) || defined(__FreeBSD__) || defined(__NetBSD__)
   ProcessPOSIXLog::Initialize();
 #endif
@@ -109,16 +104,14 @@
   Timer scoped_timer(func_cat, LLVM_PRETTY_FUNCTION);
   ObjectContainerBSDArchive::Terminate();
   ObjectFileELF::Terminate();
+  ObjectFileMachO::Terminate();
   ObjectFilePECOFF::Terminate();
 
   EmulateInstructionARM::Terminate();
   EmulateInstructionMIPS::Terminate();
   EmulateInstructionMIPS64::Terminate();
 
   ObjectContainerUniversalMachO::Terminate();
-#if defined(__APPLE__)
-  ObjectFileMachO::Terminate();
-#endif
 
 #if defined(_MSC_VER)
   ProcessWindowsLog::Terminate();
Index: source/Initialization/CMakeLists.txt
===================================================================
--- source/Initialization/CMakeLists.txt
+++ source/Initialization/CMakeLists.txt
@@ -1,7 +1,3 @@
-if ( CMAKE_SYSTEM_NAME MATCHES "Darwin" )
-  list(APPEND EXTRA_PLUGINS lldbPluginObjectFileMachO)
-endif()
-
 if ( CMAKE_SYSTEM_NAME MATCHES "Linux|Android|FreeBSD|NetBSD" )
   list(APPEND EXTRA_PLUGINS lldbPluginProcessPOSIX)
 endif()
@@ -24,6 +20,7 @@
     lldbPluginObjectContainerBSDArchive
     lldbPluginObjectContainerMachOArchive
     lldbPluginObjectFileELF
+    lldbPluginObjectFileMachO
     lldbPluginObjectFilePECOFF
     lldbPluginProcessGDBRemote
     ${EXTRA_PLUGINS}
Index: lit/Modules/lc_version_min.yaml
===================================================================
--- lit/Modules/lc_version_min.yaml
+++ lit/Modules/lc_version_min.yaml
@@ -1,6 +1,6 @@
 # RUN: yaml2obj %s > %t.out
 # RUN: lldb-test symbols %t.out | FileCheck %s
-# REQUIRES: darwin
+
 # Test that the deployment target is parsed from the load commands.
 # CHECK: x86_64-apple-macosx10.9.0
 --- !mach-o
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to