delcypher (Dan Liew)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Mon, Aug 13

delcypher committed rCRT339559: [CMake] Fix bug in `add_weak_symbols()` function..
[CMake] Fix bug in `add_weak_symbols()` function.
Mon, Aug 13, 6:02 AM
delcypher added a comment to D50059: [CMake] Fix bug in `add_weak_symbols()` function..

@george.karpenkov Thanks for the review.

Mon, Aug 13, 6:00 AM
delcypher committed rL339559: [CMake] Fix bug in `add_weak_symbols()` function..
[CMake] Fix bug in `add_weak_symbols()` function.
Mon, Aug 13, 6:00 AM
delcypher closed D50059: [CMake] Fix bug in `add_weak_symbols()` function..
Mon, Aug 13, 6:00 AM

Mon, Aug 6

delcypher requested changes to D50275: [sanitizer] When setting up shadow memory on iOS, fix handling the return value of task_info on older OS versions.
Mon, Aug 6, 6:26 AM · Restricted Project
delcypher created D50330: [RFC] Implement out-of-process allocator enumeration for macOS.
Mon, Aug 6, 6:10 AM · Restricted Project

Tue, Jul 31

delcypher created D50059: [CMake] Fix bug in `add_weak_symbols()` function..
Tue, Jul 31, 6:24 AM

Thu, Jul 26

delcypher added inline comments to D48445: [sanitizer] Use "fast mmap" kernel flag for shadow memory on macOS 10.13.4+.
Thu, Jul 26, 3:56 AM · Restricted Project

Wed, Jul 25

delcypher added inline comments to D48445: [sanitizer] Use "fast mmap" kernel flag for shadow memory on macOS 10.13.4+.
Wed, Jul 25, 8:38 AM · Restricted Project

Tue, Jul 24

delcypher accepted D49707: [tsan] Fix crash in objc_sync_enter/objc_sync_exit when using an Obj-C tagged pointer.

Other than minor suggestions the basic idea seems okay for now.

Tue, Jul 24, 8:26 AM · Restricted Project

Fri, Jul 20

delcypher accepted D15225: [Driver] Sanitizer support based on runtime library presence.
Fri, Jul 20, 3:34 PM
delcypher added a comment to D15225: [Driver] Sanitizer support based on runtime library presence.

@george.karpenkov Other than the comment that probably needs updating, LGTM.

Fri, Jul 20, 3:34 PM
delcypher added inline comments to D15225: [Driver] Sanitizer support based on runtime library presence.
Fri, Jul 20, 3:34 PM
delcypher added inline comments to D15225: [Driver] Sanitizer support based on runtime library presence.
Fri, Jul 20, 12:25 PM
delcypher committed rL337579: On Darwin switch from the `VM_MEMORY_ANALYSIS_TOOL` VM tag to.
On Darwin switch from the `VM_MEMORY_ANALYSIS_TOOL` VM tag to
Fri, Jul 20, 10:12 AM
delcypher committed rCRT337579: On Darwin switch from the `VM_MEMORY_ANALYSIS_TOOL` VM tag to.
On Darwin switch from the `VM_MEMORY_ANALYSIS_TOOL` VM tag to
Fri, Jul 20, 10:12 AM
delcypher closed D49603: On Darwin switch from the `VM_MEMORY_ANALYSIS_TOOL` VM tag to `VM_MEMORY_SANITIZER`.
Fri, Jul 20, 10:12 AM
delcypher added inline comments to D48445: [sanitizer] Use "fast mmap" kernel flag for shadow memory on macOS 10.13.4+.
Fri, Jul 20, 10:10 AM · Restricted Project
delcypher updated the diff for D49603: On Darwin switch from the `VM_MEMORY_ANALYSIS_TOOL` VM tag to `VM_MEMORY_SANITIZER`.

Update spacing as requested.

Fri, Jul 20, 10:09 AM
delcypher created D49603: On Darwin switch from the `VM_MEMORY_ANALYSIS_TOOL` VM tag to `VM_MEMORY_SANITIZER`.
Fri, Jul 20, 8:40 AM

Tue, Jul 17

