[PATCH] D50998: [LLD] [COFF] Check the instructions in ARM MOV32T relocations

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added a comment.

In https://reviews.llvm.org/D50998#1207297, @peter.smith wrote:

> For what it's worth I've checked that the bit-patterns used in the MOVT/MOVW instructions are correct. In assembler it seems like it is possible to create an error that isn't detectable at link time.
>
>       .global main
>       .global variable
>       .global variable2
>       .text
>       .thumb
>   main:
>       movw r0, :lower16:variable
>       movt r0, :upper16:variable2
>       ldr  r0, [r0]
>       bx   lr
>       .data
>   variable:
>    .long 42
>
>
> The assembler doesn't detect the different relocation targets and just emits a  0x0 IMAGE_REL_ARM_MOV32T variable relocation. It would be much better if this were caught at assembly time. I don't have any great ideas of how to solve it in MC though as that typically only presents Targets with one fixup at a time.


Indeed, it's possible to craft other malicious cases that this won't catch - but it would at least catch most other bugs where the ARM target unintentionally produces something broken.

On the LLVM level, this is handled as the movw fixup translating into `IMAGE_REL_ARM_MOV32T` while the movt fixup just gets dropped; see lib/Target/ARM/MCTargetDesc/ARMWinCOFFObjectWriter.cpp:

  unsigned ARMWinCOFFObjectWriter::getRelocType(MCContext &Ctx,
                                                const MCValue &Target,
                                                const MCFixup &Fixup,
                                                bool IsCrossSection,
                                                const MCAsmBackend &MAB) const {
    switch (static_cast<unsigned>(Fixup.getKind())) {
    [snip]
    case ARM::fixup_t2_movw_lo16:
    case ARM::fixup_t2_movt_hi16:
      return COFF::IMAGE_REL_ARM_MOV32T;
    }
  }
  
  bool ARMWinCOFFObjectWriter::recordRelocation(const MCFixup &Fixup) const {
    return static_cast<unsigned>(Fixup.getKind()) != ARM::fixup_t2_movt_hi16;
  }

So past this stage, the information about the movt instruction's intent is lost.

As for the actual bug that I was able to catch with this error, I'll post the patch now.


https://reviews.llvm.org/D50998