feat(stapsdt): Allow compiler to choose argument registers

[aapoalas]

While DTrace uses the function call ABI and a list of types, STAPSDT probes define their arguments with a list of register name and type combinations. This means that the registers can be left for the compiler to choose. When I originally implemented the STAPSDT probes, I didn't realise that it was possible to do this using the in(reg) argument format in the inline assembly block.

This PR allows the compiler to choose the register it prefers for STAPSDT parameter passing. This should also work towards #490. This necessitated changing the validity test to not expect exact registers; it's not beautiful but it does pass the CI.

As a future note, STAPSDT probes can also have pointer read instructions, including "load effective address" like ones, so in the future if Rust brings in support for these (Rust for Linux wants them so they'll probably appear eventually), then the feature can be taken into use here as well. This would mean that some arguments might be read from the stack.

[ahl]

I apologize, but I wasn't able to follow this change. I think it would be helpful to explain the goals and to provide examples of the changes to output.

[ahl]

this is necessary due to some change you're making to the value of in_regs?

[aapoalas]

Yup: the STAPSDT ELF note's argument format expects AT&T syntax. I'll make a more explicit note of it.

[ahl]

I still don't get it.

[aapoalas]

Okay; I added a larger prose section trying to really drill down to what is going on here with the att_syntax choice and why it is made. Can you check if that works to explain the thing?

[aapoalas]

I apologize, but I wasn't able to follow this change. I think it would be helpful to explain the goals and to provide examples of the changes to output.

My apologies, failure of my communication :(

Okay, so currently for a probe fn my_probe(a: u8, b: u32) the crate will, in both DTrace and STAPSDT backends, generate a similar kind of probe that passes a single byte in the first argument register (rdi/x0) and a 4 byte value in the second argument register (rsi/x1). Inside the isenabled probe's "conditional block", the compiler will thus need to make sure that the two arguments are indeed in the these registers, and will generate the code to move them into these registers if they were not there already.

With this PR, the choice of registers in the STAPSDT case is instead left to the compiler: if a happens to be in the BH byte-register for whatever reason, then the compiler can choose to assign that register for the register. In the asm! macro we enable this by using the following kind of argument format:

asm!(
  "/* {name} */",
  name = in(reg) arg
);

This effectively says "take argument arg and store it in a register, any register, and call that register name: when I refer to name, fill in the name of that register". So, in the ELF note we do not explicitly say which register we're passing an argument in, we instead give the assigned name of the argument (which we set to shadow the original argument name) and let the compiler take care of figuring out which concrete register the data happens to be in or happens to be free for use at the probe site.

Does this explain it any better?

[ahl]

Does this explain it any better?

I think so--it's just very foreign to me! So--unlike DTrace--it sounds like stap/eBPF will figure out where arguments land based on ELF data; is that right?

[ahl]

good progress; thanks

[ahl]

Suggested change

	(
	// For STAPSDT probes, we cannot "widen" the data type at the
	// interface, as we've left the register choice up to the
	// compiler and the compiler doesn't like mismatched register
	// classes and types for single-byte values (reg_byte vs usize).
	#[cfg(usdt_backend_stapsdt)]
	quote! { (*<_ as ::std::borrow::Borrow<#ty>>::borrow(&#input)) },
	#[cfg(not(usdt_backend_stapsdt))]
	quote! { (*<_ as ::std::borrow::Borrow<#ty>>::borrow(&#input) as usize) },
	#[cfg(not(usdt_backend_stapsdt))]
	let value quote! { (*<_ as ::std::borrow::Borrow<#ty>>::borrow(&#input) as usize) };
	// For STAPSDT probes, we cannot "widen" the data type at the
	// interface, as we've left the register choice up to the
	// compiler and the compiler doesn't like mismatched register
	// classes and types for single-byte values (reg_byte vs usize).
	#[cfg(usdt_backend_stapsdt)]
	let value = quote! { (*<_ as ::std::borrow::Borrow<#ty>>::borrow(&#input)) };
	(
	value

I'd suggest reflowing to something like this to make it a little easier to parse and to separate out the stap specifics more cleanly. In general I don't love having stap-specifics in common but ... meh

[aapoalas]

Would you prefer I copy out the necessary code after all? I definitely agree that the stap-specific code in common is ugly as sin.

[ahl]

I also don't think we should have a mostly the same copy; I could imagine refactoring. Perhaps @bnaecker could weight in on structure

[bnaecker]

I agree that having many cfgs in this common module seems messy. My goal is generally to keep those very narrowly-scoped (the target-arch bits) or move them to modules. For example, you could make the asm_type_convert() function backend-specific, which I think would clean a lot of this up.

[ahl]

I still don't get it.

[aapoalas]

Does this explain it any better?

I think so--it's just very foreign to me! So--unlike DTrace--it sounds like stap/eBPF will figure out where arguments land based on ELF data; is that right?

Yup! They read the "arguments" string from the ELF header and parse it as a space-separated list of {sizeAndType}@{location} arguments where location is a GNU Assembler syntax operand expression (it can be an immediate, a register, or a memory operand). That location then tells where the argument should be read from.