I am curious if LocalAddressSpaceView stuff is in use, maybe it's abandoned and can be removed?

I'm not familiar with AddressSpaceView @vitalybuka but @yln or @kubamracek may have better perspective. Would applying this patch internally to see if we have explicit dependencies be useful? I can certainly do that as a black-box test, otherwise I'd have to dig in a bit to be of much help.

Tested this diff on both x86_64 and arm64. LGTM @thetruestblue. Thank you!

Thanks for your time and guidance getting this landed.

Apply proper backticks quotes to options. Remove redundant ellipses.

Hello Egenii,

Implement suggestions from latest review.

Ping @kcc , @yln and @kubamracek

@MaskRay, thank you for your approval. @eugenis, you were added as a blocking reviewer by @vitalybuka. If you are still without objection, can we get your approval and merge? Thank you all for your input.

Renamed darwin_exclude_symbols.inc asan_abi_tbd.txt.

Missed one file in revert of combined -mllvm= change.

Suggestions for compiler-rt/docs/asan_abi.md are captured in the successor file compiler-rt/docs/ASanABI.rst and marked complete.

Suggestions for compiler-rt/docs/asan_abi.md are captured in the successor file compiler-rt/docs/ASanABI.rst and marked complete.

Thank you for your review and thoughtful input @eugenis, @MaskRay and @vitalybuka. I think we're close to having everything incorporated. (@MaskRay, the doc files went from .md to .rst and I implemented all of your suggestions there.

Applied suggestions from reviewers

In D143675#4310903, @eugenis wrote:

I'm fine with it in general. Is asan_abi.cpp meant as a temporary stub? It's not even link anywhere in the current version.

Fixed nits (missing newlines at end of files)

Added testing. Removed asan_abi_shim.h.

In D143675#4330599, @thetruestblue wrote:

Small insignificant note from me: When this lands, please be sure to add me as co-author.
https://github.blog/2018-01-29-commit-together-with-co-authors/

In D143675#4310903, @eugenis wrote:

I'm fine with it in general. Is asan_abi.cpp meant as a temporary stub? It's not even link anywhere in the current version.

@kcc @eugenis @MaskRay @vitalybuka Ok to go with this? All new functionality is under the added flag so not expecting any surprises. Rename asabi->asan_abi as suggested.

Rename asabi->asan_abi

Rename fsanitize_address_stable_abi to fsanitize_stable_abi

@yln @wrotki @thetruestblue please review/approve so we can unblock CI.

Slightly more readable way to describe the situation

@kcc @eugenis @MaskRay @vitalybuka Ok to go with this? All new functionality is under the added flag so not expecting any surprises.

This looks good to me. The build fails appear to be a handful of unrelated ORC examples tests.

Added some suggested comments

ios->tvos

Unresolved issues from this differential are resolved in D147526.

Change to improve title and summary.

Update title and add Radar reference.

In D146728#4235579, @XiaodongLoong wrote:

Why we need a dummy address? Did you met any issues?

Yes. The cherry pick failed because git wanted an email address for the commit. Since the commit is an anonymous commit in a container, I chose to provide an anonymous email address.

In D146728#4235613, @thetruestblue wrote:

Can you just do

git cherry-pick -n

aka no commit? shouldn't require an email?

