Page MenuHomePhabricator

delcypher (Dan Liew)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 21 2015, 4:29 PM (303 w, 6 d)

Recent Activity

Wed, Jun 9

delcypher added a comment to D102481: [docs] Collate CMake options.

Thanks for looking at this. Ordering the CMake options makes a lot of sense. You probably should clean up the commit description to only be about the change you're making here and handle "Frequently used options" to a separate patch. Other than that LGTM.

Wed, Jun 9, 11:19 AM · Restricted Project

Thu, Jun 3

delcypher added a comment to D103304: Update and improve compiler-rt tests for -mllvm -asan_use_after_return=(never|[runtime]|always)..

@kda This change has broken our internal builds on macOS. I'm not sure why it hasn't broken the public ones. If you add a new weak symbol it needs to be added to lib/asan/weak_symbols.txt so that the linker on macOS knowns it's okay for the symbol to not be defined. We'll put up a patch to fix this shortly.

Thu, Jun 3, 10:31 PM · Restricted Project, Restricted Project
delcypher added a comment to D103304: Update and improve compiler-rt tests for -mllvm -asan_use_after_return=(never|[runtime]|always)..

Oh, I see that that was already reported over an hour ago. Reverted for now in 5c600dc6d4b75bc71dc3033f37ea187b3fd454d7

Thu, Jun 3, 10:30 PM · Restricted Project, Restricted Project
delcypher added a comment to D103304: Update and improve compiler-rt tests for -mllvm -asan_use_after_return=(never|[runtime]|always)..

@kda This change has broken our internal builds on macOS. I'm not sure why it hasn't broken the public ones. If you add a new weak symbol it needs to be added to lib/asan/weak_symbols.txt so that the linker on macOS knowns it's okay for the symbol to not be defined. We'll put up a patch to fix this shortly.

Thu, Jun 3, 4:35 PM · Restricted Project, Restricted Project

Thu, May 20

delcypher added inline comments to D100043: [lit] Fix compatibility with upstream gtest.
Thu, May 20, 10:30 PM · Restricted Project
delcypher added inline comments to D102694: [lit][gtest] Support SKIPPED tests.
Thu, May 20, 10:23 PM · Restricted Project, Restricted Project
delcypher added a comment to D102899: [lit] Print full googletest commad line.

The patch seems reasonable. I have a minor nit about the use of the % operator but other than that it seems fine.

Thu, May 20, 10:19 PM · Restricted Project

May 15 2021

delcypher added a comment to D101681: [Compiler-rt] Distinguish between testing just built runtime libraries and the libraries shipped with the compiler..

I've landed b7f60d861ad7a8d03c09c0b72277eabaa53731fb to try to unbreak the likely misconfigured https://lab.llvm.org/buildbot/#/builders/119/builds/3801 bot.

May 15 2021, 11:13 AM · Restricted Project
delcypher committed rGb7f60d861ad7: [Compiler-rt] Downgrade another fatal error to warning (authored by delcypher).
[Compiler-rt] Downgrade another fatal error to warning
May 15 2021, 11:12 AM
delcypher added a comment to D101681: [Compiler-rt] Distinguish between testing just built runtime libraries and the libraries shipped with the compiler..

I understand this patch is trying to expose unintended configurations (thanks!). I'll note that it might not be the friendliest patch to land on a Friday.
Please see the state of the sanitizer-ppc64le-linux bot: https://lab.llvm.org/buildbot/#/builders/19.

Sorry about this. I'm going to land a patch now to downgrade the fatal error to warning to unbreak the bots for now. Owner's of the bot should investigate this warning though because it points to the testing configuration being wrong.

May 15 2021, 10:56 AM · Restricted Project
delcypher committed rG7085cd2f2945: [Compiler-rt] Downgrade fatal error about unsupported test configuration (authored by delcypher).
[Compiler-rt] Downgrade fatal error about unsupported test configuration
May 15 2021, 10:55 AM
delcypher added a comment to D101681: [Compiler-rt] Distinguish between testing just built runtime libraries and the libraries shipped with the compiler..

I understand this patch is trying to expose unintended configurations (thanks!). I'll note that it might not be the friendliest patch to land on a Friday.
Please see the state of the sanitizer-ppc64le-linux bot: https://lab.llvm.org/buildbot/#/builders/19.

