In D50964#1167235, @arrowd wrote:

The following program

#include <libunwind.h>

int main(int argc, char* argv[])
{
        unw_context_t ctx;
        return unw_getcontext(&ctx);
}

fails to link with cc unw.c -lgcc_s, but works with cc unw.c -lgcc_eh. So these two libraries aren't really counterparts.

Ok, let's go for this.

Add hack to ensure depend-cleanup.sh does not inadvertently delete static llvm/clang etc libraries.

3 days is fine, if it works. :)

Can you put a MFC after: 3 days header in the commit message, please? (I tend to use 3 days for upstream commits that are trivial; this one seems a bit more extensive so maybe you can extend the MFC period to one or two weeks)

Gleb started a discussion here: https://lists.freebsd.org/archives/freebsd-toolchain/2025-June/002326.html

I noticed that we install the /usr/include/libunwind.h file, which can be picked by ports, but symbols from this header are defined in the /usr/lib/libgcc_eh.a static library. I find this a bit surprising.

If this header is intended to be public, then we should have libunwind.so in base, so that programs can link to it. Otherwise, we should hide the header and make ports use devel/libunwind or devel/llvmXY instead.

Note that this may require a ports exp-run? I'd expect some stuff to fall over.

Btw there is still one rather nasty fly in the ointment for this patch: in tools/build/depend-cleanup.sh we have a hack to delete libllvm.a, libclang.a and liblldb.a from the obj tree. I don't know of a way to get at the MK_LLVM_STATIC option value in that shell script though. Does anybody have any good idea for that? Run ${MAKE} in some form that spits out the value?

• Adjust comment in liblldb Makefile
• Expand descriptions in WITH/WITHOUT fragments

In D50956#1163146, @emaste wrote:

IMO we should expand on the user-facing effect in the option descriptions - i.e., for WITH_LLVM_STATIC the Clang/LLVM toolchain binaries will be much larger but start more quickly. WITHOUT_LLVM_STATIC the Clang/LLVM binaries will be smaller, but will take longer to start.

Also include src.opts.mk in libllvm's and liblldb's Makefiles, otherwise
MK_ variables might not be defined.

LGTM

I think this makes sense, thanks. Proves how often I run cleanworld, which is almost never :)

Okay, let's change the default then. It would be nice if you can put a reminder in the commit message that says you have to turn on LLVM_ASSERTIONS whenever you want to report an issue with the toolchain itself.

The idea behind this being on in -CURRENT is to catch more errors. The same really holds for WITNESS and other assertions, but you could argue that those are more for the core of the system, while llvm-project is contributed software. Note that we caught many assertions since they are enabled by default, so they definitely have value.

Maybe a silly question, but when make unwraps these command lines and then prints them for the user, will the output become longer due extra spaces? Could be bad for my OCD... :)

Sure.

Are you planning on MFC'ing this? But LGTM for the rest.

It's a bit unfortunate, but better to do the same thing as most other systems.

Add OptionalObsoleteFiles.inc entries.

Add workaround for powerpc "relocation R_PPC_GOT16 out of range" errors.

I tried this out, and it solved my issue of etcupdate leaving around a /usr/src/lib/libc/nsswitch.conf file.

In D49965#1139483, @emaste wrote:

I got my bikeshedding out of the way on other reviews so will reserve comment on the filename :)

Abandoning, arc diff totally messed this up.

Split off the bsd.suffixes-extra.mk part to D49965.

In D49962#1139420, @emaste wrote:

Are you up for splitting bsd.suffixes-extra.mk out first, without any functional changes?

Note this requires D49951 to be committed first, since otherwise tests/sys/fs/fusefs falls over due to it picking sbin/mount.c over tests/sys/fs/fusefs/mount..cc for targeting mount.pieo. This has to do with some internal ordering in make that I haven't been able to fully figure out.

LGTM, as long as there are include guards in mntopts.h :)

Another minor remark, in D49958 I added include guards to mntopts.h. Since the header is now actually installed, I think it would be proper to do so here too.

Apart from the nits, LGTM!

Abandoning in favor of D49951 and D49952.

In D49958#1139282, @brooks wrote:

Sorry, I should have added you to D49951 and D49952 which this duplicates. I don't think a new library is warranted and if it is, it should probably be libmntopts or the like not something as generic as libmount.

Great, thanks.

Yes, this is fine too. At some point both gcc and clang switched to gnu++17 by default, but it's better to be explicit.

Kludge approved :)

Yeah, go for it. :)

Thanks, I will commit this.