It's true that adding -n will avoid the commit, but doing so would be a functional change (leaving uncommitted changes in the working directory). This might not matter but it's more appropriate to anonymize the email address (since it's an owner-less docker image) and keeping the functionality as it is.

In D146728#4218437, @thetruestblue wrote:

This is fine... feels a little off.
Is there a script to run the docker container? Couldn't we bring it in there or when we build it?

I don't know use case for docker here. Just for tests? Or are we actually using these scripts for a dev environment we might push? Because in that case this feels wrong.

Ping - Thank you.

Opening a new revision to limit the scope of the test to Darwin only. (https://reviews.llvm.org/D147094)

Looks like I'll have to enable this test only on Darwin.

Rebased

I've tested this on linux and it now links (and passes). I am going to land it again and watch it closely.

I have reworded with your suggestions in mind @yln. Thank you for the input. I didn't add any minimum version information since the guidance is benign going backward and good hygiene moving forward.

Changed order of examples to meet narrative.

Reword for consistency.

Include a better summary

In D146471#4207971, @vitalybuka wrote:

@rsundahl Do you thing it's going to work with D146469 ?

Thanks for including me @vitalybuka... To understand why these tests have the conditional code making the overrides "weak" please see https://reviews.llvm.org/D127929 (show older changes) for the discussion. To summarize here though:
The idea of a "strong" overriding "weak" symbols is reasonably well understood and iirc is specified by the standard(s) but only in the context of static linking. The problem is complicated with dynamic linking. For a worst case thought experience, think about what should happen to weakly bound symbols when an application calls dlopen() on a dll that contains possible overrides or additional weak symbols. dyld uses two-level name space linking which essentially precludes a strong symbol in a different module from ever replacing a weak symbol, but it has a slight variation on this behavior in that during this "weak-def coalescing", it does consider any other modules containing weak definitions. The weak attributes on the "overrides" in the tests above are there only to cause the module (the test program) to be considered during weak-def coalescing at which time the weak symbol in the test program overrides the weak symbol reference from the dll. Yes, in this case it's a weak overriding a weak but with dyld the way to think of it is that if a symbol is marked weak, then all definitions of that symbol are treated as weak during weak-def coalescing. It would be just as effective to define a dummy unused weak symbol in the test program:

__attribute__((weak)) int foo;

This too would mark the dll as having a weak symbol and so then all the symbols in the test program would be seen during weak-def coalescing.

In D146471#4207971, @vitalybuka wrote:

@rsundahl Do you thing it's going to work with D146469 ?

I'm not sure exactly what you're asking vis-a-vis https://reviews.llvm.org/D146469 @vitalybuka but I think it's a subtlety. Could you elaborate? I'm adding @thetruestblue as she has been working recently on these interceptors and may spot something that I don't.

In D146471#4207971, @vitalybuka wrote:

@rsundahl Do you thing it's going to work with D146469 ?

Thanks for including me @vitalybuka... To understand why these tests have the conditional code making the overrides "weak" please see https://reviews.llvm.org/D127929 (show older changes) for the discussion. To summarize here though:
The idea of a "strong" overriding "weak" symbols is reasonably well understood and iirc is specified by the standard(s) but only in the context of static linking. The problem is complicated with dynamic linking. For a worst case thought experience, think about what should happen to weakly bound symbols when an application calls dlopen() on a dll that contains possible overrides or additional weak symbols. dyld uses two-level name space linking which essentially precludes a strong symbol in a different module from ever replacing a weak symbol, but it has a slight variation on this behavior in that during this "weak-def coalescing", it does consider any other modules containing weak definitions. The weak attributes on the "overrides" in the tests above are there only to cause the module (the test program) to be considered during weak-def coalescing at which time the weak symbol in the test program overrides the weak symbol reference from the dll. Yes, in this case it's a weak overriding a weak but with dyld the way to think of it is that if a symbol is marked weak, then all definitions of that symbol are treated as weak during weak-def coalescing. It would be just as effective to define a dummy unused weak symbol in the test program:

__attribute__((weak)) int foo;

This too would mark the dll as having a weak symbol and so then all the symbols in the test program would be seen during weak-def coalescing.

cc: @dyung and @ormris - I have modified the test to use the canonical fuzzer entry point

int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size)

rather than

int main(int argc, char *argv[])

which was causing the duplicated symbol on Debian. It's interesting that it worked for us but not for Debian but this is the correct form in any case. Thanks for your input and for the revert. (I'm going to look into how I can get a build on the failed bot before I land this time.)

Don't use main() for testing body (error on Debian).

Reopening to push changes for Debian test error (multiple main()'s).

In D146189#4202880, @dyung wrote:

@rsundahl the test you added big-file-copy.test is failing on a buildbot, can you take a look?

https://lab.llvm.org/buildbot/#/builders/247/builds/2653

Oh, interesting... Second copy of main() on a different platform. Thanks for the heads up.

Thanks @yln!

In D146189#4200276, @yln wrote:

when a crashed worker has generated >2Gb of output.

Are we fine with printing >2GB of output? That sounds like it could lead to trouble elsewhere. Since the output is coming from a file I would hope that there is a way to get to it (even for remote CI?)

The use case here is that a worker that has accumulated >2Gb of output (over what may be days or weeks) crashes before it can append its log to stderr and in crashing, doesn't do a clean exit so more data is potentially lost. I agree though, we should convey that log file to the parent some other way and I think you're right in suspecting that this large amount of output may bite us further downstream simply because we now succeed at generating it.

Do you think it would be worth investigating (as a separate follow-up) to truncate the output?

That's a good idea. Truncate the output or just don't use stderr as a pipe for large files. (Although it's Unix, right? K&R would be probably approve of a fix that removes a limit to the amount of data going through a pipe. lol)

Linting

Removed extraneous blank line.

Clean up some wording and the title.

In D144830#4186199, @dmaclach wrote:

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?

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.