Verify that bpf2bpf updates r10

[Alan-Jowett]

This pull request includes changes to the tests/call_local.data file to ensure that the stack is preserved during function calls. The changes involve writing to the stack before a function call and reading back the values after the call to check that they were not overwritten by the function.

Stack preservation checks:

• tests/call_local.data: Added lines to write to the stack before calling local func1 and read back the values after the call. This ensures that the values are preserved and not overwritten by func1.
• tests/call_local.data: Added lines to write to the stack in stdw [%r10-4], 0xCCBBAA99. This ensures that the stack writing operation does not overwrite the callee's values.

[coveralls]

coverage: 95.339%. remained the same when pulling 34dffe26f31b9e12f0b95d41c0e0b24f67a7e6a3 on improve_local_call_test into 3e528b41a4f5131bda00d253e8af8ce57c5a3e07 on main.

[dthaler]

I don't think this PR should be merged as such as this is not an ISA requirement and this is (currently) an ISA conformance suite.

See discussion in https://github.com/microsoft/ebpf-for-windows/pull/3506#issuecomment-2105926397

[Alan-Jowett]

That makes sense.

[Alan-Jowett]

@dthaler I don't agree.

My position: Just like when making calls to helper functions, when making bpf2bpf calls the assumption is that the local variables and the non-volatile registers are preserved. How exactly that is done is implementation defined, but [r10-8] in the callee and [r10-8] in the caller must either point to different locations or the run time must save the stack state and restore it.

Possible implementations: 1) r10 is adjusted by size of locals in outer. 2) r10 is adjusted to point to a brand new BPF stack. 3) The runtime saves the values stored in [r10-8] somewhere else and restored it after the call returns.

Assume the following program:

inner:
stdw [%r10-8], 2
exit
outer:
stdw [%r10-8], 1
call local inner
ldxw %r0, [%r10-8]
exit

What value should outer return? 1 or 2? I don't see that as being related to the Linux psABI, but rather a core issue of BPF ISA conformance.

[dthaler]

@dthaler I don't agree.

My position: Just like when making calls to helper functions, when making bpf2bpf calls the assumption is that the local variables and the non-volatile registers are preserved. How exactly that is done is implementation defined, but [r10-8] in the callee and [r10-8] in the caller must either point to different locations or the run time must save the stack state and restore it.

Possible implementations:

1. r10 is adjusted by size of locals in outer.
2. r10 is adjusted to point to a brand new BPF stack.
3. The runtime saves the values stored in [r10-8] somewhere else and restored it after the call returns.

Assume the following program:

inner:
stdw [%r10-8], 2
exit
outer:
stdw [%r10-8], 1
call local inner
ldxw %r0, [%r10-8]
exit

What value should outer return? 1 or 2? I don't see that as being related to the Linux psABI, but rather a core issue of BPF ISA conformance.

r10 is not defined as a stack pointer in the ISA. It is defined as a stack pointer in the psABI.

So what should the following return?

inner:
stdw [%r1-8], 2
exit
outer:
stdw [%r1-8], 1
call local inner
ldxw %r0, [%r1-8]
exit

[Alan-Jowett]

Thanks. I didn't realize the BPF ISA doesn't reference stack at all. So should we remove all test that assume R10 is stack address?

[Alan-Jowett]

OK, I understand your point. Will close this PR.

[dthaler]

Thanks. I didn't realize the BPF ISA doesn't reference stack at all. So should we remove all test that assume R10 is stack address?

It's a good question. Right now my opinion is to keep tests for both the ISA and the Linux psABI but put the test files in separate directories and allow the conformance runner to be told which directory to use. That is, I expect tests against the psABI will be interesting once better documented, and tests that assume R10 is a stack address will be important there.

[dthaler]

BTW the very beginnings of a psABI document is at https://www.kernel.org/doc./html/latest/bpf/standardization/abi.html Clearly most of it is missing.

As shown at https://datatracker.ietf.org/wg/bpf/about/ (fourth bullet), it should be published as an IETF RFC eventually, though as Informational [I] not a Proposed Standard [PS].