May 15 2021, 10:48 AM · Restricted Project

May 14 2021

delcypher committed rGad7e12226f6b: [Compiler-rt] Distinguish between testing just built runtime libraries and the… (authored by delcypher).
[Compiler-rt] Distinguish between testing just built runtime libraries and the…
May 14 2021, 6:12 PM
delcypher closed D101681: [Compiler-rt] Distinguish between testing just built runtime libraries and the libraries shipped with the compiler..
May 14 2021, 6:11 PM · Restricted Project
delcypher updated the summary of D101681: [Compiler-rt] Distinguish between testing just built runtime libraries and the libraries shipped with the compiler..
May 14 2021, 5:58 PM · Restricted Project
delcypher updated the diff for D101681: [Compiler-rt] Distinguish between testing just built runtime libraries and the libraries shipped with the compiler..

Rename option name and lit variables to address @yln 's feedback.

May 14 2021, 5:58 PM · Restricted Project
delcypher added inline comments to D101681: [Compiler-rt] Distinguish between testing just built runtime libraries and the libraries shipped with the compiler..
May 14 2021, 4:21 PM · Restricted Project
delcypher updated the diff for D101681: [Compiler-rt] Distinguish between testing just built runtime libraries and the libraries shipped with the compiler..

Address @aralisza's feedback.

May 14 2021, 3:48 PM · Restricted Project

May 13 2021

delcypher added inline comments to D101681: [Compiler-rt] Distinguish between testing just built runtime libraries and the libraries shipped with the compiler..
May 13 2021, 1:03 PM · Restricted Project
delcypher added a comment to D101681: [Compiler-rt] Distinguish between testing just built runtime libraries and the libraries shipped with the compiler..

I tried out this patch on Linux with an in-tree build (tests run without failing the new check added) and an out of tree build (testing refuses to run as expected).

May 13 2021, 12:24 PM · Restricted Project

May 11 2021

delcypher added a comment to D101681: [Compiler-rt] Distinguish between testing just built runtime libraries and the libraries shipped with the compiler..

Ping.

May 11 2021, 3:33 PM · Restricted Project
delcypher updated the summary of D101681: [Compiler-rt] Distinguish between testing just built runtime libraries and the libraries shipped with the compiler..
May 11 2021, 3:32 PM · Restricted Project
delcypher updated the diff for D101681: [Compiler-rt] Distinguish between testing just built runtime libraries and the libraries shipped with the compiler..

Simplify patch because https://reviews.llvm.org/D101682 has landed.

May 11 2021, 3:31 PM · Restricted Project

May 8 2021

delcypher accepted D101891: [compiler-rt] Handle None value when polling addr2line pipe.

LGTM

May 8 2021, 3:18 PM · Restricted Project

May 6 2021

delcypher requested changes to D101891: [compiler-rt] Handle None value when polling addr2line pipe.
May 6 2021, 10:47 AM · Restricted Project

May 4 2021

delcypher committed rG1971823ecb9e: [Driver] Fix `ToolChain::getCompilerRTPath()` to return the correct path on… (authored by delcypher).
[Driver] Fix `ToolChain::getCompilerRTPath()` to return the correct path on…
May 4 2021, 11:29 AM
delcypher closed D101682: [Driver] Fix `ToolChain::getCompilerRTPath()` to return the correct path on Apple platforms..
May 4 2021, 11:28 AM · Restricted Project
delcypher added inline comments to D101682: [Driver] Fix `ToolChain::getCompilerRTPath()` to return the correct path on Apple platforms..
May 4 2021, 11:21 AM · Restricted Project
delcypher updated the diff for D101682: [Driver] Fix `ToolChain::getCompilerRTPath()` to return the correct path on Apple platforms..

Use isOSDarwin() instead of explicitly checking Darwin OS enum values.

May 4 2021, 11:20 AM · Restricted Project
delcypher updated subscribers of D101681: [Compiler-rt] Distinguish between testing just built runtime libraries and the libraries shipped with the compiler..
May 4 2021, 11:02 AM · Restricted Project
delcypher added a comment to D101681: [Compiler-rt] Distinguish between testing just built runtime libraries and the libraries shipped with the compiler..