delcypher added inline comments to D15225: [Driver] Sanitizer support based on runtime library presence.
Tue, Jul 17, 3:00 PM
delcypher requested changes to D15225: [Driver] Sanitizer support based on runtime library presence.

Seems mostly fine apart from some minor nits.

Tue, Jul 17, 2:50 PM

Jul 13 2018

delcypher committed rCRT336977: Remove `tsan/Darwin/gcd-after-null.mm` test..
Remove `tsan/Darwin/gcd-after-null.mm` test.
Jul 13 2018, 12:42 AM
delcypher committed rL336977: Remove `tsan/Darwin/gcd-after-null.mm` test..
Remove `tsan/Darwin/gcd-after-null.mm` test.
Jul 13 2018, 12:42 AM
delcypher closed D49269: Remove `tsan/Darwin/gcd-after-null.mm` test..
Jul 13 2018, 12:42 AM

Jul 12 2018

delcypher created D49269: Remove `tsan/Darwin/gcd-after-null.mm` test..
Jul 12 2018, 2:29 PM
delcypher committed rCRT336944: [CMake] Fix a typo in the variable used to retrieve source file names.
[CMake] Fix a typo in the variable used to retrieve source file names
Jul 12 2018, 2:00 PM
delcypher committed rL336944: [CMake] Fix a typo in the variable used to retrieve source file names.
[CMake] Fix a typo in the variable used to retrieve source file names
Jul 12 2018, 2:00 PM
delcypher closed D49178: [CMake] Fix a typo in the variable used to retrieve source file names for the `RTHwasan_dynamic` target..
Jul 12 2018, 2:00 PM
delcypher added a comment to D44579: [asan] Clean up some confusing code in `test/asan/TestCases/Darwin/segv_read_write.c` .

Actually, it's even simpler that that. The access is always valid (from the shadow's perspective). We're just trapping on a regular SEGV and the signal handler gets the information whether this is a read or a write.

Jul 12 2018, 9:49 AM
delcypher added a comment to D44579: [asan] Clean up some confusing code in `test/asan/TestCases/Darwin/segv_read_write.c` .

The comment makes sense to me. Writes are instrumented with reads from shadow in ASan. Therefore a write to addr in shadow will result in a read from shadow(addr), which is located in the mprotect-ed shadow gap, and will be reported as a read SEGV, not a write SEGV.

mmap() argument change looks fine.

Thanks for that explanation. That makes more sense now. I'm going to integrate your explanation into the comment and merge.

Jul 12 2018, 9:05 AM
delcypher added a comment to D44579: [asan] Clean up some confusing code in `test/asan/TestCases/Darwin/segv_read_write.c` .

The comment makes sense to me. Writes are instrumented with reads from shadow in ASan. Therefore a write to addr in shadow will result in a read from shadow(addr), which is located in the mprotect-ed shadow gap, and will be reported as a read SEGV, not a write SEGV.

mmap() argument change looks fine.

Jul 12 2018, 6:55 AM
delcypher committed rCRT336905: [CMake] Remove unnecesary list of source files for Xray unit tests..
[CMake] Remove unnecesary list of source files for Xray unit tests.
Jul 12 2018, 6:53 AM
delcypher committed rL336905: [CMake] Remove unnecesary list of source files for Xray unit tests..
[CMake] Remove unnecesary list of source files for Xray unit tests.
Jul 12 2018, 6:53 AM
delcypher closed D49177: [CMake] Remove unnecesary list of source files for Xray unit tests..
Jul 12 2018, 6:53 AM
delcypher added a comment to D49177: [CMake] Remove unnecesary list of source files for Xray unit tests..

I'm fine with landing this then iterating, no need for me to test myself.

Jul 12 2018, 6:50 AM
delcypher added a comment to D49177: [CMake] Remove unnecesary list of source files for Xray unit tests..

LGTM -- @nglevin was working on getting the XRay unit tests at least build and run but got interrupted.

The original reason for doing this was to ensure that when any of the implementation files changed, that the unit tests would be built. As you've learned this only made sense for platforms where the tests are built and run.

