This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by Jean-Daniel on Feb 17 2015, 2:47 AM.

Details

Reviewers
kledzik
Summary

Properly recognize dtrace symbols and convert call sites to nop.

While this change allows to compile a program with SDT providers,
the generated binary does not contain the dof section required to enable the defined probes at runtime.

Design note:
Dtrace user probes are encoded as undefined function calls in the generated objets. When reading a Mach-O object, we mark all dtrace calls as 'canBeNullAtBuildTime' so the resolver does not trigger 'unresolved symbol' error. Darwin does not support that flag, so using it on dtrace symbol should be OK.

Then, for each branch relocation that target a dtrace call, we generate a special reference, dtraceProbe or dtraceIsEnabled.

The dtraceProbe and dtraceIsEnabled relocations are used to replace the dtrace call by nop when generating the final executable.

Diff Detail

Repository
rL LLVM

Event Timeline

Jean-Daniel retitled this revision from to [Mach-O] Dtrace Support Part 1: User SDT provider handling..
Jean-Daniel updated this object.
Jean-Daniel edited the test plan for this revision. (Show Details)
Jean-Daniel added a reviewer: kledzik.
Jean-Daniel added a subscriber: Unknown Object (MLST).
Jean-Daniel updated this object.Feb 17 2015, 3:02 AM
Jean-Daniel set the repository for this revision to rL LLVM.
Jean-Daniel added a project: lld.
alexr added a subscriber: alexr.Feb 17 2015, 8:07 AM

DTrace exists for ELF as well. Can some of this be moved to support that?

I don't think. ELF and Mach-O don't share any code and are not designed to do it.
Apart from the dtrace symbol parsing code, I don't see what can be shared with ELF here..

kledzik edited edge metadata.Mar 2 2015, 6:34 PM

Thanks for working on this!

lib/ReaderWriter/MachO/ArchHandler.h
258–265

We don't need this in the public interface for ArchHandler. All the "kind" values are private to each ArchHandler subclass.

267–269

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()

lib/ReaderWriter/MachO/ArchHandler_arm.cpp
546

This could be:

bool isProbe;
if ( isDtraceSite(*target, isProbe) ) {
    *kind = isProbe ? dtraceProbe : dtraceIsEnabled;
}

Where isDtraceSite() replaces branchKindForTarget()

1023–1038

Be nice to have an assert here that reads the *loc32 and validates it was a BL before changing instruction.

1244–1245

Hmmm. We should have dtraceProbeThumb and dtraceProbeArm in ArchHandler_arm. That will simply this.

Jean-Daniel updated this revision to Diff 21336.Mar 6 2015, 1:03 AM
Jean-Daniel updated this object.
Jean-Daniel edited edge metadata.

Take 2: Thanks for the review.

Dtrace relocation kinds where exposed to simplify the Dtrace relocation handling in the DofPass implementation (the Part 2). But I think it need more work anyway, so I completely removed the Dtrace ExtraInfo support for now and will reintroduce it when needed.

As I'm not an assembly expert, I'm not 100% sure about the assert to check the replaced instructions.