Re: Should stable/1[34] and supported 1[34].*-RELEASE have -ftls-model=initial-exec fixes MFC'd and/or EC'd for armv7?

From: Mark Millard <marklmi_at_yahoo.com>
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