[PATCH] D156497: [BPF] report fatal error rather than miscompile

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/BPF/BPFISelLowering.cpp:872
+  if (HasError) {
+    report_fatal_error("failed to generate BPF instructions, see diagnostics");
+  }
----------------
arsenm wrote:
> arsenm wrote:
> > tamird wrote:
> > > arsenm wrote:
> > > > Just let the proper context error happen, this is just worse
> > > Please read the description. The problem is that without fataling, the embedder can use the miscompiled code (and we saw this because our diagnostic handler did not treat error as fatal).
> > Then your compiler driver / embedder / whatever is broken and needs to check the context. TargetLowering must not be stateful and you should not emit a fatal error here
> check the context for errors
emit proper context error is ideal for user facing errors, you just need to ensure you emit some kind of valid output. report_fatal_error can be OK for cases users cannot reasonably reach. deferring to finalizeLowering doesn't make any sense, if you're going to report_fatal_error just do it immediately. doing it here is a middle ground that doesn't make sense


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156497