I can see it both ways - I like when CMake tells me if a configuration is not supported - but on the other hand this is probably a pretty easy to spot in the compiler error what's wrong. I guess the section could be replaced with actually trying to compile a snippet with TryCompile with the sanitizer flag instead and replace the specific flag check.

In D95669#2530360, @protze.joachim wrote:

Next time please provide the full context of the change as suggested in the guide: https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

In D88674#2326878, @mstorsjo wrote:

In D88674#2326835, @thieta wrote:

Added documentation in the rst file. Anyone with commit access - feel free to commit this for me.

Also fwiw, I think you should ask for commit access.

In D88674#2326869, @alexshap wrote:

@thieta - thanks for the patch and for your patience. According to https://llvm.org/docs/DeveloperPolicy.html#commit-messages we would need your name + e-mail (to correctly specify the author of the changes).

Added documentation in the rst file. Anyone with commit access - feel free to commit this for me.

Can this be merged? @rupprecht ?

In D89142#2322663, @mehdi_amini wrote:

Wow pretty neat to see llvmbuild being removed!

That said we are still depending on it (we generate Bazel build file from this and use this in many projects using MLIR like TensorFlow, XLA, IREE...).
We need to move away from it: can you ping me and/or @GMNGeoffrey when you get closer to have this in a landing state?

Hey - I am keen on getting this done. Can we come to a consensus on what the best way is to handle this?

I don't have a strong opinion on what's best - both will solve my problem, if people think it would be better to use a regex I can rework the patch to do that (as long as there is a good regex implementation in LLVM I can use already).

A compile time option sounds good, per target configuration sounds even better!

maked comments as done

• made sure one call is using -- and one is using -
• clang-format'ed
• line breaks fix

The test fails doesn't seem related. But let me know if anyone thinks otherwise.

Marked comments as done.

Fixed all review feedback and added tests.

@lebedev.ri fix for your issue posted here: https://reviews.llvm.org/D81385

Thanks - I'll look into a fix.

@mstorsjo mind landing this for me as well?

Updated with parenthesis - feel free to commit if it looks good to you. Same as before Tobias Hieta tobias@hieta.se.

In D80493#2052698, @mstorsjo wrote

Well, wasn't it the case that lto mingw builds were broken to begin with? So either nobody did it, or they already patched around it (and will need to adjust their patch)?

Sounds good - I'll close this one and open three new ones.

I am planning to revise this one now that we have thinlto-cache-dir option landed here are my plans:

"Tobias Hieta <tobias@hieta.se>" thanks!

Could you commit this for me Martin? I don't have access yet.

Thanks guys - I don't have commit access. So could anyone of you commit this? Please use my email address tobias@hieta.se if you commit as another author.

@psmith This worked! All patches is now passing with the latest diff. I did a quick rebase as well - no conflicts.

This applies the latest patch from @psmith which means that all the tests now passes! 🎉

I have applied @psmith changes to arm-branch-rangethunk.s and reworked arm-branch.s to work with the changes above.

we are now only down to two failing tests:

Oh I see that there are some files I forgot to update the comments to have /// - I'll revisit them later this week.

This adjusts the comments as suggested by @psmith and addresses all the tests except the following three:

I have started working on the tests - I have 17 left to do - but since the diff is going to be big I thought I wanted to upload this for now and start collecting feedback to make sure I am doing the right things here.

Thanks for handling this!

A small ping here.

So it sounds like I could go ahead and start changing the tests to match this? I can get to it during the weekend I hope. I anyone have a huge objection to this getting merged - let me know asap.

@srhines Thanks for your reply! Would it be enough for you guys to expand this block in clang https://github.com/llvm/llvm-project/blob/master/clang/lib/Driver/ToolChains/Linux.cpp#L256 for Android's usage? Or would a change in lld make more sense?

Hello Nick,

Thanks everyone! It was a great learning experience getting this patch in and learning more about building/testing LLVM!

Can someone please commit this for me if everyone is happy with it now?

Updated the tests with @jhenderson comments.

Fixed all outstanding comments with documentation, test cases and linking to the correct upstream bug.

In D69188#1726328, @MaskRay wrote:

So chatted with Sledge. Their authoritative input on this matter is that we do need to carry a workaround. I think this patch is good to go once Sledge has an official bug id and the comment is updated with that piece of information.

<Sledge> it's going to take quite a while for that fix to filter through to users, particularly for Ubuntu and Debian LTS users :-/

I totally defer to you guys on if this is something you want to accept or not. We are now patching our own toolchain with this patch to not create bad binaries. If I where to make a case for accepting this patch it would be something like:

@rupprecht - Thanks! I have addressed the comments and uploaded a new rev. Hope I did it correctly. I don't have commit access - if you could commit it for me it would be great. I might have other patches in the future (looking at you MingW driver) but I'll cross that bridge then.

This revision should address all the comments in the previous revision. Comments are updated, testing --strip-all-gnu and documentation changes. Let me know if you have additional comments.

Thanks everyone for the feedback. I was meeting up with our QA team this week and they said that they seen the issue with these binaries on a much larger array of devices than I initially thought. This is not a bug in upstream GLIBC - this is a bug in debian/ubuntu versions that patch glibc to specifically check for SHT_ARM_ATTRIBUTES. We tested with all debian based NAS devices we had and saw the same issue. We even tested with raspbian on raspberry pi and this also contains the issue.

In D69188#1715040, @abrachet wrote:

Seems like a hack, no? What about using --keep-section when you need it? You say its not strictly needed on modern systems and the problem is with the dlopen implementation of these older libcs, and not llvm-objcopy, so I would be hesitant to add something like this especially given we have a solution with [[ https://llvm.org/docs/CommandGuide/llvm-objcopy.html#cmdoption-llvm-objcopy-keep-section | --keep-section ]]