[PATCH] D33574: PPC: Verify that branch fixups fit within the range.

Hal Finkel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 23 05:29:21 PDT 2017


hfinkel added a comment.

In https://reviews.llvm.org/D33574#849770, @asb wrote:

> Hi folks, I think llvm_unreachable is really the wrong error handling strategy here. It _won't_ necessarily cause the process to exit on a release build. This is actually one of the cases where it is possible to report a nice error without too much hassle. In https://reviews.llvm.org/rL299529 I added an MCContext parameter to MCAsmBackend::applyFixup in order to make it easy to use MCContext::reportError for effort reporting in helper functions like adjustFixupValue. Due to later upstream changes, you now get hold of your MCContext via the MCAssembler argument. You can see an example of this approach in action in https://reviews.llvm.org/D23568.


I think that the test here is: is this something that a user could hit, (including with inline assembly, etc.), or does this indicate an internal problem only? For internal errors, we have lots of checks that we optimize out for release builds (and there's always the option of building release+asserts, and I deploy those builds on more experimental platforms as a general rule). If it is something that a user could hit, then it should be an error that the user will always get.



================
Comment at: lib/Target/PowerPC/MCTargetDesc/PPCAsmBackend.cpp:44
   case PPC::fixup_ppc_brcond14abs:
+    if(Value > 0xffff)
+      llvm_unreachable("Cond branch absolute target overflow.");
----------------
Space after if.


Repository:
  rL LLVM

https://reviews.llvm.org/D33574





More information about the llvm-commits mailing list