This is an archive of the discontinued LLVM Phabricator instance.

[XRay][compiler-rt][x86_64] Define a tail exit trampoline.
ClosedPublic

Authored by dberris on Oct 26 2016, 9:51 PM.

Details

Summary

We define a new trampoline that's a hybrid between the exit and entry
trampolines with the following properties:

  • Saves all of the callee-saved registers according to the x86_64 calling conventions.
  • Indicate to the log handler function being called that this is a function exit event.

This fixes a bug that is a result of not saving enough of the register
states, and that the log handler is clobbering registers that would be
used by the function being tail-exited into manifesting as runtime
errors.

Event Timeline

dberris updated this revision to Diff 75979.Oct 26 2016, 9:51 PM
dberris retitled this revision from to [XRay][x86_64] Define a tail exit trampoline..
dberris updated this object.
dberris added reviewers: rSerge, echristo.
dberris added a subscriber: llvm-commits.
dberris updated this revision to Diff 75981.Oct 26 2016, 11:44 PM
dberris retitled this revision from [XRay][x86_64] Define a tail exit trampoline. to [XRay][compiler-rt][x86_64] Define a tail exit trampoline..

Reword description.

majnemer added inline comments.
lib/xray/xray_x86_64.cc
116–117

Is this comment still correct?

dberris added inline comments.Oct 27 2016, 12:24 AM
lib/xray/xray_x86_64.cc
116–117

Oh, nope, good catch. Thanks!

dberris updated this revision to Diff 75984.Oct 27 2016, 12:25 AM
dberris marked an inline comment as done.
  • Remove outdated FIXME
dberris updated this revision to Diff 75986.Oct 27 2016, 12:28 AM
  • Update comment to new reality
majnemer accepted this revision.Nov 1 2016, 5:04 PM
majnemer edited edge metadata.

LGTM

This revision is now accepted and ready to land.Nov 1 2016, 5:04 PM
This revision was automatically updated to reflect the committed changes.
rSerge added inline comments.Nov 29 2016, 10:09 AM
compiler-rt/trunk/lib/xray/xray_trampoline_x86_64.S
147 ↗(On Diff #76671)

Shouldn't here be $2, because enum XRayEntryType { ENTRY = 0, EXIT = 1, TAIL = 2 }; and it's tail exit?

dberris added inline comments.Nov 29 2016, 3:15 PM
compiler-rt/trunk/lib/xray/xray_trampoline_x86_64.S
147 ↗(On Diff #76671)

I thought about it, but as documented we don't want to introduce a new type yet (i.e. keep it simple as entry, exit only). Updating the format version allows us to introduce things like tail exits, exception exits, and other kinds of exit conditions (and maybe even different entry types too in case of, say, coroutines).