rnk added a comment.
I think the disagreement here highlights the need to have a serious discussion
about the future of error handling across the LLVM project. As you say, it
sounds like you're not going to reach agreement on this code review, so maybe
the best short term next step is to land the uncontroversial changes that Aaron
agrees with.
Regarding error handling and the wide usage of assertions to guard UB across
LLVM, we need to decide what our goals are as a community. Is it actually the
goal of the project that Clang and LLVM that no input can lead to UB? If we
can't guarantee that, is there some error budget we consider acceptable (fuzzer
runs for 24hrs and can't find bugs)? How does that goal rate against our other
goals, like performance? We could just ship with assertions enabled, sacrifice
20% code size and performance, and call it a day. We used to do that for
Chromium, but users complained that compiles were too slow and we stopped doing
it.
I think the status quo has real problems. We pretend that we can do both of
these:
- Assert liberally, with the understanding that assertion failures lead to UB
(failed bad cast check, bounds checks, unreachable code, etc)
- We can actually find and fix all cases that violate those inputs to the point
that clang is stable and secure enough for our satisfaction
Currently, it is really easy to run fuzzers and find crash bugs in clang. I
think the lesson we should take from that is that we are compromising goal 2
here, and we shouldn't kid ourselves about it.
Maybe the goal is not security, but is instead something about user or
developer experience, but we should go through some higher level process to
clarify that goal so we can write it down and agree on it.
================
Comment at: clang/lib/Analysis/CFG.cpp:1047
+ llvm_unreachable("Unexpected unary operator!");
return llvm::None;
}
----------------
This will create unreachable code warnings, which must be addressed before
landing.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135551/new/
https://reviews.llvm.org/D135551
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