Jul 12 2018, 6:47 AM
delcypher committed rCRT336904: [CMake] Rename `SANITIZER_HEADERS` to `SANITIZER_IMPL_HEADERS` under….
[CMake] Rename `SANITIZER_HEADERS` to `SANITIZER_IMPL_HEADERS` under…
Jul 12 2018, 6:42 AM
delcypher committed rL336904: [CMake] Rename `SANITIZER_HEADERS` to `SANITIZER_IMPL_HEADERS` under….
[CMake] Rename `SANITIZER_HEADERS` to `SANITIZER_IMPL_HEADERS` under…
Jul 12 2018, 6:42 AM
delcypher added a comment to D49176: [CMake] Rename `SANITIZER_HEADERS` to `SANITIZER_IMPL_HEADERS` under `lib/sanitizer_common`..

@george.karpenkov Thanks for approving.

Jul 12 2018, 6:42 AM
delcypher closed D49176: [CMake] Rename `SANITIZER_HEADERS` to `SANITIZER_IMPL_HEADERS` under `lib/sanitizer_common`..
Jul 12 2018, 6:41 AM

Jul 11 2018

delcypher added a comment to D49178: [CMake] Fix a typo in the variable used to retrieve source file names for the `RTHwasan_dynamic` target..

@eugenis Could you test this? HWAsan doesn't build on macOS right now so I can't test it. I spotted this typo when doing a recent change so I though I'd send a patch your way.

Jul 11 2018, 6:03 AM
delcypher created D49178: [CMake] Fix a typo in the variable used to retrieve source file names for the `RTHwasan_dynamic` target..
Jul 11 2018, 6:02 AM
delcypher added a comment to D49177: [CMake] Remove unnecesary list of source files for Xray unit tests..

@dberris The tests aren't built under macOS so I couldn't test if this patch actually works. Would it be possible for you to test it for me?

Jul 11 2018, 5:55 AM
delcypher created D49177: [CMake] Remove unnecesary list of source files for Xray unit tests..
Jul 11 2018, 5:54 AM
delcypher created D49176: [CMake] Rename `SANITIZER_HEADERS` to `SANITIZER_IMPL_HEADERS` under `lib/sanitizer_common`..
Jul 11 2018, 5:47 AM

Jul 10 2018

delcypher committed rCRT336699: Try to fix broken build due to r336663..
Try to fix broken build due to r336663.
Jul 10 2018, 9:27 AM
delcypher committed rL336699: Try to fix broken build due to r336663..
Try to fix broken build due to r336663.
Jul 10 2018, 9:27 AM
delcypher committed rCRT336669: [LibFuzzer] Disable MSan test on Darwin which was added by r336619. The….
[LibFuzzer] Disable MSan test on Darwin which was added by r336619. The…
Jul 10 2018, 6:40 AM
delcypher committed rL336669: [LibFuzzer] Disable MSan test on Darwin which was added by r336619. The….
[LibFuzzer] Disable MSan test on Darwin which was added by r336619. The…
Jul 10 2018, 6:40 AM
delcypher committed rL336663: [CMake] Add compiler-rt header files to the list of sources for targets.
[CMake] Add compiler-rt header files to the list of sources for targets
Jul 10 2018, 6:05 AM
delcypher committed rCRT336663: [CMake] Add compiler-rt header files to the list of sources for targets.
[CMake] Add compiler-rt header files to the list of sources for targets
Jul 10 2018, 6:05 AM
delcypher closed D48422: [CMake] Add compiler-rt header files to the list of sources for targets when building with an IDE.
Jul 10 2018, 6:05 AM

Jul 6 2018

delcypher added a comment to D48422: [CMake] Add compiler-rt header files to the list of sources for targets when building with an IDE.

@george.karpenkov @phosek I switched to using explicit lists of headers and found some problems in the process and fixed them. Could you take another look?

Jul 6 2018, 12:02 PM

Jun 28 2018

delcypher added a comment to D48422: [CMake] Add compiler-rt header files to the list of sources for targets when building with an IDE.

@george.karpenkov @phosek I switched to using explicit lists of headers and found some problems in the process and fixed them. Could you take another look?

Jun 28 2018, 6:26 AM
delcypher added a comment to D48422: [CMake] Add compiler-rt header files to the list of sources for targets when building with an IDE.

