Updated the comment based on @MaskRay's feedback, thanks!
This (and the previous) patch were based on release/10.x, I plan to commit it later today since the final tag is tomorrow.

Thanks for picking this up again. I've left some nitpicks below in a quick review.

Should this be abandoned for D49466?

Thanks, I'll push once the build and test pass.

@al3xtjames I was about to commit this but noticed that some others check whether getInit(0) is NULL or not before proceeding. Should that be done here as well? If not, why?

LGTM. Not sure when this was added, but it must have been present since at least Mac OS X 10.7.3:
https://opensource.apple.com/source/Libc/Libc-763.12/stdio/fputs.3.auto.html
https://opensource.apple.com/source/Libc/Libc-763.12/stdio/FreeBSD/fputs.c.patch.auto.html

Hi @dankm, any progress on this feature? The proposed branch off date for Clang 9.0.0 is 18 July 2019: https://lists.llvm.org/pipermail/cfe-dev/2019-June/062628.html

@rsmith The latest patch version has addressed your feedback, can you confirm that this is ready to be merged?

@rsmith Are you happy with the changes, is it ready to be merged?

Verified again!

Thanks, I have also verified this patch against the above test case.

This looks reasonable to fix the problem at hand, but would it handle nested structures too?

We should cherrypick this patch to 8.0 branch so that 8.0 will be shipped with this fix included. I'll file a bug and do, so no worries.

Can I backport this to the 8.0 release branch? Are there any special rules for that?

I mean why don't you move this comment inside if (NextOp == 0xe8) {?

Updated coding style as suggested. Only one issue is still open (whether to remove/change the comment in the source file). Thank you for the feedback!

