Thanks for working on this.

This seems to me another case that you need to fix the sysroot environment instead of adding file mapping logic to tools.
Currently you need clang and lld (lld-link or ld.lld), if you later need more tools, are you going to add the logic to all the involved tools?

I am working on cross compiling support for Fuchsia Clang toolchain targeting Windows and recently encountered an issue that would be the perfect use case for this case.

Test LLVM :: tools/llvm-reduce/fail-file-open.test is still failing on our windows bots after the fix:

We are seeing test failures on Fuchsia Clang builder for windows-x64 after this change have landed.

We are seeing test failures on Fuchsia Clang builder for windows-x64 after this change and 18b9c4637099f6ed5414d8778de8c773291a9cf9 have landed.

I think this breaks cmake configure. We saw errors like:

In D122459#3413101, @MaskRay wrote:

In D122459#3413092, @ayrtonm wrote:

It looks like the relocations pointing to an empty .eh_frame in crtbegin.o have an offset greater than the size of the input section pieces. So in that case we need to revert to the old behavior of returning the offset unchanged instead of throwing a fatal error.

@ayrtonm @haowei Is there a --reproduce=/tmp/rep.tar file for the crtbegin.o with a relocation with r_offset larger than the input section piece?

Hi, we are seeing build failures in our clang bots in Performing build step for 'runtimes-x86_64-unknown-linux-gnu' after this change lands, error message:

In D119479#3387880, @zixuw wrote:

@haowei I got a quick fix of completely purging the generator output in the test case. Could you take a look to have a sanity check before I land the fix on main?

diff --git a/clang/test/SymbolGraph/global_record.c b/clang/test/SymbolGraph/global_record.c
index ba4bf967e630..1e0294cda153 100644
--- a/clang/test/SymbolGraph/global_record.c
+++ b/clang/test/SymbolGraph/global_record.c
@@ -4,7 +4,9 @@
 // RUN: %t/reference.output.json
 // RUN: %clang -extract-api -target arm64-apple-macosx \
 // RUN: %t/input.c -o %t/output.json | FileCheck -allow-empty %s
-// RUN: sed -e "s@\"generator\": \"clang.*\"@\"generator\": \"clang\"@g" \
+
+// Generator version is not consisten across test runs, normalize it.
+// RUN: sed -e "s@\"generator\": \".*\"@\"generator\": \"?\"@g" \
 // RUN: %t/output.json >> %t/output-normalized.json
 // RUN: diff %t/reference.output.json %t/output-normalized.json
 
@@ -32,7 +34,7 @@ char unavailable __attribute__((unavailable));
       "minor": 5,
       "patch": 3
     },