@eugenis While doing this I think I spotted a typo for a hwasan target. The RTHwasan_dynamic target includes ${TSAN_RTL_CXX_SOURCES} in SOURCES which doesn't make sense. It's probably supposed to be ${HWASAN_RTL_CXX_SOURCES}. I've not fixed it in this patch but I left a FIXME near by. Do you want to fix this?

Jun 28 2018, 6:25 AM
delcypher updated the diff for D48422: [CMake] Add compiler-rt header files to the list of sources for targets when building with an IDE.

Switch to explicit lists of headers instead of globbing.

Jun 28 2018, 6:21 AM

Jun 27 2018

delcypher added inline comments to D48422: [CMake] Add compiler-rt header files to the list of sources for targets when building with an IDE.
Jun 27 2018, 6:21 AM
delcypher committed rCRT335728: [CMake] Tidy up the organisation of compiler-rt when configured as a standalone.
[CMake] Tidy up the organisation of compiler-rt when configured as a standalone
Jun 27 2018, 6:01 AM
delcypher committed rL335728: [CMake] Tidy up the organisation of compiler-rt when configured as a standalone.
[CMake] Tidy up the organisation of compiler-rt when configured as a standalone
Jun 27 2018, 6:01 AM
delcypher closed D48378: [CMake] Tidy up the organisation of compiler-rt when configured as a standalone build with an IDE (e.g. Xcode) as the generator..
Jun 27 2018, 6:01 AM
delcypher updated the diff for D48378: [CMake] Tidy up the organisation of compiler-rt when configured as a standalone build with an IDE (e.g. Xcode) as the generator..

Rebased on trunk

Jun 27 2018, 5:55 AM

Jun 26 2018

delcypher updated the diff for D48422: [CMake] Add compiler-rt header files to the list of sources for targets when building with an IDE.
  • s/crt_process_sources/compiler_rt_process_sources/
  • s/crt_find_headers_in_dir/compiler_rt_find_headers_in_dir/
  • Improve commit message.
Jun 26 2018, 7:20 AM
delcypher added reviewers for D48422: [CMake] Add compiler-rt header files to the list of sources for targets when building with an IDE: kcc, cryptoad, rnk, alekseyshl, vitalybuka, eugenis.
Jun 26 2018, 6:59 AM
delcypher added a comment to D48422: [CMake] Add compiler-rt header files to the list of sources for targets when building with an IDE.

Personally, I'd prefer to spell out all the headers explicitly rather than using globing which is more error-prone to future changes. I also agree with @kubamracek's comment about using crt (which may be also mistaken for crt*.o) to compiler_rt.

Jun 26 2018, 6:53 AM

Jun 25 2018

delcypher added a reviewer for D48422: [CMake] Add compiler-rt header files to the list of sources for targets when building with an IDE: george.karpenkov.
Jun 25 2018, 9:37 AM
delcypher added a reviewer for D48378: [CMake] Tidy up the organisation of compiler-rt when configured as a standalone build with an IDE (e.g. Xcode) as the generator.: george.karpenkov.
Jun 25 2018, 9:37 AM

Jun 22 2018

delcypher added inline comments to D48445: [sanitizer] Use "fast mmap" kernel flag for shadow memory on macOS 10.13.4+.
Jun 22 2018, 10:54 AM · Restricted Project

Jun 21 2018

delcypher requested changes to D48445: [sanitizer] Use "fast mmap" kernel flag for shadow memory on macOS 10.13.4+.
Jun 21 2018, 11:55 AM · Restricted Project
delcypher created D48422: [CMake] Add compiler-rt header files to the list of sources for targets when building with an IDE.
Jun 21 2018, 5:05 AM
delcypher updated the diff for D48378: [CMake] Tidy up the organisation of compiler-rt when configured as a standalone build with an IDE (e.g. Xcode) as the generator..

Drop unnecessary comments.

Jun 21 2018, 5:01 AM
delcypher added a comment to D48378: [CMake] Tidy up the organisation of compiler-rt when configured as a standalone build with an IDE (e.g. Xcode) as the generator..

