[PATCH] D158570: [Darwin][StableABI][ASan] Remove version mismatch check from stable abi shim

rsundahl accepted this revision. rsundahl added a comment. This revision is now accepted and ready to land. LGTM since we ship the shim with the toolchain. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158570/new/ https://reviews.llvm.org/D158570

[PATCH] D125195: [asan][ARMCXXABI] Added missing asan poison array cookie hooks.

rsundahl updated this revision to Diff 429135. rsundahl added a comment. - Refactor InitializeArrayCookie and readArrayCookieImpl. This update to the differential implements the final suggestions for refactoring ItaniumCXXABI to remove duplicated code in the function InitializeArrayCookie() and

[PATCH] D125195: [asan][ARMCXXABI] Added missing asan poison array cookie hooks.

rsundahl marked 8 inline comments as done. rsundahl added a comment. The update completes the suggested changes. The generated code is slightly different around initialization of the array cookie due to choosing one implementation over another when I "folded" ARMCXXABI::InitializeArrayCookie()

[PATCH] D125195: [asan][ARMCXXABI] Added missing asan poison array cookie hooks.

rsundahl updated this revision to Diff 430461. rsundahl added a comment. Fix (and cleanup) for failure of CodeGen arm.c check in ci/cd pipeline. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125195/new/ https://reviews.llvm.org/D125195 Files:

[PATCH] D125195: [asan][ARMCXXABI] Added missing asan poison array cookie hooks.

rsundahl added inline comments. Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2191 CGM.CreateRuntimeFunction(FTy, "__asan_poison_cxx_array_cookie"); +// CGF.Builder.CreateCall(F, NumElementsPtr.getRawPointer(CGF)); CGF.Builder.CreateCall(F,

[PATCH] D125195: [asan][ARMCXXABI] Added missing asan poison array cookie hooks.

rsundahl updated this revision to Diff 429352. rsundahl added a comment. This update corrects merge conflicts in Build #104091 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125195/new/ https://reviews.llvm.org/D125195 Files:

[PATCH] D125195: [asan][ARMCXXABI] Added missing asan poison array cookie hooks.

rsundahl marked an inline comment as not done. rsundahl added inline comments. Comment at: compiler-rt/test/asan/TestCases/Posix/new_array_cookie_test.cpp:19 C *buffer = new C[argc]; - buffer[-2].x = 10; + buffer[-1].x = 10; // CHECK: AddressSanitizer: heap-buffer-overflow

[PATCH] D125195: [asan][ARMCXXABI] Added missing asan poison array cookie hooks.

rsundahl created this revision. rsundahl added reviewers: yln, kubamracek, rjmccall, dcoughlin, delcypher, aralisza, thetruestblue, wrotki. Herald added subscribers: pengfei, kristof.beyls. Herald added a project: All. rsundahl requested review of this revision. Herald added projects: clang,

[PATCH] D125195: [asan][ARMCXXABI] Added missing asan poison array cookie hooks.

rsundahl added reviewers: vitalybuka, kcc. rsundahl added a comment. Adding Vitaly Buka and Kostya Serebryany (sanitizer maintainers). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125195/new/ https://reviews.llvm.org/D125195

[PATCH] D125195: [asan][ARMCXXABI] Added missing asan poison array cookie hooks.

