In D144830#4186256, @rsundahl wrote:

Not a waterfall, just a test failure due to an issue best read here: https://reviews.llvm.org/D127929 (show older changes). You can't test on iOS but you could test on macOS but even then your minimum deployment target has to be pretty high to see the issue manifest. In other words, you unsurprisingly couldn't anticipate this w/o better documentation and evangelization of the issue.

usama/rsundahl how do I run an iOS test? I can't do arm, but x86_64 should be equivalent for a dyld problem? Also is there a waterfall for these types of breakages that I don't know about?

Added test in asan. There wasn't an easy spot to put it in the sanitizer_common tests because I wanted to verify that the actual sanitizer libraries were calling through correctly when __sanitizer_report_error_summary was overridden.

Moved to REQUIRES: asan-static-runtime

Went with unsupported.

In D144830#4165204, @yln wrote:

Taking a step back, please check my understanding:

In D144830#4159426, @yln wrote:

In D144830#4157118, @vitalybuka wrote:

[... ] __sanitizer_print_stack_trace? Seems like more stable bet.

+1

Updated with fixed tests for replaceable_new_delete.cpp.

Yep. Apologies. Been a long time since I committed anything to LLVM. I'll try and take a look tonight/first thing tomorrow.

@usama54321 or @yln are you able to commit for me?

Switched over to using "sanitizer_sandbox_on_notify" from "sanitizer_print_stack_trace" because it's available in the internal headers and serves the same purpose.

This diff should be buildable. Adding requested test shortly.

Updated with buildable patch.

Accidentally uploaded

In D144672#4151146, @usama54321 wrote:

Sorry for this. I need to confirm that asan and tsan are not supported on darwin. Please don't merge until then. Thanks

Addressed comments. PTAL

LGTM

Thanks Puyan! Awesome to see this moving forward.

Has anything happened with this at all or did it get dropped?

So this is probably old news inside of Apple, but this can cause [NSInvocation retainArguments] to crash:

Any other comments on this?

Updated based on review

Fixed up review comments.

Okay, so I've looked into it a little bit further and it's really just that when we force inlining everything, we basically end up with a lot fewer functions being defined in the object file, and consequently the resulting code is smaller. It's not rocket science and it's what I expected.

Now, what I'm thinking is that perhaps what you might want to do is crank up the inliner using compiler flags instead of trying to influence that in the library. I'm not exactly sure how/whether one can do that, but it seems like what you want may be something like: clang++ -falways-inline or something along those lines. This would have the "added benefit" that all your code would get the same treatment, not only libc++. Does that make sense?

-O0 for everything I'm complaining about here :)

OK, here's for Google Maps (a mix of ObjC, ObjC++, C++) with multiple TU:

Chromium bug 961450 is tracking to see if it causes size/performance changes there as well

As a piece of test code:

Hey Louis,

This change caused some significant bloat and affected our performance and link times.

Akira/John/Erik - any thoughts?

Updated to fix Stephane's good catch of Objective C vs Objective-C

Full Diffs as requested.

In D55544#1326956, @theraven wrote:

It would probably be a good idea to have a similar check on properties, as property encoding strings contain the type encoding (plus extra stuff).

Added some spacing around early exit as requested by theraven.