Applied the suggested changes.

The same here! I don't think we should break builds of compiler versions that are still officially supported.

In D56873#1365075, @jhenderson wrote:

By the way, @jakehehrlich may remember exactly what the intended behaviour is in llvm-objcopy for ELF and combining rules. We discussed it a while back, but I can't remember them exactly, but it was something like explicit section keeps trump implicit section strips, and two explicits are an error. You might want to double-check with him.

In D56485#1365063, @filcab wrote:

In D56485#1364241, @pcc wrote:

This change would only impact the "diagnostic" (non-production) mode of CFI. In production builds we do not link the runtime library at all. So from a CFI perspective this is fine.

Well, that makes sense. CFI was my biggest problem with this, so I'm tempted to accept the patch (The * doesn't seem to be a valid character for Itanium C++ ABI names, so I think having the test there should only trigger on gcc code).
Please wait 1 or 2 days (to see if there are objections), and then you can consider the patch LGTMed. I'll try to poke it on tuesday/wednesday.

Thank you,

Filipe

Address review comments.

I'm sorry to bring bad news, but this fails for our gcc 4.9.2 build, when building llvm/tools/llvm-xray/xray-converter.cpp.
Failures looks like this:

In D56976#1365001, @ilya-biryukov wrote:

LGTM to unbreak the tests on *BSD, but could you please leave a comment?
It would probably be best to take argv[0] into account on *BSD systems as well (possibly only if clang could not be found).

At least if that's the behaviour on all other systems.

-EMPTY essentially implies -NEXT, it requires the next line to either be empty, or there to be EOF here. I haven't intended to use them combined but apparently I did do that in the paragraph above this. I'll change that one for consistency with the rest (that's the only one I have with NEXT-EMPTY at all).

For the record, you can't combine FileCheck suffixes (e.g. CHECK-NEXT-NOT is a NOT using a check prefix of CHECK-NEXT). There are some error checks to catch this, but I don't know how comprehensive they are.

Oops, yeah, I forgot for a moment that GNU output style is (currently) ELF-specific.

@jakehehrlich , @jhenderson, @rupprecht - maybe you have some other thoughts / I'm missing something ?

Just a quick drive-by comment to add to what @rupprecht said. Conceptually to me, the Object should be "final" when all of its details are consistent to represent a valid Object. Section indices are a good example, since there are sections within the Object, but until finalize is called, they don't have valid index values. As a result, a finalize method on the Object probably makes sense to achieve this. Writer meanwhile wants to handle the extra information that is specific to the file format, and not really generic in any meaningful sense (e.g. program headers). I agree that it's going to be hard to have a hard-and-fast rule though.

Address comments

Updated the patch, so that the extra flags are added only when -static option is used.
Used @atanasyan 's solution for CMakeLists.txt from D56836#1362293.
Made equivalent for the Makefile.

Ping

In D56485#1364241, @pcc wrote:

This change would only impact the "diagnostic" (non-production) mode of CFI. In production builds we do not link the runtime library at all. So from a CFI perspective this is fine.

LGTM.