rsundahl updated this revision to Diff 428099. rsundahl added a comment. Adding ASAN test "new_array_cookie_with_new_from_class" (rdar://92884511) This test "new_array_cookie_with_new_from_class " was dependent on the cookie size on x86_64 and was either unsupported or expected to fail on arm.

[PATCH] D125195: [asan][ARMCXXABI] Added missing asan poison array cookie hooks.

rsundahl added a comment. Adding dialog to comments made by reviewers. Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2443 + // Handle poisoning the array cookie in asan + if (CGM.getLangOpts().Sanitize.has(SanitizerKind::Address) && AS == 0 && +

[PATCH] D125195: [asan][ARMCXXABI] Added missing asan poison array cookie hooks.

rsundahl updated this revision to Diff 428718. rsundahl added a comment. Revert ItaniumCXXABI.cpp for now (unintentional push) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125195/new/ https://reviews.llvm.org/D125195 Files:

[PATCH] D125195: [asan][ARMCXXABI] Added missing asan poison array cookie hooks.

rsundahl updated this revision to Diff 428716. rsundahl added a comment. Implement suggestions from reviews. (Incremental update.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125195/new/ https://reviews.llvm.org/D125195 Files:

[PATCH] D125195: [asan][ARMCXXABI] Added missing asan poison array cookie hooks.

rsundahl updated this revision to Diff 428731. rsundahl marked 6 inline comments as done. rsundahl added a comment. Update diff back to dc7e02d4b4dc30d44ddebd832719a6e63396e718 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125195/new/

[PATCH] D140224: [Driver] Revert D139717 and add -Xparser instead

rsundahl added a comment. > I'll reject [-\Xparser for a while as well. This is a valid amendment to > D139717 , so I don't think it needs more > approval. We have projects that are failing because of -Xlinker and I'm not too excited about walking through

[PATCH] D125195: [asan][ARMCXXABI] Added missing asan poison array cookie hooks.

rsundahl added a comment. In D125195#3979478 , @vitalybuka wrote: > Is this still relevant? > If so, I would recommend to split ItaniumCXXABI from asan changes. I haven't proceeded with this because this would be ABI breaking. I intend to support the

[PATCH] D139717: Revert "[Driver] Remove Joined -X"

rsundahl created this revision. Herald added a project: All. rsundahl requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This change is breaking internal builds. We use the -Xfoo pattern but can now no longer manage whether we allow an unused

[PATCH] D139652: Add the thread sanitizer support for X86_64 WatchOS simulators

rsundahl added a comment. In D139652#3983079 , @yln wrote: > @rsundahl: can you keep an eye on this and resolve any potential > Apple-internal conflicts? Thanks! Will do @yln. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D139717: Revert "[Driver] Remove Joined -X"

This revision was automatically updated to reflect the committed changes. Closed by commit rG7f8bd8ac0658: Revert [Driver] Remove Joined -X (authored by rsundahl). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139717/new/

[PATCH] D139717: Revert "[Driver] Remove Joined -X"

rsundahl updated this revision to Diff 483297. rsundahl added a comment. Added the test warn-not-error-Xfoo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139717/new/ https://reviews.llvm.org/D139717 Files: clang/include/clang/Driver/Options.td

[PATCH] D139717: Revert "[Driver] Remove Joined -X"

rsundahl added a comment. I added a test so we can catch any regression. If we choose to deprecate then we can modify the test to watch for the "is deprecated" message that should be asserted for "some time" appropriate. Thank you all for your input. Repository: rG LLVM Github Monorepo

[PATCH] D139717: Revert "[Driver] Remove Joined -X"

rsundahl added a comment. In D139717#3987963 , @MaskRay wrote: >> This change is breaking internal builds. We use the -Xfoo pattern but can >> now no longer manage whether we allow an unused -Xfoo option to pass as a >> warning or promote it to an

[PATCH] D137992: [asan] -fsanitize-address-outline-instrumentation -> -asan-max-inline-poisoning-size=0

rsundahl updated this revision to Diff 475497. rsundahl added a comment. Removed extraneous blank line. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137992/new/ https://reviews.llvm.org/D137992 Files: clang/lib/Driver/SanitizerArgs.cpp

[PATCH] D142421: [Sanitizers] fix -fno-sanitize-link-runtime for darwin

rsundahl accepted this revision. rsundahl added a comment. LGTM as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142421/new/ https://reviews.llvm.org/D142421 ___ cfe-commits mailing list

[PATCH] D137992: [asan] -fsanitize-address-outline-instrumentation -> -asan-max-inline-poisoning-size=0

rsundahl created this revision. Herald added a project: All. rsundahl requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. The clang flag -fsanitize-address-outline-instrumentation should not only pass

[PATCH] D145715: Remove -lower-global-dtors-via-cxa-atexit flag

rsundahl added inline comments. Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1205 unsigned Priority, const MCSymbol *KeySym) const { - // TODO(yln): Remove -lower-global-dtors-via-cxa-atexit fallback flag - // (LowerGlobalDtorsViaCxaAtExit) and always

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

rsundahl added a comment. In D143675#4160084 , @vitalybuka wrote: > Usually freezing signatures is not a big concern, we can agree to preserve > existing functions. > The stuff like ASanStackFrameLayout is the concern. compiler and runtime must >

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

rsundahl updated this revision to Diff 501308. rsundahl added a comment. Herald added subscribers: cfe-commits, luke, yaneury, supersymetrie, Chia-hungDuan, pcwang-thead, frasercrmck, luismarques, apazos, sameer.abuasal, s.egerton, Jim, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01,

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

rsundahl added a comment. Added to Discourse: https://discourse.llvm.org/t/darwin-sanitizers-stable-abi/68834 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143675/new/ https://reviews.llvm.org/D143675

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

rsundahl updated this revision to Diff 501358. rsundahl added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143675/new/ https://reviews.llvm.org/D143675 Files: clang/include/clang/Driver/Options.td

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

rsundahl updated this revision to Diff 501357. rsundahl added a comment. Deleted extraneous line from compiler-rt/cmake/config-ix.cmake Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143675/new/ https://reviews.llvm.org/D143675 Files:

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

rsundahl updated this revision to Diff 515559. rsundahl added a comment. Rename fsanitize_address_stable_abi to fsanitize_stable_abi Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143675/new/ https://reviews.llvm.org/D143675 Files:

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

rsundahl marked an inline comment as done. rsundahl added inline comments. Comment at: clang/include/clang/Driver/Options.td:1785 HelpText<"Use default code inlining logic for the address sanitizer">; +def

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

rsundahl added inline comments. Comment at: compiler-rt/lib/asabi/CMakeLists.txt:2 +# Build for the ASAN Stable ABI runtime support library. +set(ASABI_SOURCES + asabi_shim.cpp vitalybuka wrote: > does it need to be asabi? > maybe better asan_abi, files and

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

rsundahl added a comment. @kcc @eugenis @MaskRay @vitalybuka Ok to go with this? All new functionality is under the added flag so not expecting any surprises. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143675/new/

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

rsundahl updated this revision to Diff 518552. rsundahl added a comment. Rename asabi->asan_abi Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143675/new/ https://reviews.llvm.org/D143675 Files: clang/include/clang/Driver/Options.td

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

rsundahl marked an inline comment as done. rsundahl added a comment. @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. Comment at:

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

rsundahl added a comment. Hello Egenii, Thank you for your time and consideration of this PR. Since you last commented, @vitalybuka has approved the PR and added @maskray and yourself as blocking reviewers. @maskray has approved and we are awaiting your approval if you remain positive to it

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

rsundahl updated this revision to Diff 525644. rsundahl added a comment. Apply proper backticks quotes to options. Remove redundant ellipses. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143675/new/ https://reviews.llvm.org/D143675 Files:

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6f026ff02985: Discussion: Darwin Sanitizers Stable ABI (authored by rsundahl). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

rsundahl marked 2 inline comments as done. rsundahl added a comment. Thanks for your time and guidance getting this landed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143675/new/ https://reviews.llvm.org/D143675

[PATCH] D156718: [ASanStableABI][Driver] Stop linking to asan dylib when stable abi is enabled

rsundahl accepted this revision. rsundahl added a comment. This revision is now accepted and ready to land. This looks good to me. Thanks for the additional clean-up @thetruestblue. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156718/new/ https://reviews.llvm.org/D156718

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

rsundahl added a comment. 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. Right, we should be using it... We will add a test that

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

rsundahl added a comment. 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/ I've not seen this

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

rsundahl marked 4 inline comments as done. rsundahl added a comment. Suggestions for compiler-rt/docs/asan_abi.md are captured in the successor file compiler-rt/docs/ASanABI.rst and marked complete. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

rsundahl marked 2 inline comments as done. rsundahl added a comment. Suggestions for compiler-rt/docs/asan_abi.md are captured in the successor file compiler-rt/docs/ASanABI.rst and marked complete. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

rsundahl updated this revision to Diff 522452. rsundahl added a comment. Missed one file in revert of combined -mllvm= change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143675/new/ https://reviews.llvm.org/D143675 Files:

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

rsundahl updated this revision to Diff 522417. rsundahl added a comment. Applied suggestions from reviewers Cleaned up options parsing Moved test into stanalone file fsanitize-stable-abi.c Changed target triple to arm64-apple-darwin Changed documentation style from proposal to specification

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

rsundahl marked 22 inline comments as done. rsundahl added a comment. 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

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

rsundahl updated this revision to Diff 522592. rsundahl added a comment. Renamed darwin_exclude_symbols.inc asan_abi_tbd.txt. This file contains the entrypoints that aren't strictly in the interface between the instrumentation and the runtime but may still be part of a public API that needs to

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

rsundahl updated this revision to Diff 521479. rsundahl added a comment. Added testing. Removed asan_abi_shim.h. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143675/new/ https://reviews.llvm.org/D143675 Files:

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

rsundahl added a comment. 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. We now use it during testing to close the loop on the

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

rsundahl updated this revision to Diff 521482. rsundahl added a comment. Fixed nits (missing newlines at end of files) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143675/new/ https://reviews.llvm.org/D143675 Files:

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

rsundahl added a comment. Ping @kcc , @yln and @kubamracek Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143675/new/ https://reviews.llvm.org/D143675 ___ cfe-commits mailing list

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

rsundahl added a comment. @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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

rsundahl updated this revision to Diff 523409. rsundahl added a comment. Implement suggestions from latest review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143675/new/ https://reviews.llvm.org/D143675 Files:

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

rsundahl marked an inline comment as done. rsundahl added inline comments. Comment at: compiler-rt/docs/ASanABI.rst:29 + + Following are some examples of reasonable responses to such changes: + MaskRay wrote: > How does the 2-space indentation render in the