Changes still look reasonable, but the preceding path (https://reviews.llvm.org/D56769) needs some work.

This has some broken edge cases. Consider extending the ReplacePathPrefix test in unittests/Support/Path.cpp for the new cases.

In D47819#1356498, @mgorny wrote:

PASS: MemorySanitizer-X86_64 :: Linux/sunrpc_bytes.cc (2932 of 6195)
PASS: MemorySanitizer-X86_64 :: Linux/sunrpc_string.cc (2935 of 6195)
PASS: MemorySanitizer-X86_64 :: Linux/sunrpc.cc (2974 of 6195)
PASS: ThreadSanitizer-x86_64 :: sunrpc.cc (5110 of 6195)

So yep, it works. Tested both ways now.

The direction of this patch looks reasonable to me. Is it worth mentioning the issue (https://github.com/google/sanitizers/issues/974) in the commit message?

Still fine by me, thanks!

Tests pass here, using it on a large CMake project with a CMAKE_BUILD_TYPE=Debug and c/cxxflags -ffile-prefix-map=$builddir= -ffile-prefix-map=$srcdir/= -fuse-ld=lld successfully strips all traces of $builddir and $srcdir.

Except one thing, it looks reasonable to me. I'll try to run some tests and report back tomorrow.

Could you add more tests to check the error message for bad options (missing =):

It would be nice to have this for Clang 8.0, the branch date is within 5 days :)

There appears to be other definitions for HAVE_TIRPC_RPC_XDR_H in
lib/sanitizer_common/sanitizer_platform.h
lib/sanitizer_common/sanitizer_platform_limits_freebsd.cc

Added braces and updated comment. Thanks for the feedback!

In D55860#1335943, @mstorsjo wrote:

I'm not intimately familiar with the structure of ar files, but the change looks like it makes sense to me at least. Does MSVC link.exe produce import libraries with deduplicated member names as well?

This was addressed by https://reviews.llvm.org/D51183 (without new tests though).

Changes:

• Return 1 (instead of 0) if none of the files could be parsed (and add a test for it)
• Propagate any error code (like 2 in case of some missing files) from Tool.buildASTs instead of returning 0.
• Change test to accomodate the change in behavior/output due to https://reviews.llvm.org/D51729 (Thanks to Sam McCall for feedback.)

In D51729#1288360, @sammccall wrote:

I'm not entirely sure what to do here. The old behavior works great in cases where a complete database is available (produced by CMake). The new behavior might work better for clangd (?), but it breaks a use case (see above).

For clangd, but also clang-tidy and clang-query when the user *does* want to use it on files not represented in the CDB. (e.g. stale or headers)
There's indeed a tension here, because the CDB discovery needs to have a default configuration.

Before this patch, missing compilation database entries resulted in "Skipping .... Compile command not found." which is assumed by the tests in this clang-query patch: https://reviews.llvm.org/D54109

The functionality looks correct to me, but could you include some tests in test/Driver/ and test/Preprocessor/ just to be sure?
test/Driver/debug-prefix-map.c and test/CodeGen/debug-prefix-map.c could serve as inspiration.

ping

In D47817#1155717, @mgorny wrote:

This would be the first user of pkg-config here. I am not sure if this would be the best fix. Usually you cannot (easily) recompile libc and override it, but for external libs such as libtirpc this should be more doable (I think).

I don't think libtirpc's include path is expected to be predictable by design. I think it's something distro maintainers have to choose to avoid collision with headers that (used to be) installed by glibc. In any case, I can't think of a better solution than pkg-config here (libtirpc doesn't come with CMake modules).

Hi, thank you for the patch. First a disclaimer, I am not familiar with this RPC API at all.

Superseded by https://reviews.llvm.org/D48265

Superseded by https://reviews.llvm.org/D48265

With the three whitespace issues fixed up, LGTM.
(I am also observing that the version check in ThreadDescriptorSize is Linux-only (minus Android), such that the cpp guards !SANITIZER_FREEBSD && !SANITIZER_ANDROID && ... will be broad enough.)

Oops, wrong (unsplit) diff. This is the correct one and depends on D48254.

Wait, what happened to compiler-rt/test/asan/TestCases/log-path_test.cc? It should not be removed.

One small comment, LGTM otherwise.

In D35396#1132500, @kubamracek wrote:

In D35396#1132499, @Lekensteyn wrote:

Hi, what is the status with this patch? It was referred to by https://reviews.llvm.org/D36495 (which is committed), and there still seem to be a lot of users of %T.

Yes, that's why this one didn't land yet. We need to get rid of all the %T users first.

Good idea, the workaround was only needed for glibc (which officially supports Linux and Hurd only).

Hi, what is the status with this patch? It was referred to by https://reviews.llvm.org/D36495 (which is committed), and there still seem to be a lot of users of %T.

Thanks for the reviews, I fixed both typos in r334719 and disabled the test due to a lit problem on Windows. I'll be looking at supporting glob patterns for commands in general or just the "cd" builtin.

FYI, this caused a regression: http://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/49533/testReport/junit/LLVM/Other/statistic_ll/

In D46545#1130319, @morehouse wrote:

In D46545#1129900, @Lekensteyn wrote:

Since the code appears to be functionally correct, I am considering adding XFAIL for both cases.

Please do this for now to get the bot green again. If you figure out the root cause you can submit a fix later.

In D46545#1129800, @morehouse wrote:

This is breaking the Android sanitizer bot: http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-android/builds/11563.

Please take a look and/or revert.

Thanks Vitaly, that's a very useful tool!

Since performance was pretty bad at exit (see https://github.com/google/sanitizers/issues/963#issuecomment-396323984), I decided that optimization is mandatory for this version. UnregisterGlobals is apparently not only called for dlclose, but also for other shared libaries.

Note: the issue was reproduced on both Linux (clang from trunk) and Windows (Clang 6.0.0). The fix was tested for Linux, but not on Windows (if someone has a setup ready, please test!). I also confirmed that the latest patch passes the test on Windows 7 and 10 (x64) and that without the patch, it crashes.