In D23229#509463, @jhb wrote:

Yes, I was testing this locally, though without the :=, just a plain =. I was testing it as part of a make tinderbox run though that is only on amd64 and hasn't gotten to mips, so hadn't seen breakage that would require :=.

In D23204#509367, @jhb wrote:

In D23204#509143, @arichardson wrote:

mips64hf is currently broken because libc_nossp is built as soft-float (I think this is the lib32 version only since the error happens when building rtld-elf32):

ld: error: /home/alr48/obj/build/freebsd-universe-build/exports/users/alr48/sources/freebsd-universe/mips.mips64hf/libexec/rtld-elf32/rtld_libc.a(lstat.nossppico): floating point ABI '-msoft-float' is incompatible with target floa
ting point ABI '-mdouble-float'

I almost fixed this when fixing the mips64el case. Probably what we should do is set the 32-bit MACHINE_ARCH to MACHINE_ARCH:s/64// or some such to preserve el and hf suffixes.
However, note that part of the real problem here is that rtld-elf32 is somewhat broken in that it lets CFLAGS from the non-compat MACHINE_ARCH "leak" through. This doesn't
happen for the lib32 stage which is why the lib32 stage was building as soft-float since the MACHINE was "mips" but does happen for rtld-elf32. That's probably a bug in the bsd.compat.mk thing that @brooks might want to look at.

In D23204#509149, @arichardson wrote:

I also tried building n32 but that fails when linking the syscall objects. My guess is that SYS.h is missing the PIC n32 case and always generates position dependent assembly.

ld: error: can't create dynamic relocation R_MIPS_HI16 against symbol: __cerror in readonly segment; recompile object files with -fPIC or pass '-Wl,-z,notext' to allow text relocations in the output
>>> defined in cerror.pico
>>> referenced by shm_rename.S:4
>>>               shm_rename.pico:(__sys_shm_rename)

I also tried building n32 but that fails when linking the syscall objects. My guess is that SYS.h is missing the PIC n32 case and always generates position dependent assembly.

mips64hf is currently broken because libc_nossp is built as soft-float (I think this is the lib32 version only since the error happens when building rtld-elf32):

Looks fine to me but someone else should give the final approval.

I guess I could try to build and boot a n32 world, but I'm not sure which kernel I should be using to test it.

No longer accurate, I have changes to allow bootstrapping localedef and CDDL code.

I am not sure what the correct process is to merge upstream LLVM changes. Is it fine to just commit this diff or are any other steps required?

Ping? Can we merge this or do I have to convince upstream LLVM to add checks for the FreeBSD _FOO_DECLARED macros to their stddef.h/stdarg.h? I'm not sure how willing they would be to make that change though.

I agree that a small separate tool would be nicer. I don't really have time to write one right now though. I'll wait before committing this until it's the last blocker for cross-building. Maybe by then I'll find some time to add a write_locked (or whatever name we want to choose) bootstrap tool.

Ping?

Looks good. Compiles for me with latest LLVM.

In D22630#497926, @cem wrote:

