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

Thu Aug 17 18:36:06 PDT 2023

tamird added a comment.

In D156497#4597470 <https://reviews.llvm.org/D156497#4597470>, @ast wrote:

> Sorry for the delay. I've been on PTO.
> The change looks fine, but it's not clear to me what code will be generated with UNDEF. 
> Before the change it would be 'r0=0; exit' and now just 'exit' ?
> Meaning that r0 will be left in whatever state that previous code did.
> So instead of generating consistent return 0 the user would have to debug random return values ?
> The chance of failing the verifier is slim. The functions are rarely completely empty.
> r0 is likely used in the earlier code and the verifier won't complain that it's not initialized.
> So the change contradicts the intent to remove the footgun.

When I first wrote this patch, I tested it on real code and did see verifier errors. It's a deep stack to unwind to reconstruct the state of the world back then to be able to quote what those errors were and what the generated code looked like. I can do it, but I'd like us to converge on what the right thing to do here is.

It seems you now agree that there's a footgun here. I'm not married to the use of UNDEF, is there a better solution you can think of that would reliably produce errors? Would reconstructing the generated code and verifier errors from a month ago be useful?

Lastly: @yonghong-song suggested that the tests be removed. In light of your comments, should we restore the tests so that we can make claims about the behavior in the presence of more complicated functions?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156497