Do we know for sure that people are using compiler-rt test suite to test runtime libraries shipped with their compiler? I'm a bit surprised by that since it's not a case I've ever considered.

May 4 2021, 10:56 AM · Restricted Project

May 3 2021

delcypher updated the diff for D101681: [Compiler-rt] Distinguish between testing just built runtime libraries and the libraries shipped with the compiler..

Use -print-runtime-dir to support non Darwin platforms.

May 3 2021, 12:17 PM · Restricted Project

May 1 2021

delcypher retitled D101682: [Driver] Fix `ToolChain::getCompilerRTPath()` to return the correct path on Apple platforms. from [Driver] Fix `ToolChain::getCompilerRTPath()` to return the correct path on Darwin. to [Driver] Fix `ToolChain::getCompilerRTPath()` to return the correct path on Apple platforms..
May 1 2021, 7:55 AM · Restricted Project
delcypher added inline comments to D101682: [Driver] Fix `ToolChain::getCompilerRTPath()` to return the correct path on Apple platforms..
May 1 2021, 7:54 AM · Restricted Project
delcypher updated the diff for D101682: [Driver] Fix `ToolChain::getCompilerRTPath()` to return the correct path on Apple platforms..
  • Support other Apple target triples.
May 1 2021, 7:53 AM · Restricted Project

Apr 30 2021

delcypher added inline comments to D101681: [Compiler-rt] Distinguish between testing just built runtime libraries and the libraries shipped with the compiler..
Apr 30 2021, 7:28 PM · Restricted Project
delcypher requested review of D101682: [Driver] Fix `ToolChain::getCompilerRTPath()` to return the correct path on Apple platforms..
Apr 30 2021, 7:20 PM · Restricted Project
delcypher added inline comments to D101681: [Compiler-rt] Distinguish between testing just built runtime libraries and the libraries shipped with the compiler..
Apr 30 2021, 6:07 PM · Restricted Project
delcypher abandoned D101335: [ASan][Darwin] Fix `TestCases/Darwin/init_for_dlopen.cpp` when running in a standalone build..

I'm abandoning this change. It is superseded by the much more general https://reviews.llvm.org/D101681

Apr 30 2021, 5:18 PM · Restricted Project
delcypher requested review of D101681: [Compiler-rt] Distinguish between testing just built runtime libraries and the libraries shipped with the compiler..
Apr 30 2021, 5:17 PM · Restricted Project

Apr 29 2021

delcypher committed rG2d42b2ee7baf: [ASan] Rename `-fsanitize-address-destructor-kind=` to drop the `-kind` suffix. (authored by delcypher).
[ASan] Rename `-fsanitize-address-destructor-kind=` to drop the `-kind` suffix.
Apr 29 2021, 11:56 AM
delcypher closed D101491: [ASan] Rename `-fsanitize-address-destructor-kind=` to drop the `-kind` suffix..
Apr 29 2021, 11:56 AM · Restricted Project
delcypher added a comment to D101491: [ASan] Rename `-fsanitize-address-destructor-kind=` to drop the `-kind` suffix..

@jansvoboda11 I'm going to land this patch as is (with your nit fixed). If you would like me to add an alias please follow up with me and I can put up a patch to do that.

Apr 29 2021, 11:55 AM · Restricted Project
delcypher updated the diff for D101491: [ASan] Rename `-fsanitize-address-destructor-kind=` to drop the `-kind` suffix..

Rename def too.

Apr 29 2021, 11:54 AM · Restricted Project
delcypher added a comment to D101491: [ASan] Rename `-fsanitize-address-destructor-kind=` to drop the `-kind` suffix..

@jansvoboda11 Should I use the the alias feature so that the old -fsanitize-address-destructor-kind argument is still parsed? I'm not sure if it's worth it but if doing so is very simple and has a low maintenance burden we should probably do it.

I'm fine with omitting the alias if the original flag didn't make it into a release and it's unlikely that downstream TOT users are using it.

Apr 29 2021, 11:02 AM · Restricted Project

Apr 28 2021

delcypher updated the diff for D101491: [ASan] Rename `-fsanitize-address-destructor-kind=` to drop the `-kind` suffix..

Remove metavar