Okay with me. I would prefer to drop the "For IDEs" comments as they are just clutter.

Jun 21 2018, 5:01 AM

Jun 20 2018

delcypher created D48378: [CMake] Tidy up the organisation of compiler-rt when configured as a standalone build with an IDE (e.g. Xcode) as the generator..
Jun 20 2018, 10:10 AM
delcypher accepted D48309: [sanitizer] Unify and generalize Apple platforms in CMake and lit test configs.
Jun 20 2018, 2:08 AM · Restricted Project
delcypher added a comment to D48309: [sanitizer] Unify and generalize Apple platforms in CMake and lit test configs.

For the whole "ios/iossim" naming and ios_or_iossim: Let's not try to fix that in this patch. But in general, I don't think this is too terrible. The rule simply is: All embedded platforms claim they are also "ios" because they are derivatives of iOS. Maybe this could be greatly improved with just a bit more documentation? I like the fact that you can disable a test for all embedded platforms with just "DISABLE: ios" and you don't have to list all of them.

Jun 20 2018, 2:02 AM · Restricted Project

Jun 19 2018

delcypher requested changes to D48309: [sanitizer] Unify and generalize Apple platforms in CMake and lit test configs.

Overall looks very good. I have a few comments on some changes I'd like to see. The only one I think is essential is renaming the ios lit feature. However I think this could be done in a separate commit.

Jun 19 2018, 2:38 AM · Restricted Project

Jun 18 2018

delcypher added a comment to rL334881: [sanitizer_common] Use O_TRUNC for WrOnly access mode..

Hi @MaskRay,

It seems that internal_lseek and internal_iserror are not implemented in lib/sanitizer_common/sanitizer_mac.cc and fails to link here:http://green.lab.llvm.org/green/job/clang-stage1-configure-RA/46240/consoleFull.

