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

[gmirazchiyski]

Summary

Verify the possibility of accidental passing of an invalid MCFixupKind value which can cause an invalid access in the array, and guard from it via assert.

[arsenm]

llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUAsmBackend.cpp

188

Isn’t this equivalent to the previous bounds check?

[gmirazchiyski]

llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUAsmBackend.cpp

188

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.

[gmirazchiyski]

A friendly ping. Thanks! :)

llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUAsmBackend.cpp

const MCFixupKindInfo &AMDGPUAsmBackend::getFixupKindInfo(
   };
 
   if (Kind >= FirstLiteralRelocationKind)
     return MCAsmBackend::getFixupKindInfo(FK_NONE);
 
   if (Kind < FirstTargetFixupKind)
     return MCAsmBackend::getFixupKindInfo(Kind);
 
+  assert(unsigned(Kind - FirstTargetFixupKind) < getNumFixupKinds() &&
+         "Invalid kind!");
   return Infos[Kind - FirstTargetFixupKind];
 }
 
 bool AMDGPUAsmBackend::shouldForceRelocation(const MCAssembler &,
                                              const MCFixup &Fixup,
                                              const MCValue &) {
   return Fixup.getKind() >= FirstLiteralRelocationKind;
 }