In D50294#1246980, @MaskRay wrote:

In D50294#1245454, @emaste wrote:

I'm using this change: https://github.com/emaste/freebsd/commit/1c3deab6d518feb1a7e88de5b342a139e4022a21

In FreeBSD 12 and later we use Clang, lld, and ELF Tool Chain. (We still have gas and objdump from the outdated binutils 2.17.50.)

Will you upstream this commit (I can abandon this one)? I created this revision because I was learning clangDriver... I had a vague and probably incorrect impression that some folks said kgdb or dtrace or whatever might not support DWARF 3 or 4.

LGTM. I much prefer only - short and -- long options, except for lld where unfortunately there's a precedent of accepting both.

In D58102#1422441, @hjl.tools wrote:

We are using a special CET SDV for CET enabling. We have been enabling CET in Fedora piece by piece by adding -fcf-protection to CFLAGS.

In D58102#1422340, @xiangzhangllvm wrote:

H.J. have enable the CET in GNU-toolchain and rebuild/rewrite the related libs/linux-kernal, his team are testing it for at last one year.

@xiangzhangllvm, a question for you not directly related to the patch but I suspect you might be best suited to find an answer: how do we test CET support today? We started trying to add CET to FreeBSD some time ago but have no way to test any work that we do.

In D58102#1411701, @fweimer wrote:

How does lld handle the non-executable stack flag? From a developer point of view, the CET processing is supposed to be very similar.

@dim

In D57371#1390001, @ruiu wrote:

Ed,

Did -z ifunc-noplt work well for FreeBSD kernel? If it works well, maybe we should merge it to the mainline lld tree?

In D57749#1385666, @joerg wrote:

The description is certainly not right. IE can be used from dlopen'd objects without problems as long as there is still reserved space around and the data is not initialized non-trivially or the dynamic linker is willing to fix up all threads. The point of the flag is to allow the dynamic linker to make a quick decision without having to scan the relocation table first. A classic user of this was libGL. It also applies to many platforms, not just i386. As such, please update the description before any commit.

In D33041#1382751, @grimar wrote:

In D33041#1382592, @emaste wrote:

FreeBSD patch adding initial exec TLS mode for dynamically loaded shared objects: https://reviews.freebsd.org/D19072. Can we revisit this lld patch?

I'll take a look tomorrow.

FreeBSD patch adding initial exec TLS mode for dynamically loaded shared objects: https://reviews.freebsd.org/D19072. Can we revisit this lld patch?

There is a list of 140-150 unsafe (LLD_UNSAFE) packages with LLD.

I think the arch-change (switching from a whitelist to a MIPS blacklist) is reasonable. What is the motivation for dropping DT_HASH, just binary size reduction?

Can I go ahead with this version as a starting point?

• fix warnings from mandoc -Tlint and/or igor
• remove options that should not be listed

This is fine for me (from FreeBSD's perspective), but I suspect some people (on operating systems without ifuncs in libc) may be surprised by having a(n empty) .rela.plt in all of their statically linked binaries.

I have an interim copy in FreeBSD and am updating it based on the feedback here; I'll update this review shortly.

Additional comment that wasn't submitted while reviews.llvm.org was down on Friday: I started working on this as it's needed for us to switch from GNU objdump to llvm-objdump in FreeBSD (FreeBSD PR229046).

Review feedback

I think this is reasonable.

Perhaps someone else can review?

In D54864#1307053, @Higuoxing wrote:

lgtm, but seems that some hidden/alias options are not listed here (I could help add them). I am not expert in this area, so I will leave the decision to @kristina

Confirmed this fixes my bug.

I think that the inconsistency of linkers behavior shown above shows that LLD behavior is not incorrect. It is different. But we never tried to be fully bug compatible with bfd/gold. First of all the problem is caused by a weak assumption in the linker script, which assumes that orphans are placed at the particular place before the assignment command, and my feeling it should be fixed.

I backported the change to lld 6.0 currently in FreeBSD-HEAD and it has the expected behaviour, but for my test case I think sh_info should reference .got.plt not .plt - note the address marked with ######## below.

This change looks good to me; testing it out in the FreeBSD tree now.

This change LGTM.

Looks good to me

For SHT_REL and SHT_RELA, sh_link references the associated symbol table and sh_info the "section to which the relocation applies."

I don't think you can fix the issue by recompiling it without -fPIC

I would think a fix would be compiling with -fPIC.

Maybe "available for some architectures in at least..."? Or maybe we shouldn't bother trying to list versions, and mention it is dependent on CPU arch, linker, and rtld?

I am sure IFUNC+DT_TEXTREL does not affect FreeBSD, but OpenBSD can be affected. Many other systems (including musl) may not support IFUNC at all.

Probably. We'll need this information though I think. Otherwise, how will we find that it is safe to remove the flag?

The benefit from this that we can track and update the status of the fix and even maybe provide info about the known distributions affected/fixed on the page what can be helpful for removing this flag at some longer-term point.

I'm using this change: https://github.com/emaste/freebsd/commit/1c3deab6d518feb1a7e88de5b342a139e4022a21

The FreeBSD build bot was down for a little over a week due to an IP address issue, but is back now: http://lab.llvm.org:8011/builders/lldb-amd64-ninja-freebsd11

In D50297#1236972, @ruiu wrote:

In theory this change shouldn't affect the amount of memory used by an application

Agreed - when I submitted it I thought it might be uncommon but worth supporting, but the evidence Mark found suggests it is a single-purpose feature that is not relevant to the platforms lld supports.

Thanks for finding that Mark - based on that IMO this feature is not worth supporting.

keep flags in order

Reuse existing FLAGS: test

See also https://reviews.freebsd.org/D17172 for a lld 6 version intended for commit to FreeBSD's in-tree lld.

@ruiu what do you suggest as the next step here?

LGTM. I made the same change in FreeBSD r336745.

FWIW I don't have a strong opinion on | vs newline to separate aliases, but if we are going to go with the newline -compact and .Pp approach I'd suggest making that change as a separate commit first so that the actual content changes here are easier to see/review.

For reference I had a discussion on markup for option aliases with Ingo Schwarze and he wrote:

In D41821#1152337, @atanasyan wrote:

I'm going to implement both parts of the plan.

In D31528#1114868, @atanasyan wrote:

Ping?

@ruiu could I use a post-commit review option in that case?

In D46690#1094394, @grimar wrote:

After the second look, I think it is ok to add check here that we still keep non-a section and keep its SHT_NOTE flag for safety though. I'll update the patch.

note-noalloc2.s is close to my original test, I think that's reasonable.

1. what is the content of .debug-ghc-link-info? If the section contains an optional, human-readable string, they might have just used .comment instead.

Committed to FreeBSD in r333401

Yeah, @espindola was added by a Herald rule.