[COFF][ARM] Clear the J1 and J2 bits when applying relocations to 24 bit branches

[mstorsjo]

Summary

The opcode for the bl branches can initially be F000 F800, i.e.
the J1 and J2 bits are already set. Therefore mask these bits out
before or'ing in the new bits.

[peter.smith]

Hello Martin,

I'm away on holiday with limited internet and no access to a computer, so I can't check the details till I get back at some point next week. I suggest adding the code-owner for the COFF part of lld, Rui Ueyama as you'd likely need to get some input from him on the coding style.

[ruiu]

Adding Saleem as a reviewer because he knows more about ARM than me.

[compnerd]

COFF/Chunks.cpp

107

Yeah, the bits need to be cleared out as the value may be altered by the new computation.

There is no guarantee that the bits are set IIRC, so I find the comment misleading. You can add a comment along the lines of clear out the J1 and J2 bits which may be set, though, Im not sure if it really adds much value.

[mstorsjo]

COFF/Chunks.cpp

107

Ok, uploading a new version of the patch with the comment changed.

[mstorsjo]

Updated the comment and the summary text

[compnerd]

SVN r277836

COFF/Chunks.cpp

static void applyBranch20T(uint8_t *Off, int32_t V) {
   or16(Off + 2, (J1 << 13) | (J2 << 11) | ((V >> 1) & 0x7ff));
 }
 
 static void applyBranch24T(uint8_t *Off, int32_t V) {
   uint32_t S = V < 0 ? 1 : 0;
   uint32_t J1 = ((~V >> 23) & 1) ^ S;
   uint32_t J2 = ((~V >> 22) & 1) ^ S;
   or16(Off, (S << 10) | ((V >> 12) & 0x3ff));
-  or16(Off + 2, (J1 << 13) | (J2 << 11) | ((V >> 1) & 0x7ff));
+  // Clear out the J1 and J2 bits which may be set.
+  write16le(Off + 2, (read16le(Off + 2) & 0xd000) | (J1 << 13) | (J2 << 11) | ((V >> 1) & 0x7ff));
 }
 
 void SectionChunk::applyRelARM(uint8_t *Off, uint16_t Type, Defined *Sym,
                                uint64_t P) const {
   uint64_t S = Sym->getRVA();
   // Pointer to thumb code must have the LSB set.
   if (Sym->isExecutable())
     S |= 1;

test/COFF/reloc-arm.test

 # RUN: yaml2obj < %s > %t.obj
 # RUN: lld-link /out:%t.exe /entry:main %t.obj
 # RUN: llvm-objdump -s %t.exe | FileCheck %s
 
 # CHECK: .text:
 # CHECK: 402000 01104000 00000000 00000000 00000000
 # CHECK: 402010 01100000 00000000 00000000 00000000
 # CHECK: 402020 01000100 00004000 00000000 00000000
 # CHECK: 402030 fe07e62f 00000000 00000000 00000000
 # CHECK: 402040 3e04de2f 00000000 00000000 00000000
 # CHECK: 402050 fe07d62f 00000000 00000000 00000000
-# CHECK: 402060 00000000 00000000 00000000 00000000
+# CHECK: 402060 fef0cef7 00000000 00000000 00000000
 
 --- !COFF
 header:
   Machine:         IMAGE_FILE_MACHINE_ARMNT
   Characteristics: []
 sections:
   - Name:            .aaa
     Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_PURGEABLE, IMAGE_SCN_MEM_16BIT, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
     Alignment:       4096
     SectionData:     0000000000000000
   - Name:            .text
     Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_PURGEABLE, IMAGE_SCN_MEM_16BIT, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
     Alignment:       4096
-    SectionData:     00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
+    SectionData:     00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000f000f8000000000000000000000000
     Relocations:
       - VirtualAddress:  0
         SymbolName:      foo
         Type:            1  # IMAGE_REL_ARM_ADDR32
       - VirtualAddress:  16
         SymbolName:      foo
         Type:            2  # IMAGE_REL_ARM_ADDR32NB
       - VirtualAddress:  32
         SymbolName:      foo
         Type:            17  # IMAGE_REL_AMD64_MOV32T
       - VirtualAddress:  48
         SymbolName:      foo
         Type:            20  # IMAGE_REL_AMD64_BRANCH24T
       - VirtualAddress:  64
         SymbolName:      foo
         Type:            18  # IMAGE_REL_ARM_BRANCH20T
       - VirtualAddress:  80
         SymbolName:      foo
         Type:            21  # IMAGE_REL_AMD64_BLX23T
+      - VirtualAddress:  96
+        SymbolName:      bar
+        Type:            20  # IMAGE_REL_ARM_BRANCH24T
 symbols:
   - Name:            .aaa
     Value:           0
     SectionNumber:   1
     SimpleType:      IMAGE_SYM_TYPE_NULL
     ComplexType:     IMAGE_SYM_DTYPE_NULL
     StorageClass:    IMAGE_SYM_CLASS_STATIC
   - Name:            .text
- Name:            main
     ComplexType:     IMAGE_SYM_DTYPE_NULL
     StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
   - Name:            foo
     Value:           0
     SectionNumber:   1
     SimpleType:      IMAGE_SYM_TYPE_NULL
     ComplexType:     IMAGE_SYM_DTYPE_NULL
     StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+  - Name:            bar
+    Value:           0x500000
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
 ...