Apr 28 2021, 7:02 PM · Restricted Project
delcypher committed rG1bbbcff99de8: [NFC] Rename SanitizeAddressDtorKind codegen opt to not have `Kind` suffix. (authored by delcypher).
[NFC] Rename SanitizeAddressDtorKind codegen opt to not have `Kind` suffix.
Apr 28 2021, 6:37 PM
delcypher closed D101490: [NFC] Rename SanitizeAddressDtorKind codegen opt to not have `Kind` suffix..
Apr 28 2021, 6:37 PM · Restricted Project
delcypher added a comment to D101491: [ASan] Rename `-fsanitize-address-destructor-kind=` to drop the `-kind` suffix..

@jansvoboda11 Should I use the the alias feature so that the old -fsanitize-address-destructor-kind argument is still parsed? I'm not sure if it's worth it but if doing so is very simple and has a low maintenance burden we should probably do it.

Apr 28 2021, 2:41 PM · Restricted Project
delcypher added inline comments to D101122: introduce flag -fsanitize-address-detect-stack-use-after-return-mode. No functional change..
Apr 28 2021, 2:37 PM · Restricted Project, Restricted Project
delcypher requested review of D101491: [ASan] Rename `-fsanitize-address-destructor-kind=` to drop the `-kind` suffix..
Apr 28 2021, 2:32 PM · Restricted Project
delcypher requested review of D101490: [NFC] Rename SanitizeAddressDtorKind codegen opt to not have `Kind` suffix..
Apr 28 2021, 2:31 PM · Restricted Project
delcypher added inline comments to D101122: introduce flag -fsanitize-address-detect-stack-use-after-return-mode. No functional change..
Apr 28 2021, 9:57 AM · Restricted Project, Restricted Project
delcypher added a comment to D101335: [ASan][Darwin] Fix `TestCases/Darwin/init_for_dlopen.cpp` when running in a standalone build..

@yln Your comments have convinced me that I should try doing this a different way. I'll try the approach I outlined and I'll see how far I get.

Apr 28 2021, 9:50 AM · Restricted Project

Apr 27 2021

delcypher added a comment to D101335: [ASan][Darwin] Fix `TestCases/Darwin/init_for_dlopen.cpp` when running in a standalone build..