This is for cross-building?
FreeBSD's stddef.h includes sys/cdefs.h. If you aren't finding FreeBSD's stddef.h in a cross build, why would you expect to find FreeBSD's sys/cdefs.h?
I guess glibc's sys/cdefs.h includes __BEGIN_DECLS, etc, but I don't know how portable that assumption is (no idea what MacOS has, and I don't know if windows environments like mingw64 have a sys/cdefs.h).
I'd prefer a more holistic approach — use the src tree FreeBSD's headers when we're cross-compiling, rather than the host's.

In D22464#491516, @kevans wrote:

In D22464#491447, @arichardson wrote:

In D22464#491412, @kevans wrote:

In D22464#491409, @arichardson wrote:

LGTM. Only concern might be that adding /usr/libexec to PATH adds too much stuff.
Hopefully I can get the strict tmppath usable by default soon so it shouldn't matter anymore then.

Yeah, so what I really wanted here with flua was some way to indicate that flua can/should be symlinked in from the host (preferably to the same location that I've added to BPATH) on the versions it's *not* being bootstrapped (__FreeBSD_version after I added it) to simplify this, since I don't really want to add /usr/libexec. Any advice on making that happen (or just getting flua into one of the standard BPATH/XPATH paths while leaving it installing to /usr/libexec) is quite welcome. =)

It should be possible to modify tools/build/Makefile and change the host-symlinks target to add a flua symlink if it exists on the host.
Something like (untested):

.if exists(/usr/libexec/flua)
    mkdir -p ${DESTDIR}/usr/libexec
    ln -sf /usr/libexec/flua ${DESTDIR}/usr/libexec/flua
.endif

Yeah, I like that, thanks! I think the mkdir -p can safely be omitted -- we create it in the installdirs target now in case it's bootstrapped, which should always be run prior to host-symlinks if I understand correctly. If we later determine flua needs to be bootstrapped, that just gets built+installed over the symlink, right?

In D22464#491412, @kevans wrote:

In D22464#491409, @arichardson wrote:

LGTM. Only concern might be that adding /usr/libexec to PATH adds too much stuff.
Hopefully I can get the strict tmppath usable by default soon so it shouldn't matter anymore then.

Yeah, so what I really wanted here with flua was some way to indicate that flua can/should be symlinked in from the host (preferably to the same location that I've added to BPATH) on the versions it's *not* being bootstrapped (__FreeBSD_version after I added it) to simplify this, since I don't really want to add /usr/libexec. Any advice on making that happen (or just getting flua into one of the standard BPATH/XPATH paths while leaving it installing to /usr/libexec) is quite welcome. =)

LGTM. Only concern might be that adding /usr/libexec to PATH adds too much stuff.

In D17002#489675, @jhb wrote:

Does this work with GCC? Seems no:

> mips-unknown-freebsd12.1-gcc -print-resource-dir
mips-unknown-freebsd12.1-gcc: error: unrecognized command line option '-print-resource-dir'; did you mean '-print-search-dirs'?
mips-unknown-freebsd12.1-gcc: fatal error: no input files
compilation terminated.

This change is required to build with an unmodified upstream LLVM since the default upstream clang builtin headers are incompatible with the ones in /usr/bin/include (see my earlier comments in this review).

ping

Disable all flags except -l/-u when bootstrapping

In D21971#480086, @brooks wrote:

In D21971#480075, @arichardson wrote:

I wish the C standard library had some kind of dynamic array so that this is not reimplement in every program...
I wonder if we could use something like the utarray.h macros from uthash?
Could we import the five headers that https://github.com/troydhanson/uthash into contrib and use utarray instead? I know the library is not that efficient since it uses strdup() for all strings that you insert but at least it means not reinventing the wheel.
We already have two copies of uthash.h in the source tree and elftoolchain also includes with utarray.h.

We certainly could use utarray instead, though I'm not super convinced by its readability.
(Importing uthash.h into contrib is on my todo list since I'd like to merge my changes to use it instead of bdb for hashes in init and a couple other places.)

I wish the C standard library had some kind of dynamic array so that this is not reimplement in every program...

If prefer to just spell out the assembly rather than just the magic cp* macros.
Apparently they place a $gp reload after every call: Additionally, it causes the assembler to emit:

Looks good. Although I'm not sure if we want this to be conditional on clang/lld since modern GCC+bfd should work (and maybe even clang+new bfd).

In D21699#473438, @arichardson wrote:

I can't remember why we need this change. There were some cases where we really need inital-exec for executables rather than global-dynamic.
I tend to forget everything about TLS once I no longer need to know it but maybe @jrtc27_jrtc27.com, @brooks or @jhb know.

I can't remember why we need this change. There were some cases where we really need inital-exec for executables rather than global-dynamic.
I tend to forget everything about TLS once I no longer need to know it but maybe @jrtc27_jrtc27.com, @brooks or @jhb know.

Looks good to me but I'd wait for @jhb to review it too.

I guess this happens because some files have more static globals than can be accessed in the immediate. In general lld should be able to use the multi-GOT trick but in that case it won't work.
If we really care about performance we could track down the individual files that need mxgot and only add it there but asking it globally should be fine.

I believe that splitting the operation into two asm blocks with C code between will break at -O0 since any variable access goes via the stack. This might cause the sc to fail.

LGTM but I also haven't tested it. I don't know how useful the profile stuff is. If it's not implemented for MIPS64 anyway we could just not build it?

Just found one more issue but otherwise LGTM. Maybe better to consistently use the _Tthr prefix.

I can confirm that it works when I apply the following change on top of your patch:

diff --git a/lib/libthr/thread/thr_init.c b/lib/libthr/thread/thr_init.c
index d9bde8a37c9d..78146c2d6dcd 100644
--- a/lib/libthr/thread/thr_init.c
+++ b/lib/libthr/thread/thr_init.c
@@ -257,7 +257,7 @@ static pthread_func_t jmp_table[][2] = {
        [PJT_RWLOCK_TRYWRLOCK] = {DUAL_ENTRY(_Tthr_rwlock_trywrlock)},
        [PJT_RWLOCK_UNLOCK] = {DUAL_ENTRY(_Tthr_rwlock_unlock)},
        [PJT_RWLOCK_WRLOCK] = {DUAL_ENTRY(_Tthr_rwlock_wrlock)},
-       [PJT_SELF] = {DUAL_ENTRY(_thr_self)},
+       [PJT_SELF] = {DUAL_ENTRY(_Tthr_self)},
        [PJT_SETCANCELSTATE] = {DUAL_ENTRY(_thr_setcancelstate)},
        [PJT_SETCANCELTYPE] = {DUAL_ENTRY(_thr_setcanceltype)},
        [PJT_SETSPECIFIC] = {DUAL_ENTRY(_thr_setspecific)},
diff --git a/lib/libthr/thread/thr_private.h b/lib/libthr/thread/thr_private.h
index 90385d1ab31c..47b82009765d 100644
--- a/lib/libthr/thread/thr_private.h
+++ b/lib/libthr/thread/thr_private.h
@@ -1069,7 +1069,7 @@ int _thr_setspecific(pthread_key_t, const void *);
 void *_thr_getspecific(pthread_key_t);
 int _thr_setcancelstate(int, int *);
 int _thr_setcanceltype(int, int *);
-pthread_t _thr_self(void);
+pthread_t _Tthr_self(void);
 int _thr_rwlock_init(pthread_rwlock_t *, const pthread_rwlockattr_t *);
 int _thr_rwlock_destroy(pthread_rwlock_t *);
 int _Tthr_rwlock_rdlock(pthread_rwlock_t *);
diff --git a/lib/libthr/thread/thr_self.c b/lib/libthr/thread/thr_self.c
index 168d91721e0b..68bf5bc39312 100644
--- a/lib/libthr/thread/thr_self.c
+++ b/lib/libthr/thread/thr_self.c
@@ -38,11 +38,11 @@ __FBSDID("$FreeBSD$");

Some more information:

(lldb) register read
General Purpose Registers:
       rax = 0x00000000000001b0
       rbx = 0x0000000000000001
       rcx = 0x00007fffffffeb10
       rdx = 0x00000000002050fd
       rdi = 0x000000000031d2e8  __sF + 312
       rsi = 0x000000000031bec0  __xlocale_global_locale
       rbp = 0x00007fffffffe890
       rsp = 0x00007fffffffe878
        r8 = 0x00007fffffffeb10
        r9 = 0x00007fffffffeb10
       r10 = 0x00007fffffffeb10
       r11 = 0x0000000000000201
       r12 = 0x00000000002002e0
       r13 = 0x00000000002002e0
       r14 = 0x00007fffffffebd8
       r15 = 0x00007fffffffebe8
       rip = 0x00000000002fd4b8  test-c-first`__sys_thr_self + 8
    rflags = 0x0000000000000202
        cs = 0x0000000000000043

With -O0 at the time of the crash:

(lldb) p *__stdoutp
(FILE) $1 = {
  _p = 0x00000000000186e8 ""
  _r = 0
  _w = 0
  _flags = 2185
  _file = 1
  _bf = (_base = "pthread_create = 0x301e10\n\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5)
  _lbfsize = -4096
  _cookie = 0x000000000031d2e8
  _close = 0x00000000002fe580 (test-c-first`__sclose at stdio.c:80)
  _read = 0x00000000002fe4c0 (test-c-first`__sread at stdio.c:56)
  _seek = 0x00000000002fe540 (test-c-first`__sseek at stdio.c:72)
  _write = 0x00000000002fe500 (test-c-first`__swrite at stdio.c:64)
  _ub = (_base = 0x0000000000000000, _size = 0)
  _up = 0x0000000000000000
  _ur = 0
  _ubuf = ""
  _nbuf = ""
  _lb = (_base = 0x0000000000000000, _size = 0)
  _blksize = 4096
  _offset = 0
  _fl_mutex = 0x0000000000000000
  _fl_owner = 0x0000000000000000
  _fl_count = 1
  _orientation = 0
  _mbstate = (__mbstate8 = "", _mbstateL = 0)
  _flags2 = 0
}
(lldb) p __stdoutp->_bf._base
(unsigned char *) $4 = 0x000000080033b000 "pthread_create = 0x301e10\n\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa.
(lldb) p __stdoutp->_bf._size
(int) $5 = 4096

Just to be clear this only happens if I actually start a thread and call pthread_join (using the program I posted here). The simple case from the PR works fine.

In D21088#458174, @kib wrote:

In D21088#457869, @arichardson wrote:

I am now getting a very strange error:

root@qemu-native-alr48:~ # clang -static -o test-thr-first -nodefaultlibs -lthr -lc test.c -fuse-ld=lld && ./test-thr-first
pthread_create = 0x2693d0
result = 0x1234
root@qemu-native-alr48:~ # clang -static -o test-c-first -nodefaultlibs -lc -lthr test.c -fuse-ld=lld && ./test-c-first
pthread_create = 0x2a58a0
Segmentation fault (core dumped)
root@qemu-native-alr48:~ # clang -static -o test-c-first -nodefaultlibs -lc -lthr test.c -fuse-ld=lld && lldb -- ./test-c-first
(lldb) target create "./test-c-first"
Current executable set to './test-c-first' (x86_64).
(lldb) r
Process 746 launching
Process 746 launched: '/root/test-c-first' (x86_64)
pthread_create = 0x2a58a0
Process 746 stopped
* thread #1, name = 'test-c-first', stop reason = signal SIGSEGV: invalid address (fault address: 0x186e8)
    frame #0: 0x00000000002a4b8d test-c-first`memcpy at memmove.S:306
(lldb) bt
* thread #1, name = 'test-c-first', stop reason = signal SIGSEGV: invalid address (fault address: 0x186e8)
  * frame #0: 0x00000000002a4b8d test-c-first`memcpy at memmove.S:306
    frame #1: 0x00000000002a2b53 test-c-first`__sfvwrite(fp=<unavailable>, uio=0x00007fffffffe888) at fvwrite.c:188:5
    frame #2: 0x0000000000285442 test-c-first`__vfprintf [inlined] __sprint(fp=<unavailable>, uio=<unavailable>, locale=<unavailable>) at vfprintf.c:166:8
    frame #3: 0x0000000000285436 test-c-first`__vfprintf [inlined] io_flush(iop=<unavailable>, locale=<unavailable>) at printfcommon.h:157
    frame #4: 0x000000000028541f test-c-first`__vfprintf(fp=0x00000000002b7ff8, locale=0x00000000002b6bf8, fmt0=<unavailable>, ap=0x00007fffffffeb10) at vfprintf.c:1033
    frame #5: 0x0000000000281ebd test-c-first`vfprintf_l(fp=0x00000000002b7ff8, locale=0x00000000002b6bf8, fmt0="result = %p\n", ap=0x00007fffffffeb10) at vfprintf.c:285:9
    frame #6: 0x00000000002a57ba test-c-first`printf(fmt=<unavailable>) at printf.c:57:8
    frame #7: 0x00000000002a5707 test-c-first`main + 119
    frame #8: 0x000000000022510f test-c-first`_start(ap=<unavailable>, cleanup=<unavailable>) at crt1.c:76:7

Indeed strange. I cannot reproduce it locally. For me it is just

x% ./test-c-first                     /usr/home/pooma/build/bsd/DEV/stuff/tests
pthread_create = 0x2a58b0
x%

Can you build libc with debugging info and lower optimization level (DEBUG_FLAGS=-g should be enough), and then print out the stdout FILE state ? I suspect there is some problem there.

I am now getting a very strange error:

root@qemu-native-alr48:~ # clang -static -o test-thr-first -nodefaultlibs -lthr -lc test.c -fuse-ld=lld && ./test-thr-first
pthread_create = 0x2693d0
result = 0x1234
root@qemu-native-alr48:~ # clang -static -o test-c-first -nodefaultlibs -lc -lthr test.c -fuse-ld=lld && ./test-c-first
pthread_create = 0x2a58a0
Segmentation fault (core dumped)
root@qemu-native-alr48:~ # clang -static -o test-c-first -nodefaultlibs -lc -lthr test.c -fuse-ld=lld && lldb -- ./test-c-first
(lldb) target create "./test-c-first"
Current executable set to './test-c-first' (x86_64).
(lldb) r
Process 746 launching
Process 746 launched: '/root/test-c-first' (x86_64)
pthread_create = 0x2a58a0
Process 746 stopped
* thread #1, name = 'test-c-first', stop reason = signal SIGSEGV: invalid address (fault address: 0x186e8)
    frame #0: 0x00000000002a4b8d test-c-first`memcpy at memmove.S:306
(lldb) bt
* thread #1, name = 'test-c-first', stop reason = signal SIGSEGV: invalid address (fault address: 0x186e8)
  * frame #0: 0x00000000002a4b8d test-c-first`memcpy at memmove.S:306
    frame #1: 0x00000000002a2b53 test-c-first`__sfvwrite(fp=<unavailable>, uio=0x00007fffffffe888) at fvwrite.c:188:5
    frame #2: 0x0000000000285442 test-c-first`__vfprintf [inlined] __sprint(fp=<unavailable>, uio=<unavailable>, locale=<unavailable>) at vfprintf.c:166:8
    frame #3: 0x0000000000285436 test-c-first`__vfprintf [inlined] io_flush(iop=<unavailable>, locale=<unavailable>) at printfcommon.h:157
    frame #4: 0x000000000028541f test-c-first`__vfprintf(fp=0x00000000002b7ff8, locale=0x00000000002b6bf8, fmt0=<unavailable>, ap=0x00007fffffffeb10) at vfprintf.c:1033
    frame #5: 0x0000000000281ebd test-c-first`vfprintf_l(fp=0x00000000002b7ff8, locale=0x00000000002b6bf8, fmt0="result = %p\n", ap=0x00007fffffffeb10) at vfprintf.c:285:9
    frame #6: 0x00000000002a57ba test-c-first`printf(fmt=<unavailable>) at printf.c:57:8
    frame #7: 0x00000000002a5707 test-c-first`main + 119
    frame #8: 0x000000000022510f test-c-first`_start(ap=<unavailable>, cleanup=<unavailable>) at crt1.c:76:7

Approach is definitely viable. However, if we need to update all functions a macro is probably a good idea.

Seems like it will still break for all other functions that aren't updated?

However, I just noticed that the following test case still results in an infinite loop/recursion even with this patch. I prints the address of pthread_create, but then freezes. Guess we need to do the same trick for even more functions.

#include <stdio.h>
#include <pthread.h>

Thanks! I can confirm that it works for the testcase from the PR if I apply the suggested fix.

I'm not sure why I made that change. I just remember that we had some compiler bugs in that function that went away when I added a printf. I think this was just one of the suspicious things that I saw so I made the change hoping that it would fix the miscompilation.

@bdrewery is this okay to commit now?

lld has been fixed.