Provide the offset of the instruction data to the VM's r2 register

[buffalojoec]

Problem

The current serialization layout of the VM's input region makes it inefficient for sBPF programs to access the serialized instruction data directly. This stems from the need to parse the entire accounts section in order to locate the instruction data.

When using default entrypoints like the SDK's entrypoint! the input region is fully deserialized into the canonical &Pubkey, &[AccountInfo] and &[u8] parameters. Optimized alternatives, such as Pinocchio's lazy_program_entrypoint! allow skipping most of this deserialization if the program doesn’t need all accounts.

However, even with Pinocchio's optimized entrypoint, it's not possible to access the instruction data without first walking over the entire accounts section to calculate the offset of the instruction data.

Suggested Change

A number of contributors have suggested providing the offset of the serialized instruction data to the VM's r2 register.

Programs currently accept a pointer to the beginning of the input region, which is the number of accounts and also the beginning of the accounts section.

pub unsafe extern "C" fn entrypoint(
    input: *mut u8,
) -> u64

With the additional r2 argument, entrypoints can look like this:

pub unsafe extern "C" fn entrypoint(
    input: *mut u8,
    instruction_data_offset: u64,
) -> u64

The input region pointer is provided as a pointer in the r1 register. The instruction data offset would be provided as a u64 offset to r2, not a pointer.

We can provide the new additional parameter to r2 in two ways:

• Globally, to all programs, provide the offset in r2
• Conditionally, if requested, provide the offset in r2

ABI v2

The new serialization layout presented in SIMD-0177 (Program Runtime ABI v2) also solves the aforementioned inefficiency.

Once a program has been updated to use the new ABI layout proposed in SIMD-0177, they would have no use for the offset in r2 anymore. Hence, this is one main reason we are hesitant to add the change.

---

Example implementation:

https://github.com/buffalojoec/solana/blob/4bd1624e7b64a45d4802fbed673543a11d9fc6b3/programs/bpf_loader/src/lib.rs#L1652-L1658
https://github.com/buffalojoec/solana-sdk/blob/3a881e89a783f8cf1a102de90e294b88c5f1ba82/program-entrypoint/src/lib.rs#L206-L229
https://github.com/anza-xyz/mollusk/blob/r2-input-data-offset/test-programs/register-metadata/src/lib.rs

[deanmlittle]

If we start building things around this pattern, if we just push the pointer of ABIv2 into r2, existing Rust programs will inherit compatibility with ABIv2 by default. We can also just not embark on multi-year-long wastes of time that take the chain down all the time.

[LucasSte]

existing Rust programs will inherit compatibility with ABIv2 by default

This is not true, because the underlying data layout is different between versions, not only the arguments passed to entry point.

[cavemanloverboy]

i was told to come here to voice my support for this change.

I am in favor of doing this, but with setting r2 to the offset corresponding to the start of the ix data, not the ix data length. This is because those of us that still respect the compiler actually use e.g. rust slices which require this pointer. pointing at the ix data len would allow us to do something like

let ix_data = unsafe { core::slice::from_raw_parts(r2, r2.sub(8).cast::<usize>().load()) };

which would compile to something like

ldxdw r4, [r2 - 8]
// r2 is already set to ix data

instead of having to do

ldxdw r4, [r2 + 0]
add64 r2, 8

before we can construct a slice.

Additionally, any static offsets are easier to read in the assembly, e.g.

// let ix_data = unsafe { core::slice::from_raw_parts(r2, r2.sub(8).cast::<usize>().load()) };
let disc = ix_data[0];
let some_u64_payload = unsafe { ix_data.add(1).cast::<u64>().read_unaligned() };

compile to

ldxb r?, [r2 + 0],
ldxdw r?, [r2 + 1],

instead of

ldxb r?, [r2 + 8],
ldxdw r?, [r2 + 9],

[febo]

I am in favor of doing this, but with setting r2 to the offset corresponding to the start of the ix data, not the ix data length.

Agree with this – we can provide a "friendlier" API that parses the instruction data length and the slice for the data if needed.

[LucasSte]

This consideration does not take into account the proper r2 usage. r2 is used as a function call argument, so whenever you call another function, you'll kill the r2 definition. What you posted here is only valid if the program has only the entry point function and does not call any other function afterwards.

Moreover, in an entrypoint with enough operations, even without any other call, you'll likely end up having to spill the r2 register to make space for other variables.

I'm not confident the gains will be noticeable, given that the change in the pointer appears an anti-pattern to me. In almost all cases, you'll need to read the length first, before the data so that you can validate that the next offset is not going to contain garbage or result in an out-of-bounds read.

[cavemanloverboy]

this spillage would occur regardless of whether r2 is set and what it is set to. when writing small programs, a lot of us optimizooors inline everything and do everything with a single stack frame

[LucasSte]

do everything with a single stack frame

Do not forget that syscalls will also use r2.

The inline hints used in C and Rust does not guarantee that functions will be inlined. The compiler counts how many call sites a function has and the inlining impact on code size before doing the inline.

Also, a slice in Rust is a data structure consisting of the pointer to the data and the length. You'll only see the length in one register and the pointer in another one when the compiler determines it is worth to break down the data structure into its member parts (this is called scalar replacement of aggregates).

I believe the optimization you're suggesting is only valid for writing programs with assembly. Since you'll need most of the times to read the length first to validate your data and even avoid an out of bounds read, having the pointer to the length is more straightforward. The design we are proposing must incentivize the correct management of memory regions and pointers. My concern about pointing to the data and not to the length is that this is an anti-pattern choice.

[stegaBOB]

I like this change a lot, assuming it doesn't break existing programs! Very simple to understand and seems like it's effectively already been implemented.

[buffalojoec]

I've enabled a non-voting node on mb with all four GPRs (r2, r3, r4, r5) occupied to see who breaks.
https://github.com/buffalojoec/solana/blob/9471543d64f15b7c5674b7e7e5381abff9d4fb39/programs/bpf_loader/src/lib.rs#L1650-L1656

As for the specific implementation, the offset being the data, not the length, works for me.

[cavemanloverboy]

could do rand::random() for each of these to fuzz a bit :P

[buffalojoec]

Good idea, done: buffalojoec/solana@4c853c8

[Aursen]

I'm in favor of this change too, so I'm pushing for it. But I prefer the approach of @cavemanloverboy

[buffalojoec]

OK, mb node is healthy and I'll continue to monitor it. In the meantime, the SIMD is up, with @cavemanloverboy's implementation request mentioned above.

#321