• Reorder test cases for smaller diffs

Rebase past rL344307

Rebase past rL344307

lgtm, thanks for the test!

In D53064#1260942, @mstorsjo wrote:

In D53064#1260876, @rupprecht wrote:

I'm not sure @FILE syntax is actually supported by llvm-nm. At least, I can't find any unit tests that test it, and @ is not present in llvm-nm.cpp. I think that change should come first?

It does support it just fine right now (even if there's no explicit unit test for it). llvm-nm.cpp calls ParseCommandLineOptions, which internally calls ExpandResponseFiles, so this is implicit in more or less every llvm tool.

I'm not sure @FILE syntax is actually supported by llvm-nm. At least, I can't find any unit tests that test it, and @ is not present in llvm-nm.cpp. I think that change should come first?

In D52398#1257133, @aaronpuchert wrote:

In D52398#1257092, @rupprecht wrote:

I patched the proposed fix-forward and it seems to have things building again. Is there an ETA on landing that? If it's going to take a bit, is there any chance we could revert this patch until then?

The fix was just committed (rC343902). Generally I'm for reverting if there are functional deficiencies, but here it's more that an unrelated issue was accidentally uncovered by this patch.

In D52398#1255290, @aaronpuchert wrote:

@hokein Please have a look at D52888, maybe you can try it out already. The problem was that ?: expressions are considered a branch point and when merging both branches the warning was emitted. Before this change, we couldn't look into __builtin_expect and so we didn't see that the result of TryLock is used to branch.

Thanks Petr, I completely missed that it was going to stderr!
There are two tests in lld and clang to update as well, see rL343743 and rL343744.

In D52810#1254103, @MaskRay wrote:

Looks good, GNU and some BSD nm use: nm: /bin/cat: no symbols but the repeated command name and filename do not seem useful.

In POSIX.1-2017 specification of nm, "If no symbolic information is available for a valid input file, the nm utility shall report that fact, but not consider it an error condition."

In D52316#1249892, @RKSimon wrote:

This probably needs to wait until the compile time regressions have been addressed: https://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20180924/590785.html

• Elaborate libm dependency in comment

@compnerd, can you take a look? Or add another reviewer comfortable approving if you can't review?

Another ping for @howard.hinnant or @compnerd

The GNU objcopy name is only-section, not only-sections (note singular vs plural)
But otherwise LGTM :)

Thanks for fixing this!

In D49678#1226534, @plotfi wrote:

In D49678#1222836, @chandlerc wrote:

Reverted temporarily in r341360 to unbreak my build and the aarch64 build bot. Let me know if I can help with the investigation in any way.

@jakehehrlich @chandlerc I need help reproing this. I finally got an aarch64 setup up and running and the tests pass. I don't understand why this would fail.

• Simplify test

• Use SmallVector for CopyConfigs

• Add test case for -o flag error

In D51493#1223898, @rupprecht wrote:

In D51493#1223877, @jakehehrlich wrote:

When I run that yaml2obj test I actually get the following error rather than a silent failure.

error: Unknown section referenced: '0' at YAML section '.rela.plt'.

You might need to modify yaml2obj to test this or just convince me that this doesn't need testing. Alternatively you can upload a binary to the repo in Inputs that reproduces the issue.

I'm using yaml2obj from head -- maybe there's a recent change that fixed this?

Looks like r339873 changed/fixed this.

In D51493#1223877, @jakehehrlich wrote:

When I run that yaml2obj test I actually get the following error rather than a silent failure.

error: Unknown section referenced: '0' at YAML section '.rela.plt'.

You might need to modify yaml2obj to test this or just convince me that this doesn't need testing. Alternatively you can upload a binary to the repo in Inputs that reproduces the issue.

In D51493#1223839, @jakehehrlich wrote:

Did you confirm that yaml2obj respects the explicit Link field?

• Explicitly set Link = 0 when there's no symbol, and update test case to check Link = 0 in the input file as well.

I'm still against having a space in the string as the basis for interpreting it as a usage parameter. It's way too surprising.

In rL341002#285538, @rsmith wrote:

Wait, what? Adding documentation for this attribute caused #pragma clang attribute to start supporting it?!

In rL341002#285532, @nickdesaulniers wrote:

Thanks for cleaning this up, sorry I missed it. What's the command to run this test to verify?

BTW, seems like these docs have tests that weren't updated. Fixed with rL341002, but please take a look if that's not the right fix.

Left out of the commit message: this fixes the test after rL340987

Some uses of this already use a space and probably don't want this behavior -- e.g. tools/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp invokes this w/ "clang -cc1". (There are a few others).

Ping for @howard.hinnant @compnerd -- mind taking a quick look for a second LGTM?

rebase/fix stale filename in a test case

• crs->cr

• Fix tests for windows compatibility

• Reorganize CopyConfig sections

In D50343#1200318, @jhenderson wrote:

LGTM, but there is one thing still outstanding, I think, which @jakehehrlich mentioned in a comment regarding the CopyConfig options:

A pattern of grouping by type does seem to have formed organically however. Let's stick with it.

I don't have a strong preference either way, but you should get @jakehehrlich to confirm he is happy before committing this change.

• Make test case simpler by using -elf-output-style=GNU