On Fri, 25 Jul 2025, Radek Barton wrote: > Hello. > > Sending another version with suggestion to use more strict conditions for > instructions detection applied.
Sorry to keep nitpicking... > Radek > > --- > From fed90421a5c9c8539a7aad92588730f0a84fd306 Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Radek=20Barto=C5=88?= <radek.bar...@microsoft.com> > Date: Sat, 19 Jul 2025 19:17:12 +0200 > Subject: [PATCH v4] Cygwin: mkimport: implement AArch64 +/-4GB relocations > MIME-Version: 1.0 > Content-Type: text/plain; charset=UTF-8 > Content-Transfer-Encoding: 8bit > > Based on https://sourceware.org/pipermail/cygwin-patches/2025q3/014154.html > suggestion, this patch implements +/-4GB relocations for AArch64 in the > mkimport > script by using adrp and ldr instructions. This change required update > in winsup/cygwin/mm/malloc_wrapper.cc where those instructions are > decoded to get target import address. > > Signed-off-by: Radek BartoĊ <radek.bar...@microsoft.com> > --- > winsup/cygwin/mm/malloc_wrapper.cc | 29 ++++++++++++++++++++--------- > winsup/cygwin/scripts/mkimport | 7 +++++-- > 2 files changed, 25 insertions(+), 11 deletions(-) > > diff --git a/winsup/cygwin/mm/malloc_wrapper.cc > b/winsup/cygwin/mm/malloc_wrapper.cc > index 863d3089c..db6ffb5a0 100644 > --- a/winsup/cygwin/mm/malloc_wrapper.cc > +++ b/winsup/cygwin/mm/malloc_wrapper.cc > @@ -51,16 +51,27 @@ import_address (void *imp) > __try > { > #if defined(__aarch64__) > - // If opcode is an adr instruction. > - uint32_t opcode = *(uint32_t *) imp; > - if ((opcode & 0x9f000000) == 0x10000000) > + /* If opcode1 is an adrp and opcode2 is ldr instruction: > + - https://www.scs.stanford.edu/~zyedidia/arm64/adrp.html > + - https://www.scs.stanford.edu/~zyedidia/arm64/ldr_imm_gen.html https://www.scs.stanford.edu/~zyedidia/arm64/br.html Comment doesn't mention new check for opcode3 is br instruction. I don't know if you want to mention that all now require x16 register. Specifically, this now checks for adrp x16, X ldr x16, [x16, #X] br x16 it might be less effort to simply list that. > + NOTE: This implementation assumes that the relocation table is made > of > + those specific AArch64 instructions as generated by the > + winsup/cygwin/scripts/mkimport script. Please, keep it in sync. */ > + uint32_t opcode1 = *((uint32_t *) imp); > + uint32_t opcode2 = *(((uint32_t *) imp) + 1); > + uint32_t opcode3 = *(((uint32_t *) imp) + 2); > + if (((opcode1 & 0x9f00001f) == 0x90000010) && > + ((opcode2 & 0xffc003ff) == 0xf9400210) && > + (opcode3 == 0xd61f0200)) > { > - uint32_t immhi = (opcode >> 5) & 0x7ffff; > - uint32_t immlo = (opcode >> 29) & 0x3; > - int64_t sign_extend = (0l - (immhi >> 18)) << 21; > - int64_t imm = sign_extend | (immhi << 2) | immlo; > - uintptr_t jmpto = *(uintptr_t *) ((uint8_t *) imp + imm); > - return (void *) jmpto; > + uint32_t immhi = (opcode1 >> 5) & 0x7ffff; > + uint32_t immlo = (opcode1 >> 29) & 0x3; > + uint32_t imm12 = ((opcode2 >> 10) & 0xfff) * 8; // 64 bit scale > + int64_t sign_extend = (0l - ((int64_t) immhi >> 32)) << 33; // sign > extend from 33 to 64 bits > + int64_t imm = sign_extend | (((immhi << 2) | immlo) << 12); > + int64_t base = (int64_t) imp & ~0xfff; > + uintptr_t* jmpto = (uintptr_t *) (base + imm + imm12); > + return (void *) *jmpto; > } > #else > if (*((uint16_t *) imp) == 0x25ff) > diff --git a/winsup/cygwin/scripts/mkimport b/winsup/cygwin/scripts/mkimport > index 0c1bcafbf..33d8b08fb 100755 > --- a/winsup/cygwin/scripts/mkimport > +++ b/winsup/cygwin/scripts/mkimport > @@ -73,8 +73,11 @@ EOF > .extern $imp_sym > .global $glob_sym > $glob_sym: > - adr x16, $imp_sym > - ldr x16, [x16] > + # NOTE: Using instructions that are used by MSVC and LLVM. Binutils are > + # using adrp/add/ldr-0-offset though. Please, keep it in sync with > + # import_address implementation in winsup/cygwin/mm/malloc_wrapper.cc. Whitespace nit: everything else seems to be using a tab but this line is using two spaces. > + adrp x16, $imp_sym > + ldr x16, [x16, #:lo12:$imp_sym] > br x16 > EOF > } else { > -- > 2.50.1.vfs.0.0 >
