[PATCH] D156497: [BPF] Emit UNDEF rather than constant 0

[Tamir Duberstein via Phabricator via llvm-commits]

tamird added a comment.

In D156497#4599845 <https://reviews.llvm.org/D156497#4599845>, @yonghong-song wrote:

> Thanks @eddyz87 for a detailed example. The following is my understanding.
>
> - In any case, there is a compilation error. The binary should not be used since it is invalid with large chance.
> - The continual optimization will be done with either existing code or this patch (Constant 0 -> UNDEF)
> - For existing code, for *extra* parameters, the compilation will proceed with those extra parameters as constant 0.
> - With this patch, for *extra* parameters, the compilation will proceed with those extra parameters as undefined.
> - 'constant 0' may simplify the code and largely preserve the code flow.
> - 'undefined' may also simplify the code as compiler may discard some codes involved with 'undefined' values.
>
> I think either 'constant 0' or 'undefined' func proto argument should not impact subsequent compilation warnings.
> The only possible difference is generated code and with 'constant 0' is surely more close to the original code.
> Considering the buggy binary should not be really used by verifier, I think staying with existing 'constant 0'
> seems more reasonable, unless a more stronger argument favors 'undef' approach.

Perhaps we should draw inspiration from libbpf's behavior when it is unable to relocate: https://github.com/torvalds/linux/blob/d4ddefee5160dc477d0e30c9d7a10ce8861c3007/tools/lib/bpf/relo_core.c#L982

Rather than UNDEF or constant zero, we could use some marker values:

- when the function reads arguments from the stack: `0xbad4265` "bad args"
- when the function return is larger than a register: `0xbad2375` "bad rets"

WDYT?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156497/new/

https://reviews.llvm.org/D156497