Would it be possible (and wouldn't it be better?) to set config.compiler_rt_libdir to the right path in the first place? This way we wouldn't get into the "path mismatch" state that we warn about.

Apr 27 2021, 5:15 PM · Restricted Project
delcypher added inline comments to D101335: [ASan][Darwin] Fix `TestCases/Darwin/init_for_dlopen.cpp` when running in a standalone build..
Apr 27 2021, 12:28 PM · Restricted Project

Apr 26 2021

delcypher requested review of D101335: [ASan][Darwin] Fix `TestCases/Darwin/init_for_dlopen.cpp` when running in a standalone build..
Apr 26 2021, 5:26 PM · Restricted Project

Apr 23 2021

delcypher added a comment to D89493: [lit] Implement `not` as a builtin in the Lit internal shell.

LGTM other than my nits.

Apr 23 2021, 10:21 PM · Restricted Project
delcypher added a reviewer for D89493: [lit] Implement `not` as a builtin in the Lit internal shell: yln.
Apr 23 2021, 10:21 PM · Restricted Project

Apr 22 2021

delcypher added a comment to D100998: [sanitizer] Use COMPILER_RT_EMULATOR with gtests.

@vitalybuka Damn looks like you landed the patch while I was in the middle of writing a review. Could you make the fix I requested as a follow up patch?

sure, sorry, I didn't noticed comments.
I even can revert if there is serious issues.

Apr 22 2021, 11:16 AM · Restricted Project, Restricted Project
delcypher added a comment to D100998: [sanitizer] Use COMPILER_RT_EMULATOR with gtests.

@vitalybuka Damn looks like you landed the patch while I was in the middle of writing a review. Could you make the fix I requested as a follow up patch?

Apr 22 2021, 10:44 AM · Restricted Project, Restricted Project
delcypher added a comment to D100998: [sanitizer] Use COMPILER_RT_EMULATOR with gtests.

The approach looks good but I'd like to see us type check the new run_under parameter.

Apr 22 2021, 10:43 AM · Restricted Project, Restricted Project
delcypher added a comment to D100998: [sanitizer] Use COMPILER_RT_EMULATOR with gtests.

@vitalybuka Thanks for tackling this. I've always wanted to have our unit tests be runnable on targets that aren't the host. This looks like the first step towards that.

Apr 22 2021, 10:34 AM · Restricted Project, Restricted Project
delcypher added reviewers for D100998: [sanitizer] Use COMPILER_RT_EMULATOR with gtests: delcypher, yln.
Apr 22 2021, 10:32 AM · Restricted Project, Restricted Project

Apr 21 2021

delcypher accepted D100784: [compiler-rt] check max address from kernel is <= mmap range size.

LGTM apart from the description. The description probably should mention this is the runtime counterpart to the previous patch that added static checks. We have to do this check at runtime because we get the VM size from the kernel at runtime.

Apr 21 2021, 11:31 AM · Restricted Project

Apr 20 2021

delcypher committed rG6f4f0afaa8ae: [Compiler-rt] Fix bug when considering CMake path returned by llvm-config. (authored by delcypher).
[Compiler-rt] Fix bug when considering CMake path returned by llvm-config.
Apr 20 2021, 11:57 AM

Apr 19 2021

delcypher accepted D100239: [compiler-rt] assert max virtual address is <= mmap range size.
Apr 19 2021, 12:56 PM · Restricted Project
delcypher added inline comments to D100784: [compiler-rt] check max address from kernel is <= mmap range size.
Apr 19 2021, 12:55 PM · Restricted Project
delcypher added a comment to D100239: [compiler-rt] assert max virtual address is <= mmap range size.

LGTM

Apr 19 2021, 12:52 PM · Restricted Project
delcypher added a comment to D100239: [compiler-rt] assert max virtual address is <= mmap range size.

LGTM apart from the static keyword which "I think" shouldn't be necessary here.

Apr 19 2021, 12:50 PM · Restricted Project

Apr 16 2021

delcypher requested changes to D100239: [compiler-rt] assert max virtual address is <= mmap range size.
Apr 16 2021, 12:48 PM · Restricted Project
delcypher accepted D100234: [compiler-rt][asan] use full vm range on apple silicon macs.

LGTM

Apr 16 2021, 12:32 PM · Restricted Project

Apr 13 2021

delcypher committed rG4c0bc69490a5: Ship `llvm-cxxfilt` in the toolchain. (authored by delcypher).
Ship `llvm-cxxfilt` in the toolchain.
Apr 13 2021, 11:59 AM
delcypher closed D100405: Ship `llvm-cxxfilt` in the toolchain..
Apr 13 2021, 11:58 AM · Restricted Project
delcypher requested review of D100405: Ship `llvm-cxxfilt` in the toolchain..
Apr 13 2021, 11:56 AM · Restricted Project

Apr 12 2021

delcypher added a comment to D100234: [compiler-rt][asan] use full vm range on apple silicon macs.

LGTM. Do we have a test that fails without this, but succeeds with it (on AS)?

Apr 12 2021, 3:38 PM · Restricted Project
delcypher requested changes to D100234: [compiler-rt][asan] use full vm range on apple silicon macs.
Apr 12 2021, 3:32 PM · Restricted Project

Apr 9 2021

delcypher added a comment to D100157: [compiler-rt] add SANITIZER_OSX.

Just floating this as on option. We could call the new define SANITIZER_MACOS.

Apr 9 2021, 2:41 PM · Restricted Project
delcypher added a comment to D100157: [compiler-rt] add SANITIZER_OSX.

@aralisza You can ignore the lint checks here.

Apr 9 2021, 2:37 PM · Restricted Project

Apr 8 2021

delcypher added a comment to D100157: [compiler-rt] add SANITIZER_OSX.

I'm not a big fan of the name but the sensible name (SANTIZER_MAC) is already taken unfortunately.

Apr 8 2021, 11:32 PM · Restricted Project
delcypher committed rGf66e05a720f7: Include `count` in AppleClang toolchains. (authored by delcypher).
Include `count` in AppleClang toolchains.
Apr 8 2021, 2:01 PM
delcypher closed D100087: Include `count` in AppleClang toolchains..
Apr 8 2021, 2:00 PM · Restricted Project

Apr 7 2021

delcypher accepted D96295: [Darwin] enable duplicate_os_log_reports lit test.
Apr 7 2021, 11:23 PM · Restricted Project
delcypher added a comment to D96295: [Darwin] enable duplicate_os_log_reports lit test.

@aralisza Sorry I missed this patch. LGTM.

Apr 7 2021, 11:23 PM · Restricted Project
delcypher requested review of D100087: Include `count` in AppleClang toolchains..
Apr 7 2021, 9:38 PM · Restricted Project
delcypher committed rG73cbc7f60ed9: Include `llvm-config` and `not` in AppleClang toolchains. (authored by delcypher).
Include `llvm-config` and `not` in AppleClang toolchains.
Apr 7 2021, 9:30 PM
delcypher closed D100086: Include `llvm-config` and `not` in AppleClang toolchains..
Apr 7 2021, 9:30 PM · Restricted Project
delcypher requested review of D100086: Include `llvm-config` and `not` in AppleClang toolchains..
Apr 7 2021, 9:24 PM · Restricted Project

Apr 6 2021

delcypher added a comment to D99434: [TSAN] Honor failure memory orders in AtomicCAS.

@bruno Thanks for the patch. TSan's runtime isn't my specialty so I've added other reviewers.

Apr 6 2021, 8:52 AM · Restricted Project, Restricted Project
delcypher added a reviewer for D99434: [TSAN] Honor failure memory orders in AtomicCAS: kubamracek.
Apr 6 2021, 8:45 AM · Restricted Project, Restricted Project
delcypher added a reviewer for D99434: [TSAN] Honor failure memory orders in AtomicCAS: aralisza.
Apr 6 2021, 8:45 AM · Restricted Project, Restricted Project
delcypher added a comment to D99621: [CMake][Compiler-rt] Make it possible to configure standalone compiler-rt without `LLVMConfig.cmake`..

LGTM (I've tested this together with D99621 locally for the Fuchsia build and everything seems to be working).

Apr 6 2021, 8:34 AM · Restricted Project
delcypher added a comment to D99620: [CMake][Compiler-rt] Compute LLVM_MAIN_SRC_DIR assuming the monorepo layout..

@phosek Thanks for the review.

Apr 6 2021, 8:34 AM · Restricted Project
delcypher committed rGfd28517d878e: [CMake][Compiler-rt] Make it possible to configure standalone compiler-rt… (authored by delcypher).
[CMake][Compiler-rt] Make it possible to configure standalone compiler-rt…
Apr 6 2021, 8:33 AM
delcypher committed rGfa818bb0357d: [CMake][Compiler-rt] Compute `LLVM_MAIN_SRC_DIR` assuming the monorepo (authored by delcypher).
[CMake][Compiler-rt] Compute `LLVM_MAIN_SRC_DIR` assuming the monorepo
Apr 6 2021, 8:33 AM
delcypher closed D99621: [CMake][Compiler-rt] Make it possible to configure standalone compiler-rt without `LLVMConfig.cmake`..
Apr 6 2021, 8:33 AM · Restricted Project
delcypher closed D99620: [CMake][Compiler-rt] Compute LLVM_MAIN_SRC_DIR assuming the monorepo layout..
Apr 6 2021, 8:33 AM · Restricted Project
delcypher updated the diff for D99620: [CMake][Compiler-rt] Compute LLVM_MAIN_SRC_DIR assuming the monorepo layout..
  • Renamed CRT_SRC_ROOT_PATH to COMPILER_RT_ROOT_SRC_PATH
Apr 6 2021, 8:26 AM · Restricted Project
delcypher accepted D99911: [Sanitizer] Adopt Python 3 for iOS simulator test scripts.

LGTM

Apr 6 2021, 8:09 AM · Restricted Project

Apr 4 2021

delcypher added a comment to D99690: [builtins] Build for arm64_32 for watchOS (Darwin).

The code in compiler-rt/cmake/builtin-config-ix.cmake is reasonable. Although we (Apple) should probably upstream our change because it's slightly different. I'm not sure about the change in compiler-rt/lib/builtins/CMakeLists.txt we don't have the change you made. I'll put a patch up to upstream our version and we'll see how it goes.

Apr 4 2021, 9:14 AM · Restricted Project