[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