@kubamracek, @delcypher : what is the right thing to do here? Follow the way it was fixed for Windows (https://reviews.llvm.org/rCRT334884)?

Jun 18 2018, 6:30 AM

Jun 14 2018

delcypher abandoned D17296: Improve the semantics ``fptrunc``.
Jun 14 2018, 9:23 AM
delcypher added a comment to D48141: [ASan] Linker-initialize static ScopedInErrorReport::current_error_..

Isn't this basically https://reviews.llvm.org/D44243 ?

It is! Thanks, missed that one. I'm fine with that one being committed too. Is the explanation on this patch satisfactory?

Jun 14 2018, 4:23 AM
delcypher added inline comments to D47995: [ASAN] fix startup crash in dlsym for long paths since glibc 2.27.
Jun 14 2018, 4:16 AM · Restricted Project

Jun 13 2018

delcypher added a comment to D48141: [ASan] Linker-initialize static ScopedInErrorReport::current_error_..

Isn't this basically https://reviews.llvm.org/D44243 ?

Jun 13 2018, 12:17 PM

Jun 9 2018

delcypher added a comment to D47210: [lit] Fix the `--max-time` flag feature which was completely broken..
In D47210#1118125, @rnk wrote:

Okay. Is there a portable and reliable way to catch pool.terminate() being called from the parent process in the worker processes? Performing process traversal and kill in the parent process is likely to be racey so it's better if we can do it in the worker processes.

I don't think so. I think terminate is intended to be unclean, i.e. you might get SIGKILL like behavior on some platforms.

Jun 9 2018, 5:07 PM

May 31 2018

delcypher added a comment to D47210: [lit] Fix the `--max-time` flag feature which was completely broken..
In D47210#1116629, @rnk wrote:

+@thakis, who has been prototyping some lit optimizations, I think.

Another approach is to depend on the pustil module again and have it walk through all child processes of lit and kill them. This is the simplest short term solution. This probably the right thing to do because --max-time doesn't seem that widely used. Otherwise someone would have already tried to fix --max-time before me.

That sounds like a good way to go.

The multiprocessing shutdown behavior leads a lot to be desired, and it's basically impossible to write reliable tests for it. I regularly have problems interrupting lit test execution here on Windows. It worked better when I used mintty, but I switched back to cmd after updating to Windows 10. I think improving all that behavior is a separate issue that's out of scope for this, and probably a higher priority.

May 31 2018, 11:58 AM
delcypher added inline comments to D47560: [CMake] Use find_package to find zlib.
May 31 2018, 11:49 AM

May 30 2018

delcypher added inline comments to D47560: [CMake] Use find_package to find zlib.
May 30 2018, 5:04 PM
delcypher added inline comments to D47561: [CMake] Unify handling of libxml2 with other libraries.
May 30 2018, 4:30 PM
delcypher added inline comments to D47560: [CMake] Use find_package to find zlib.
May 30 2018, 4:28 PM
delcypher added inline comments to D47560: [CMake] Use find_package to find zlib.
May 30 2018, 4:26 PM
delcypher requested changes to D47560: [CMake] Use find_package to find zlib.

The idea behind this change is fine. The main problem in the implementation is not considering the scenario where LLVM_ENABLE_ZLIB is initially TRUE but is then changed at a later date. The current implementation would ignore the value of LLVM_ENABLE_ZLIB because the HAVE_LIBZ cache variable is still present from a previous configure run.

May 30 2018, 4:16 PM

May 29 2018

delcypher added a comment to D47210: [lit] Fix the `--max-time` flag feature which was completely broken..

@rnk Hmm this isn't quite ready to land. I've just noticed that the shtest-max-global-time-multiprocess.py leaves the 1_infinite_loop.py process (and the shell - bash in my case - that spawned it) lying around. This will cause problems on the bots so we can't commit this.

May 29 2018, 6:45 PM
delcypher added a comment to D44598: [lit] Report line number for failed RUN command.

Ping.

What seems to be the problem?
This was accepted before, reverted due to windows bot breakage,
https://reviews.llvm.org/D44598#1091098 suggests that breakage is fixed.
Just commit again?

There was enough of a code change that I thought the reviewer would want to take another look. Is that not the right approach?

May 29 2018, 1:23 PM
delcypher added a comment to D47210: [lit] Fix the `--max-time` flag feature which was completely broken..

Ping.

May 29 2018, 10:42 AM

May 23 2018

delcypher updated the diff for D47210: [lit] Fix the `--max-time` flag feature which was completely broken..

Avoid use of child_lit_config where possible and add a comment explaining why we assign to the child_lit_config global.

May 23 2018, 1:24 AM
delcypher added inline comments to D47210: [lit] Fix the `--max-time` flag feature which was completely broken..
May 23 2018, 1:07 AM
delcypher added a comment to D47153: [Support] Avoid normalization in sys::getDefaultTargetTriple.

I'm not familiar with this code but if I'm reading the patch correctly, then the commit message omits the fact that it changes he windows target triple from having a win32 suffix to windows-msvc suffix. I'm not sure what the implications are here but the fact you've done this is probably worth mentioning in the commit message.

Done. I've also tested this change on Windows and this seems to be passing now so should be ready for re-land.

May 23 2018, 12:39 AM

May 22 2018

delcypher created D47210: [lit] Fix the `--max-time` flag feature which was completely broken..
May 22 2018, 11:01 AM
delcypher committed rL332987: [lit] Try to make `shtest-timeout.py` test more reliable by using a.
[lit] Try to make `shtest-timeout.py` test more reliable by using a
May 22 2018, 8:13 AM
delcypher committed rL332986: [lit] Don't run `slow.py` in `shtest-timeout.py` test..
[lit] Don't run `slow.py` in `shtest-timeout.py` test.
May 22 2018, 8:13 AM
delcypher committed rL332985: [lit] Don't check output of commands used in `shtest-timeout.py` test..
[lit] Don't check output of commands used in `shtest-timeout.py` test.
May 22 2018, 8:12 AM

May 21 2018

delcypher added a comment to D47153: [Support] Avoid normalization in sys::getDefaultTargetTriple.

I'm not familiar with this code but if I'm reading the patch correctly, then the commit message omits the fact that it changes the windows target triple from having a win32 suffix to windows-msvc suffix. I'm not sure what the implications are here but the fact you've done this is probably worth mentioning in the commit message.

May 21 2018, 11:08 AM