We also face this problem on FreeBSD/mips64 when trying to link with lld.

(Aside, for FreeBSD we are rather eager to have this support arrive in lld upstream and will from there backport it to the lld 5.0.0 in our src tree. As it stands we cannot link the full base system with lld because lldb has grown too large -- https://bugs.freebsd.org/215691)

Thank you, this will be quite useful for FreeBSD and the change seems reasonable to me.

Ping for two open questions

Still would appreciate diffs uploaded with full context - i.e.,

Note that you can upload patches created through various diff tools, including git and svn. To make reviews easier, please always include as much context as possible with your diff! Don’t worry, Phabricator will automatically send a diff with a smaller context in the review email, but having the full file in the web interface will help the reviewer understand your code.

Adding the comment above the list seems like a good idea to me.

I've committed this to FreeBSD's copy of lldb in r322326. @labath if you're happy with this patch I will commit to lldb for @karnajitw. I'm not sure how the patch ended up with a conflict, but it's just a whitespace issue.

Actually, I think you probably need to extend the @skipIfLinux to apply to freebsd as well.

With this patch i386 test results are comparable to amd64, and I'm happy with it from a FreeBSD perspective (modulo the PlatformOpenBSD patch conflict).

The change in PlatformOpenBSD.cpp failed to apply for me (although it was trivial to manually apply it).

With this patch I observed three new failures on FreeBSD and three new unexpected passes on FreeBSD. An example of a new failure:

The patch LGTM, I will test it soon.

Sorry I missed this when first uploaded, I will look at it shortly.

Sorry I missed this when first uploaded, I will look at it shortly.

Handy Phabricator link: rL307364

Fixed in rL308382

Bug report in http://llvm.org/pr33821

For reference reverted in rL308239

Approach LGTM, modulo @grimar's issue. I wonder if we should call the option --sysroot (and make it available to users) though? We'd have a little more work to do to handle a user-supplied sysroot when creating reproducers.

One note, the ordering of the value tags differs wrt GNU ld. It shouldn't matter, just something I noticed.

I've successfully tested this change imported into my FreeBSD work branch
https://github.com/emaste/freebsd/commit/12f8c0fd362c128c03acc9689c76d8c5d5c69dbe

One note, the ordering of the value tags differs wrt GNU ld. It shouldn't matter, just something I noticed.

This LGTM.

I believe -F / --filter can be used multiple times, as with -f.

Should we instead switch DT_NEEDED to be like DT_AUXILIARY? This would be similar to the format used by binutils readelf and ELF Tool Chain readelf.

For future uploads can you please include context (e.g. git diff -U9999)
This seems reasonable to me.

As an aside it looks like cmake's omp lib rule has no dependency on the version script / export list file.

LGTM from the FreeBSD side. The launch code for FreeBSD came from the original (in-process) implementation that Linux and FreeBSD shared.

I like this idea.

FreeBSD build and test in progress.

Fine from FreeBSD

In D34140#779285, @ruiu wrote:

This seems fine, but who actually passes "armelf"? Just want to confirm that this is in use.

This LGTM, I had hacked something like this locally for my FreeBSD/lld work.

Generally LGTM with two questions inline.

LGTM from my perspective.

Thanks! I will try to add an equivalent section for FreeBSD shortly after this goes in.

In D32271#759361, @dim wrote:

As I found out in rL303015, the KERN_PROC_PATHNAME has one drawback: if an executable file has multiple hard links, you will get just one of its filenames as the result. Since that filename is more or less randomly chosen, it does *not* have to correspond to the actual argv[0] the executable was invoked with. If that does not matter, this approach is fine, though.

Two small nits noted inline, but now the test does not pass without D32271 and does with it. Note that in the "without" case it returned error rather than failing, perhaps related to the path issue I noted?

Without tcmalloc, on Ubuntu 14.04, 40 core VM: 13%
With tcmalloc, on Ubuntu 14.04, 40 core VM: 24% (built using cmake ... -DCMAKE_EXE_LINKER_FLAGS=-ltcmalloc_minimal, which amazingly only works when building with clang, not gcc...)

Does it make sense to make it read-only from beginning?

The change looks fine to me from a FreeBSD perspective

This test passes even without the change in D32271, presumably because exe = os.path.join(os.getcwd(), 'newdir/proc_attach') is providing us with an absolute path for argv[0].

Test case in D32522

In D32271#735098, @krytarowski wrote:

I have plan to revisit corresponding files in NetBSD and switch from kvm(3) to sysctl(3). But this is lower priority than Process Plugin right now.

Slightly simpler approach after early returns added in rL301100

In D32120#734603, @krytarowski wrote:

I don't agree. It would mean that it's supposed to work on BSD4.4 and derivate systems like BSD/OS.

Sorry I've been away from LLDB for a bit, I will take a look at this soon.

In D32125#728166, @krytarowski wrote:

Is this just GNU specific? BSD moved to 64-bit off_t on 32-bit platforms 20+ years ago.

It's perhaps no-op, but it might be noted in the commit message what platforms are supposed to be improved.

As an aside we may want pthread_getthreadid_np() (the user-facing thread ID) on FreeBSD instead of pthread_self() (an opaque pointer), as a separate / later change.

For freebsd/Host.cpp I will also move Host::GetEnvironment near the end of the file (in the same place as linux/Host.cpp).

I compared the tmp objects in the failing and passing case using diffoscope and noticed one unexpected difference, although it does not appear to be root cause of the failure.

In D31528#716759, @atanasyan wrote:

Thanks a lot for your help. BTW what tools and libraries do you use to build LLD: Clang (what version?) or gcc, libc++ or libstdc++?

FreeBSD ostensibly supports o32, n32, and n64, so n32 is indeed relevant for us. That said the in-tree toolchain still uses GCC 4.2.1 for MIPS, and the LLVM work is still somewhat forward-looking for us.

One other note, I tried linking the FreeBSD base system on mips64 with this patch applied (since we require mxgot or multigot to be able to link the entirety of the base system), and have a reproducible segfault in lld. A reproducer is at https://people.freebsd.org/~emaste/lld/lld-mips.tar. It seems the crash is reproducible after reverting the patch, so I'll submit a bug for it shortly.

In D31528#715638, @atanasyan wrote:

Is the error reproduced each time? I just made a clean build with the patch and did not see the error.

I had ELF/mips-got-and-copy.s fail for me with this applied, with .../mips-got-and-copy.s.tmp.o:(.text+0xC): relocation R_MIPS_GOT16 out of range