Re: Should stable/1[34] and supported 1[34].*-RELEASE have -ftls-model=initial-exec fixes MFC'd and/or EC'd for armv7?
Date: Wed, 20 Nov 2024 16:29:42 UTC
[Just CC'ing Jessica C. instead of kib. This is based on https://reviews.freebsd.org/D42415 having Jessica's note: If IE is fixed then lib/libc/Makefile probably should enable it on arm as a follow-up, which I *think* is the only architecture not covered by that if statement, unless I'm missing something ] On Nov 17, 2024, at 10:50, Mark Millard <marklmi@yahoo.com> wrote: [Not that they could be timed for 14.2-RELEASE at this point.] Given an update to the bootstrap lang/rust compiler that has already been fixed, the below fixes why lang/rust has not built on the official package build server s for 1[34].* since at least lang/rust 1.77.0 . (The traceable records do not go back beyond that. But it may be that rust became dependent at 1.77.0 for all I know.) It also blocks private armv7 lang/rust builds for stable/1[34] . Other packages may have build failures that are related but I'll use the lang/rust activity here: used as the test case. I have locally tested building lang/rust 1.82.0_1 on stable/14 both with and without the changes in question. With the 2 commits it built just fine, unlike without. (I did not try just 1 of the 2, just the pair as I unit.) The primary, recent bugzilla activity is at: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=282663 But the below avoids going through the discovery process and history that is recorded there. (There are also other related bugzilla's around but this one is where the potential MFC's were identified.) See also the fallout records for lang/rust for armv7: https://portsfallout.com/fallout?port=lang%2Frust&maintainer=&env=armv7&category=&flavor= (but ignore the lang/rustpython lines that also match the pattern used). Only main got the fixes back in 2023-Nov and only main has worked for lang/rust for the official armv7 package build servers since then for any records that I've found -- lang/rust 1.77.0 and later. The 2 commits are: https://cgit.freebsd.org/src/commit/?id=98fd69f0090da73d9d0451bd769d7752468284c6 https://cgit.freebsd.org/src/commit/?id=6e5b1ff71e01bd48172483cb6df921f84300ea3a I show the details below. Whitespace might not be accurately preserved below. https://cgit.freebsd.org/src/commit/?id=98fd69f0090d is: author R. Christian McDonald <rcm@rcm.sh> 2023-11-03 12:56:58 +0000 committer Kristof Provost <kp@FreeBSD.org> 2023-11-03 21:43:40 +0000 . . . rtld/arm: fix initial-exec (IE) thread-local storage relocation net/frr[89] revealed an interesting edge-case on arm when dynamically linking a shared library that declares more than one static TLS variable with at least one using the "initial-exec" TLS model. In the case of frr[89], this library was libfrr.so which essentially does the following: #include <stdio.h> #include "lib.h" static __thread int *a __attribute__((tls_model("initial-exec"))); void lib_test() { static __thread int b = -1; printf("&a = %p\n", &a); printf(" a = %p\n", a); printf("\n"); printf("&b = %p\n", &b); printf(" b = %d\n", b); } Allocates a file scoped `static __thread` pointer with tls_model("initial-exec") and later a block scoped TLS int. Notice in the above minimal reproducer, `b == -1`. The relocation process does the wrong thing and ends up pointing both `a` and `b` at the same place in memory. The output of the above in the broken state is: &a = 0x4009c018 a = 0xffffffff &b = 0x4009c018 b = -1 With the patch applied, the output becomes: &a = 0x4009c01c a = 0x0 &b = 0x4009c018 b = -1 Reviewed by: kib Sponsored by: Rubicon Communications, LLC ("Netgate") Differential Revision: https://reviews.freebsd.org/D42415/ Diffstat -rw-r--r-- libexec/rtld-elf/arm/reloc.c 7 1 files changed, 5 insertions, 2 deletions diff --git a/libexec/rtld-elf/arm/reloc.c b/libexec/rtld-elf/arm/reloc.c index c3e95940be74..6efc9f499761 100644 --- a/libexec/rtld-elf/arm/reloc.c +++ b/libexec/rtld-elf/arm/reloc.c @@ -280,10 +280,13 @@ reloc_nonplt_object(Obj_Entry *obj, const Elf_Rel *rel, SymCache *cache, return -1; tmp = (Elf_Addr)def->st_value + defobj->tlsoffset; - if (__predict_true(RELOC_ALIGNED_P(where))) + if (__predict_true(RELOC_ALIGNED_P(where))) { + tmp += *where; *where = tmp; - else + } else { + tmp += load_ptr(where); store_ptr(where, tmp); + } dbg("TLS_TPOFF32 %s in %s --> %p", obj->strtab + obj->symtab[symnum].st_name, obj->path, (void *)tmp); https://cgit.freebsd.org/src/commit/?id=6e5b1ff71e01 is: author R. Christian McDonald <rcm@rcm.sh> 2023-11-09 20:22:21 +0000 committer Kristof Provost <kp@FreeBSD.org> 2023-11-09 20:24:23 +0000 . . . Reviewed by: kib Sponsored by: Rubicon Communications, LLC ("Netgate") Differential Revision: https://reviews.freebsd.org/D42415/ libc: enable initial-exec (IE) as default thread-local storage model on arm As suggested by jrtc27@ in https://reviews.freebsd.org/D42415, this patch enables IE as default thread-local storage model in libc on arm. Reviewed by: kib Sponsored by: Rubicon Communications, LLC ("Netgate") Differential Revision: https://reviews.freebsd.org/D42445 Diffstat -rw-r--r-- lib/libc/Makefile 4 1 files changed, 0 insertions, 4 deletions diff --git a/lib/libc/Makefile b/lib/libc/Makefile index 7540eb8c21ad..e817104642b8 100644 --- a/lib/libc/Makefile +++ b/lib/libc/Makefile @@ -54,11 +54,7 @@ CFLAGS+=${CANCELPOINTS_CFLAGS} # Use a more efficient TLS model for libc since we can reasonably assume that # it will be loaded during program startup. -.if ${LIBC_ARCH} == "aarch64" || ${LIBC_ARCH} == "amd64" || \ - ${LIBC_ARCH} == "i386" || ${LIBC_ARCH} == "riscv" || \ - ${LIBC_ARCH:Mpowerpc*} != "" CFLAGS+= -ftls-model=initial-exec -.endif # # Link with static libcompiler_rt.a. === Mark Millard marklmi at yahoo.com === Mark Millard marklmi at yahoo.com