[PATCH] [Mach-O] Dtrace Support Part 1: User SDT provider handling.

[kledzik at apple.com]

Thanks for working on this!


REPOSITORY
  rL LLVM

================
Comment at: lib/ReaderWriter/MachO/ArchHandler.h:258-265
@@ -257,1 +257,10 @@
 
+  // References common to all Mach-O formats
+  enum MachOReferenceKind : Reference::KindValue {
+    invalid,               /// for error condition
+
+    dtraceProbe,           /// ___dtrace_probe$…
+    dtraceIsEnabled,       /// ___dtrace_isenabled$…
+    dtraceExtraInfo
+  };
+
----------------
We don't need this in the public interface for ArchHandler.  All the "kind" values are private to each ArchHandler subclass. 

================
Comment at: lib/ReaderWriter/MachO/ArchHandler.h:267-269
@@ +266,5 @@
+
+  /// Utility method to check if an undefined reference is a dtrace reference
+  Reference::KindValue branchKindForTarget(const Atom *target,
+                                           Reference::KindValue def);
+
----------------
This should be virtual and each subclass implements, then you don't need to pass in the "def" parameter.  See later comment about changing this to isDtraceSite()

================
Comment at: lib/ReaderWriter/MachO/ArchHandler_arm.cpp:542
@@ -534,2 +541,3 @@
       return ec;
+    *kind = branchKindForTarget(*target, *kind);
     // Instruction contains branch to addend.
----------------
This could be:
  bool isProbe;
  if ( isDtraceSite(*target, isProbe) ) {
      *kind = isProbe ? dtraceProbe : dtraceIsEnabled;
  }

Where isDtraceSite() replaces branchKindForTarget() 

================
Comment at: lib/ReaderWriter/MachO/ArchHandler_arm.cpp:1015-1030
@@ -1005,1 +1014,18 @@
     break;
+  case dtraceProbe:
+    if (thumbMode)
+      // change 32-bit blx call site to two thumb NOPs
+      *loc32 = 0x46C046C0;
+    else
+      // change call site to a NOP
+      *loc32 = 0xE1A00000;
+    break;
+  case dtraceIsEnabled:
+    if (thumbMode)
+      // change 32-bit blx call site to 'nop', 'eor r0, r0'
+      *loc32 = 0x46C04040;
+    else
+      // change call site to 'eor r0, r0, r0'
+      *loc32 = 0xE0200000;
+    break;
+  case dtraceExtraInfo:
----------------
Be nice to have an assert here that reads the *loc32 and validates it was a BL before changing instruction.

================
Comment at: lib/ReaderWriter/MachO/ArchHandler_arm.cpp:1230-1234
@@ -1196,3 +1229,7 @@
   uint16_t other16;
-  switch (static_cast<Arm_Kinds>(ref.kindValue())) {
+  Arm_Kinds kind = static_cast<Arm_Kinds>(ref.kindValue());
+  if (kind == dtraceProbe || kind == dtraceIsEnabled) {
+    kind = isThumbFunction(atom) ? thumb_b22 : arm_b24;
+  }
+  switch (kind) {
   case modeThumbCode:
----------------
Hmmm.  We should have dtraceProbeThumb and dtraceProbeArm in ArchHandler_arm.  That will simply this.

http://reviews.llvm.org/D7695

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/