-    "generator": "clang"
+    "generator": "?"
   },
   "module": {
     "name": "",

We are seeing test failures after this patch is landed:

Filed https://github.com/llvm/llvm-project/issues/54407 with reproducer attached.

In D121121#3387334, @ABataev wrote:

In D121121#3387320, @ronlieb wrote:

Hi Alexey
with this patch, i noticed an assert building one of our runtime files , the test case .c produced is around 24000 lines
would you like it as is? or reduced ?

Instruction does not dominate all uses!

%39 = call <8 x i16> @llvm.fshl.v8i16(<8 x i16> %18, <8 x i16> %38, <8 x i16> <i16 8, i16 8, i16 8, i16 8, i16 8, i16 8, i16 8, i16 8>)
%23 = shufflevector <8 x i16> %22, <8 x i16> %39, <8 x i32> <i32 0, i32 8, i32 9, i32 10, i32 11, i32 12, i32 13, i32 14>

Instruction does not dominate all uses!

%67 = call <8 x i16> @llvm.fshl.v8i16(<8 x i16> %44, <8 x i16> %66, <8 x i16> <i16 15, i16 15, i16 15, i16 15, i16 15, i16 15, i16 15, i16 15>)
%51 = shufflevector <8 x i16> %50, <8 x i16> %67, <8 x i32> <i32 0, i32 8, i32 9, i32 10, i32 11, i32 12, i32 13, i32 14>

in function eshift
fatal error: error in backend: Broken function found, compilation aborted!

Hi Ron, it would be good if you can provide reduced case. I'll revert the patch meanwhile.

While c1d4c67718db88be48f451b91f7bddab8fff2424 clears the cmake error. I am now seeing runtimes-i386-unknown-linux-gnu build failures related to the same symbol:

I just suppressed the error in c1d4c67718db88be48f451b91f7bddab8fff2424.

Can you provide i386-linux-gnu/crti.o and i386-linux-gnu/libc_nonshared.a(elf-init.oS)?
I need to understand the issue better to craft a better test. I believe the state before this patch worked for such symbols were accidental.

In D120626#3383810, @haowei wrote:

We are seeing build failures in cmake config step for the clang runtime builds after this patch. Example of error messages:

Thanks for the report, but with Host compiler does not support '-fuse-ld=lld' it might be difficult to know for sure whether the culprit was in an lld change or a brittle configure.
Providing CMakeError.log or CMakeOutput.log with the linker diagnostic could make me more sure that it was related to this lld change.
Anycase, I think the issue was exactly the one for i386 sanitizers and it should have been fixed by the aforementioned commit.

We are seeing build failures in cmake config step for the clang runtime builds after this patch. Example of error messages:

Filed https://github.com/llvm/llvm-project/issues/54122

Please correct the comments of the test file.
Please wait for @mcgrathr 's approval before you land the change.

The implementation of the filtering feature LGTM. My concerns is that do we need both 2 flags are needed at the same time. I am not very familiar with the __asan symbols use case so I will ask @mcgrathr and @phosek to comment on this.

I wasn't aware yaml2obj support -D as well. Thanks for pointing it out.

Test added in https://reviews.llvm.org/D119741

Adds minimal validation of STRSZ from .dynamic.

The latest patch no longer causes clang-refactor test failures on stage2 mac build on Fuchsia builders.

In D118376#3296441, @spatel wrote:

In D118376#3295685, @haowei wrote:

I am not an expert on Mac. I did some experiments. Using the clang built from this revision (b4b97ec813a02585000f30ac7d532dda74e8bfda) to build stage 2 clang, regardless of the revision of the second stage clang, will cause clang-refactor related test to fail. So I think this patch introduced a bug that changed clang's code gen behavior and cause clang to generate buggy code on the Mac. I still don't quite understand why the clang-refactor tool is the only one affected so far. If you need any binaries from stage 1 and 2 build to debug this issue, please let me know. I can help build them for you.

Thanks - I'll take any help I can get to reduce the bug. :)
I have reverted the patch on main, and I think I should also revert it on the 14 branch (if you can confirm that the bug is present resolved with that revert).

Is it correct that the bug is only visible on the Mac build (the same tests are not failing on other platforms)?
It's not clear to me from the logs what the failure is: all 5 of the tests in "clang/test/Refactor/Extract" cause clang-refactor to crash?

Hi, we are seeing clang Windows build breakages after this change was relanded, error message:

In D118376#3293280, @spatel wrote:

In D118376#3292090, @haowei wrote:

Hi, we found out that this patch breaks following tests in the Fuchsia's stage-2 mac clang builders:

Clang :: Refactor/Extract/ExtractExprIntoFunction.cpp
Clang :: Refactor/Extract/ExtractionSemicolonPolicy.cpp
Clang :: Refactor/Extract/ExtractionSemicolonPolicy.m
Clang :: Refactor/Extract/FromMethodToFunction.cpp
Clang :: Refactor/Extract/ObjCProperty.m

An example of the failure: https://ci.chromium.org/ui/p/fuchsia/builders/prod/clang-mac-x64/b8823769353085181809/overview . And test error messages can be found at https://ci.chromium.org/ui/p/fuchsia/builders/prod/clang-mac-x64/b8823769353085181809/test-results . These failures only happens in stage 2 build so it took us some time to bisect and confirm it. Could you take a look please? And if it takes a long time to fix, could you revert this change first please? Thanks.

Thanks for letting me know. I don't have a system set up to debug a stage 2 failure, so this is going to take a while unless someone else has a way to diagnose/reduce the errors more quickly. I will revert within a day if there is no update/progress.

In D118838#3292100, @tstellar wrote:

Is this something I should be updating when I bump the version in trunk?

In D118838#3292088, @ldionne wrote:

Oh, thanks, I was literally going to land the same change! I assume you noticed the Bootstrapping build was failing?

@haowei Just for the sake of the process, in the future, please try to follow the libc++ review guidelines and wait for the libc++ review group to be green before committing. You can always ping us on Discord if you're missing some approvals on a simple (and time critical) change like this, and we'll be happy to take a look quickly. Thanks for the patch!

Hi, we found out that this patch breaks following tests in the Fuchsia's stage-2 mac clang builders:

Clang :: Refactor/Extract/ExtractExprIntoFunction.cpp
Clang :: Refactor/Extract/ExtractionSemicolonPolicy.cpp
Clang :: Refactor/Extract/ExtractionSemicolonPolicy.m
Clang :: Refactor/Extract/FromMethodToFunction.cpp
Clang :: Refactor/Extract/ObjCProperty.m

If there is no further objection or comments, can I get a LGTM to land this change please?

In D116957#3251926, @Quuxplusone wrote:

Hi @haowei , could you please provide the output from the failed test?
The link you gave did lead me to https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket/8825027967296150705/+/u/clang/test/stdout?format=raw but there's no output there; it just says

PASS: libc++ :: libcxx/min_max_macros.compile.pass.cpp (6610 of 13450)
PASS: libc++ :: libcxx/ranges/range.nonprop.cache/emplace.pass.cpp (6611 of 13450)
FAIL: libc++ :: libcxx/nasty_macros.compile.pass.cpp (6612 of 13450)
PASS: libc++ :: libcxx/ranges/range.nonprop.cache/emplace_from.pass.cpp (6613 of 13450)
PASS: libc++ :: libcxx/no_assert_include.compile.pass.cpp (6614 of 13450)

What I'd need to see is, how did the test fail? What error messages did the compiler produce?
If you have access to a machine with the "Fuchsia" config on it, you can probably run that test yourself, grab the output and paste it here.

Hi, we are seeing test failures from libc++ :: libcxx/nasty_macros.compile.pass.cpp in Fuchsia's linux-x64 and arm64 clang builders after this patch was landed.

Change the fallback logics to Diff3. Also adds parents revision to make presubmit check happy.

In D116769#3230703, @jhenderson wrote:

I've not really dug into this patch at this point, but one high-level thing I wanted to highlight: it looks like you're doing a lot of reading of the object file to interpret the contents, in ways that tools like llvm-readelf and llvm-objdump already do. Do you actually need this much additional code as a result? It might need some refactoring, but it seems like we're otherwise reinventing the wheel...!

We are seeing assertion errors in some of Fuchsia's clang builders after this change was landed, the error message is:

If you insist, please reach out to all of them with your understanding. I don't think I'm the only one to take your viewpoint.

OK, your team knows this but you need to show the proof publicly.

I feel you are asking the proof for a gentlemen's agreement. It is my bad that I didn't mention that the patch was confirmed by locally reverted this patch. If that caused some confusions, I apologize. But except for saying I did the local revert, I cannot give you any proof for that. If you feel local console logs will work I can attach them next time.

In rG21dbfd430021cc8f4270daaec15897b023a5cacc#1046543, @MaskRay wrote:

In rG21dbfd430021cc8f4270daaec15897b023a5cacc#1046539, @haowei wrote:

In rG21dbfd430021cc8f4270daaec15897b023a5cacc#1046529, @MaskRay wrote:

In rG21dbfd430021cc8f4270daaec15897b023a5cacc#1046524, @haowei wrote:

Hi, this change as well as 7c0881a38f5e0f50375a089e5a3c9b2d215be0e9 broke Fuchsia rust build and we are seeing crashes from some build tools. Both of your changes are not NFC but they landed without proper review. Could you revert them now so we can restore the build please?

No.

Rust has broken usage https://github.com/rust-lang/rust/issues/92181 and I have worked around it in 48161b7490e4aa174faf2b0e840e743a3ee56a4a

That work around does not clear the build issues we saw on our end. We are still seeing breakages at LLVM ToT with Fuchsia build. At this point, most of our engineers are out for vacation and it would be difficult to fix the issue in a timely manager from Fuchsia side. Besides, this patch, as well as 7c0881a38f5e0f50375a089e5a3c9b2d215be0e9, were not properly reviewed and affected multiple users of lld. According to the policy, they should be reverted.

Thanks for providing the link. Your original comment did not provide a link and did not make it clear reverting the commits resolved your issue, so your argument got a question.

Sometimes a patch author is willing to take a compromise, but reluctant if they are not at fault and you miss such points (Rust is at fault, and the llvm/lib/CodeGen somehow made the latent issues possible in the first place).
At the very least, I wished that the original message would be associated with the section name.

In rG21dbfd430021cc8f4270daaec15897b023a5cacc#1046529, @MaskRay wrote:

In rG21dbfd430021cc8f4270daaec15897b023a5cacc#1046524, @haowei wrote:

Hi, this change as well as 7c0881a38f5e0f50375a089e5a3c9b2d215be0e9 broke Fuchsia rust build and we are seeing crashes from some build tools. Both of your changes are not NFC but they landed without proper review. Could you revert them now so we can restore the build please?

No.

Rust has broken usage https://github.com/rust-lang/rust/issues/92181 and I have worked around it in 48161b7490e4aa174faf2b0e840e743a3ee56a4a

Hi, this change as well as 7c0881a38f5e0f50375a089e5a3c9b2d215be0e9 broke Fuchsia rust build and we are seeing crashes from some build tools. Both of your changes are not NFC but they landed without proper review. Could you revert them now so we can restore the build please?

Fix some "no newline" warnings.

In D115024#3169966, @mcgrathr wrote:

This feature seems like a good addition. I'm wondering about the interactions with other switches and if it might make sense to rearchitect some of the CLI API here for the more arcane corners. Maybe it's already orthogonal enough, but I'm not sure. I'm thinking in particular about the various --strip-* options. Perhaps it's the case that all such variations apply only to text IFS output? I'm also wondering if there might be use cases for multiple different kinds of IFS output (i.e. different sets of stripping / arch options). Come to think of it, we could even have uses for multiple ELF stub outputs from the same IFS input with different switches (e.g. arch). So perhaps it makes sense to go in a direction where each output file is separately described with both its format (IFS vs ELF) and its format options (stripping, arch, etc), and there can be any number of such output files in whatever combination. I'm not sure exactly what a CLI syntax for that would look like. Another possibility that might fit well for build system integration is to accept a complex output specification in JSON either as a command-line switch value or from a file.

I suspect that even if we do want to pursue a fancier CLI API along those lines, this might be a reasonable incremental step in that direction.

I updated the unit tests and adds new tests for the new flags.

Example of build races failures: