[PATCH] D158725: [NFC][AMDGPU] Guard the custom fixups kind array from invalid access

[Georgi Mirazchiyski via Phabricator via llvm-commits]

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