RFC: Add compiler support for instrumenting functions

[pmur]

This RFC proposes to add support for instrumenting function calls using mcount fentry or xray, and adding a new builtin attribute to toggle instrumentation.

Today, mcount and xray enjoy experimental support, but I was unable to find any RFC proposing their inclusion.

Rendered

[programmerjake]

Suggested change

	These features are all very similar, are effectively mutually exclusive (e.g., mcount and fentry).
	These features are all very similar, and are effectively mutually exclusive (e.g., mcount and fentry).

[programmerjake]

maybe there should be #[instrument_fn = "off"] or similar to opt out of all function instrumentation, not just entry/exit (you mentioned loop instrumentation earlier), that would be useful when implementing the instrumentation function to avoid recursively calling itself.

[pmur]

For usability, that seems nice for most common cases. I'll add it.

As for ignore-loops, it is an xray option to modify its heuristics for deciding to whether to instrument the function. I wonder if there is a case where you want to instrument more than *logues, but less than every basic block.

[Darksonn]

How does this related to RFC patchable-function-entry by @maurer ?

[pmur]

For linux ftrace, both provide similar features. Though, x86-64 seems to support both, and some distros use both. I am curious why.

For using existing instrumentation frameworks like gprof or xray, there are more expectations beyond inserting and tracking nops at entry.

[pmur]

I've also created a draft PR for implementation, rust-lang/rust#153742.

[nikic]

I think the RFC needs some more clarity on why we want/need to support all of these instrumentation options. In particular, for mcount I'm not sure we should be moving towards stabilization, rather than moving towards removal.

The RFC mentions two potential use cases, one being gprof (which, I think, has been pretty much completely superseded by other profiling tools nowadays) and the other kernel ftrace. Ftrace also supports using patchable function entries instead, and as far as I know, that is the generally preferred mechanism. I think some legacy architectures (most of which rustc doesn't even support) use mcount because patchable function entry is not implemented for them -- but that's probably better addressed by implementing it, if anyone actually cares.

One thing I'm particularly concerned about is exposing something that has very fragmented platform support. E.g. LLVM currently only supports -mrecord-mcount and -mnop-mcount (here as -Z instrument-mcount-opts=record and -Z instrument-mcount-opts=no-call) on the s390x architecture, and their functionality is essentially a subset of what -Z patchable-function-entry provides (on more architectures).

If the kernel has sound technical reasons for why it uses it uses -pg -mrecord-mcount -mnop-mcount on one platform and -fpatchable-function-entry on another, we should find out what those are and document them. If it doesn't (that is, the reason is historical) then I think we should encourage the kernel to consolidate their instrumentation mechanism, rather than replicating this patchwork approach in Rust.

[Darksonn]

Linux Kernel still uses mcount for ftrace on x86_64.

[nikic]

Linux Kernel still uses mcount for ftrace on x86_64.

Does it use mcount or fentry? I thought it was the latter.

[Darksonn]

Hmm, as I read the makefile, it passes -mrecord-mcount and -mfentry. But it doesn't use patchable function entry, like it does on arm64.

[pmur]

Does it use mcount or fentry? I thought it was the latter.

Both, the functionality intertwined. I believe the usage looks something like -pg -fentry -mnop-mcount -mrecord-mcount for dynamic ftrace on x86_64.

I also learned that -pg on the x86_64*windows*gnu gcc toolchains also uses fentry. glibc and cywin implement both counting functions on x86.

I think there is still utility for these counting functions where there is no operating system, or a robust sampling profiler is not available.

[pmur]

I sent a mail to lkml seeking answers and advice to some of the open questions. fentry seems like it will only ever be suitable for architectures like s390 and x86. Though, I haven't received any feedback from maintainers.

I wonder if adding an annotation to allow overriding the default section where patchable-function-entries stores records would be sufficient to replace the usage of fentry.

[nikomatsakis]

Speaking personally (not on behalf of a team), I support the goal that Rust can do integrate with anything a C compiler can integrate with and that makes me inclined in favor of this RFC, particularly since I think the RfL folks are interested in it. I don't know enough about this area to judge if this is the right set of things. Having a specific list of integrations feels a bit special-case to me but maybe it makes sense.

Longer term, I want the Rust compiler to be more extensible, so that this kind of use-cases could be supported as a library, but that's neither here nor there (I wouldn't block on it). 😁

[nikomatsakis]

One other note:

The RFC should not be talking about -Z. -Z represents inherently unstable surface area, so you would never need it in an RFC. I would expect the RFC to proposal stable compiler options.

(We often start with a -Z flag and then migrate to a stable flag later, but I think that's not really the ideal pattern anyway, this is what -Zunstable-options is for.)

[joshtriplett]

This is certainly well-motivated, and many users want this, including Rust-for-Linux. @rust-lang/lang felt like most of this is more a compiler matter than a lang one, and the lang surface area is somewhat minimally exposing that (via an attribute on functions).

We may not necessarily want to expose, in a lang attribute, the ability to choose on a function-by-function basis which instrumentation you're using. We had the general feeling, when discussing this, that we'd expect the compiler flags to choose a supported instrumentation framework, and then the lang attribute should just be toggling such instrumentation. For instance, "this function is not instrumented", so that it doesn't create recursion because it's called from the instrumentation internals, and possibly also a mechanism for "this function is instrumented".

[joshtriplett]

Speaking for myself: I also think we should ask the question of whether we want to support all of these different instrumentation mechanisms, or if we want to support a subset of them. Ultimately, I think that's a question of what the compiler wants to support, and the lang surface area shouldn't expose which one is in use. But I'd be interested to know if the prospective users of which (e.g. Rust-for-Linux) need support for all of them or just a subset.

[pmur]

I think RfL is only interested in the narrow slice required to support fentry/ftrace for x86, and the related ability to disable it per function. Most (all?) the other architectures are exclusively using patchable-function-entries; they don't have to contend with the security mitigations which complicate x86.

All the interesting bits of this RFC are implemented as experimental features now. fentry support is bolted onto -Zinstrument-mcount. I think the only question this RFC asks is whether these frameworks should be wrapped into a single option.

Now, I am not sure if there is much value in consolidating the options. I think stabilizing mcount and the language attribute are the only useful parts. Is there still interest in xray instrumentation?