[PATCH] D149711: [PowerPC] Remove asserts from the disassembler.

Nemanja Ivanovic via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 18 07:54:50 PDT 2023


nemanjai added a comment.

Please note that this assert can trip even when disassembling with `--disassemble` and not just `-disassemble-all`. Namely, the offending word can appear in the text section (for example, the AIX traceback table, etc.).
This is very similar to https://reviews.llvm.org/rGc86f8d4276ae



================
Comment at: llvm/lib/Target/PowerPC/Disassembler/PPCDisassembler.cpp:118-119
                                             const MCDisassembler *Decoder) {
-  assert(RegNo <= 30 && "Expecting a register number no more than 30.");
-  assert((RegNo & 1) == 0 && "Expecting an even register number.");
   return decodeRegisterClass(Inst, RegNo >> 1, FpRegs);
----------------
barannikov88 wrote:
> shchenz wrote:
> > Little confused. From the function name and its parameter seems like this function should only be called for instructions. Is it possible that there is a wrong call to this function? For example, for data, we should not decode fpr RC as there is no RC for data at all?
> The removed assertion should have been changed to `if (...) return DecodeStatus::Fail;` so that random data is not decoded into invalid instructions.
> The added test should be a negative one.
> 
Lets not just remove these. Please change it to something like:
```
  if (RegNo > 30 || (RegNo & 1))
    return MCDisassembler::Fail;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149711



More information about the llvm-commits mailing list