Re: clang14 issue triggering PR264094?

> On May 22, 2022, at 17:20 , John F Carr <jfc@mit.edu> wrote:
> 
> On May 22, 2022, at 16:41 , tuexen@freebsd.org wrote:
>> 
>> Dear all,
>> 
>> I'm trying to analyze https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=264094
>> 
>> The relevant file is:
>> https://cgit.freebsd.org/src/tree/sys/netinet/cc/cc_htcp.c
>> 
>> It is interesting that the panic happens on arm64, but not amd64. It does
>> happen when using clang14 (most recent version in the main tree), it does
>> not happen when using clang13.
>> I also does not happen using clang14 when forcing htcp_recalc_beta() not
>> to be inlined.
>> 
>> The panic happens when accessing V_htcp_adaptive_backoff in
>> https://cgit.freebsd.org/src/tree/sys/netinet/cc/cc_htcp.c#n471
>> 
>> Since this looks strange to me, I disassembled htcp_recalc_beta() when
>> using clang14 and the function not being inlined. This is the relevant
>> code:
>> 
>> (kgdb) disassemble htcp_recalc_beta
>> Dump of assembler code for function htcp_recalc_beta:
>>  0x00000000000113cc <+0>:	stp	x29, x30, [sp, #-16]!
>>  0x00000000000113d0 <+4>:	mov	x29, sp
>>  0x00000000000113d4 <+8>:	ldr	x8, [x0]          ; x8 = ccv
>>  0x00000000000113d8 <+12>:	ldr	x9, [x18]         ; x9 = curthread
>>  0x00000000000113dc <+16>:	adrp	x10, 0x21000      ; x10 = ???
>>  0x00000000000113e0 <+20>:	ldr	x9, [x9, #1368]   ; x9 = curthread->td_vnet
>>  0x00000000000113e4 <+24>:	ldr	x10, [x10, #2168] ; x10 = ???
>>  0x00000000000113e8 <+28>:	ldr	x9, [x9, #40]     ; x9 = curthread->td_vnet->vnet_data_base
>>  0x00000000000113ec <+32>:	ldr	w9, [x9, x10]     ; w9 = V_htcp_adaptive_backoff ???
>>  0x00000000000113f0 <+36>:	cbz	w9, 0x11428 <htcp_recalc_beta+92>
>> 
>> I don't understand the computations in relation to x10, which is the offset used to get the relevant variable.
>> 
>> However, this code works.
>> 
>> Looking at the code generated by clang13 when htcp_recalc_beta() is inlined, one gets:
>> 
>>  0xffff000150610f28 <+212>:	ldr	x10, [x0]                ; x10 = ccv
>>  0xffff000150610f2c <+216>:	ldr	x11, [x18]               ; x11 = curthread
>>  0xffff000150610f30 <+220>:	ldr	x11, [x11, #1368]        ; x11 = curthread->td_vnet
>>  0xffff000150610f34 <+224>:	ldr	x12, [x11, #40]          ; x12 = curthread->td_vnet->vnet_data_base
>>  0xffff000150610f38 <+228>:	adrp	x11, 0xffff000150621000  ; ???
>>  0xffff000150610f3c <+232>:	ldr	x11, [x11, #2256]        ; ???
>>  0xffff000150610f40 <+236>:	ldr	w12, [x12, x11]
>>  0xffff000150610f44 <+240>:	cbz	w12, 0xffff000150610f7c <htcp_ack_received+296>
>> 
>> It looks similar and it does work.
>> 
>> Now comes the inlined code from clang14:
>> 
>>  0xffff0001016acf28 <+212>:	ldr	x10, [x0]         ; x10 = ccv
>>  0xffff0001016acf2c <+216>:	ldr	x11, [x18]        ; x11 = curthread
>>  0xffff0001016acf30 <+220>:	ldr	x12, [x11, #1368] ; x12 = curthread->td_vnet
>>  0xffff0001016acf34 <+224>:	nop
>>  0xffff0001016acf38 <+228>:	adr	x11, 0xffff0001016bd520 <vnet_entry_htcp_adaptive_backoff>
>>  0xffff0001016acf3c <+232>:	ldr	x12, [x12, #40]   ; x12 = curthread->td_vnet->vnet_data_base
>> ==>0xffff0001016acf40 <+236>:	ldr	w12, [x12, x11]
>>  0xffff0001016acf44 <+240>:	cbz	w12, 0xffff0001016acf7c <htcp_ack_received+296>
>> 
>> The line marked with ==> is the line where the panic happens. It looks that the offset computation is different.
>> 
>> Is this an issue with clang14? Any idea what is going wrong?
>> 
>> Thanks for any help!
>> 
>> Best regards
>> Michael
>> 
>> 
> 
> That nop next to the adr instruction makes me think a 32 bit relocation went wrong.  These relocations normally consume two instructions but the linker can patch one into a nop if it is not needed.  Usually you have a pair of instructions adrp+adr or, as in the clang13 example, adrp+ld or adrp+st.   The adrp computes a page-aligned address within a 32 bit offset of the PC and the next instruction has low 12 bits of the address.  The problem could be in the compiler or the linker.  What does the assembly or disassembled .o look like before it gets linked?
> 
> 

I have an arm64 running CURRENT so I was able to reproduce the problem.

The assembly code is (cc -S)

        ldr     x10, [x0]
        ldr     x11, [x18]
        ldr     x12, [x11, #1368]
        adrp    x11, :got:vnet_entry_htcp_adaptive_backoff
        ldr     x11, [x11, :got_lo12:vnet_entry_htcp_adaptive_backoff]
        ldr     x12, [x12, #40]
        ldr     w12, [x12, x11]


cc_htcp.o disassembles to (objdump --disassemble --reloc)

 1f4:	f940000a 	ldr	x10, [x0]
 1f8:	f940024b 	ldr	x11, [x18]
 1fc:	f942ad6c 	ldr	x12, [x11, #1368]
 200:	9000000b 	adrp	x11, 0 <htcp_mod_init>
			200: R_AARCH64_ADR_GOT_PAGE vnet_entry_htcp_adaptive_backoff
 204:	f940016b 	ldr	x11, [x11]
			204: R_AARCH64_LD64_GOT_LO12_NC	vnet_entry_htcp_adaptive_backoff
 208:	f940158c 	ldr	x12, [x12, #40]
 20c:	b86b698c 	ldr	w12, [x12, x11]



cc_htcp.ko disassembles to

   10f44:	f940000a 	ldr	x10, [x0]
   10f48:	f940024b 	ldr	x11, [x18]
   10f4c:	f942ad6c 	ldr	x12, [x11, #1368]
   10f50:	d503201f 	nop
   10f54:	10082f6b 	adr	x11, 21540 <vnet_entry_htcp_adaptive_backoff>
   10f58:	f940158c 	ldr	x12, [x12, #40]
   10f5c:	b86b698c 	ldr	w12, [x12, x11]

An adrp+ld pair, with paired relocations, is being transformed into a nop+adr pair.  So it computes the address of the variable instead of loading its value.   I am not familiar with ARM relocation codes and can not say if the R_AARCH64_ADR_GOT_PAGE+ R_AARCH64_LD64_GOT_LO12_NC combination is correct here, or whether the :got: and :got_lo12: prefixes in the assembly are correct.  I can say that I have seen this sort of behavior due to a linker bug before on other systems, when the linker assumes a relocation type is only used with a certain opcode.