[PATCH] D158725: [NFC][AMDGPU] Guard the custom fixups kind array from invalid access
Georgi Mirazchiyski via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 24 07:07:04 PDT 2023
gmirazchiyski added inline comments.
================
Comment at: llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUAsmBackend.cpp:188
+ assert(unsigned(Kind - FirstTargetFixupKind) < getNumFixupKinds() &&
+ "Invalid kind!");
----------------
arsenm wrote:
> Isn’t this equivalent to the previous bounds check?
Thank you for checking this @arsenm ! I don't think it is or at least see it as exactly the same check.
Kind < 128 (FirstTargetFixupKind) -> returns FK_NONE which I think it's no-op here
Kind >= 256 (FirstLiteralRelocationKind) -> returns the base getFixupKindInfo(Kind)
Those bounds checks are fair and needed, but for anything in the range of [128;255], Kind is allowed, and anything above 128 will be invalid.
Yes, obviously only the AMDGPU::fixup_si_sopp_br fixup kind which is valid will be used for AMDGPU but the assert is here as a security guard to the Infos array in case of misuse.
I can agree to the point this can do and is safe without the assert, but from the outside it looks like a security concern, which we will only know it will not occur after we dig into the logic.
Moreover, other backends seem to similarly assert too.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D158725/new/
https://reviews.llvm.org/D158725
More information about the llvm-commits
mailing list