Page MenuHomePhabricator

cryptoad (Kostya Kortchinsky)
User

Projects

User does not belong to any projects.

User Details

User Since
May 5 2016, 2:57 PM (267 w, 3 h)

Recent Activity

Yesterday

cryptoad committed rG8b062b616062: [scudo] Ensure proper allocator alignment in TSD test (authored by cryptoad).
[scudo] Ensure proper allocator alignment in TSD test
Wed, Jun 16, 2:22 PM
cryptoad closed D104402: [scudo] Ensure proper allocator alignment in TSD test.
Wed, Jun 16, 2:22 PM · Restricted Project
cryptoad updated the diff for D104402: [scudo] Ensure proper allocator alignment in TSD test.

Using using instead of typedef on the newly added ones.

Wed, Jun 16, 2:18 PM · Restricted Project
cryptoad updated the diff for D104402: [scudo] Ensure proper allocator alignment in TSD test.

Adding more isAligned DCHECK as Vitaly suggested.

Wed, Jun 16, 2:07 PM · Restricted Project
cryptoad added inline comments to D104402: [scudo] Ensure proper allocator alignment in TSD test.
Wed, Jun 16, 1:50 PM · Restricted Project
cryptoad updated the diff for D104402: [scudo] Ensure proper allocator alignment in TSD test.

Add stdlib.h to the test for posix_memalign. My builds don't
complain but I assume something will eventually.

Wed, Jun 16, 10:56 AM · Restricted Project
cryptoad added a comment to D103119: [scudo] Get rid of initLinkerInitialized.

Hi,

this is due to a misaligned data access, in Thumb the code generated for unmapTestOnly
contains a Vector Store instruction vst1.64 {d16-d17}, [r0, :128] which needs a 16-bytes
alignement, but the adress of TSDs (which is in r0) is not.

Wed, Jun 16, 10:55 AM · Restricted Project
cryptoad requested review of D104402: [scudo] Ensure proper allocator alignment in TSD test.
Wed, Jun 16, 10:52 AM · Restricted Project

Mon, Jun 14

cryptoad added a comment to D103119: [scudo] Get rid of initLinkerInitialized.

Hi,

This change introduced a regression on ARMv7 Thumb bots which wasn't noticed due to other issues.

logs can be seen here: https://lab.llvm.org/buildbot/#/builders/26/builds/2251

The 3 tests fail with a bus error raised from tsd_shared.h line 49

Mon, Jun 14, 11:24 AM · Restricted Project

Tue, Jun 8

cryptoad committed rG2551053e8d8d: [scudo] Add Scudo support for Trusty OS (authored by danieljm).
[scudo] Add Scudo support for Trusty OS
Tue, Jun 8, 2:03 PM
cryptoad closed D103578: [scudo] Add Scudo support for Trusty OS.
Tue, Jun 8, 2:02 PM · Restricted Project

Mon, Jun 7

cryptoad added inline comments to D103578: [scudo] Add Scudo support for Trusty OS.
Mon, Jun 7, 1:13 PM · Restricted Project
cryptoad accepted D103578: [scudo] Add Scudo support for Trusty OS.

I patched it locally and everything passes for me.
Given the current timeline, I am OK with landing it now as is, and improve gradually on it.
I will pick up some of the action items, notably to try and reduce the memory footprint.

Mon, Jun 7, 11:10 AM · Restricted Project

Fri, Jun 4

cryptoad accepted D103718: [Scudo] Improve ScopedString constructor.
Fri, Jun 4, 6:08 PM · Restricted Project
cryptoad added a comment to D103718: [Scudo] Improve ScopedString constructor.

Was it the DCHECK issues I fixed with https://reviews.llvm.org/D103716 or something else?

Fri, Jun 4, 4:26 PM · Restricted Project
cryptoad accepted D103725: [scudo] Remove ScopedString::Length.
Fri, Jun 4, 4:07 PM · Restricted Project
cryptoad added inline comments to D103721: [NFC][scudo] Make formatString return uptr.
Fri, Jun 4, 3:19 PM · Restricted Project
cryptoad added a comment to D103641: [scudo] Rework Vector/String.

This looks like it broke the sanitizer-x86_64-linux-qemu.
https://lab.llvm.org/buildbot/#/builders/169/builds/501

Found the issue, testing the fix.

Fri, Jun 4, 1:42 PM · Restricted Project
cryptoad committed rG5019b0a56588: [scudo] Fix String DCHECK (authored by cryptoad).
[scudo] Fix String DCHECK
Fri, Jun 4, 1:42 PM
cryptoad closed D103716: [scudo] Fix String DCHECK.
Fri, Jun 4, 1:42 PM · Restricted Project
cryptoad requested review of D103716: [scudo] Fix String DCHECK.
Fri, Jun 4, 1:38 PM · Restricted Project
cryptoad added a comment to D103641: [scudo] Rework Vector/String.

This looks like it broke the sanitizer-x86_64-linux-qemu.
https://lab.llvm.org/buildbot/#/builders/169/builds/501

Fri, Jun 4, 1:21 PM · Restricted Project
cryptoad added a comment to rG868317b3fd76: [scudo] Rework Vector/String.

This looks like it broke the sanitizer-x86_64-linux-qemu.
https://lab.llvm.org/buildbot/#/builders/169/builds/501

Fri, Jun 4, 12:56 PM
cryptoad added inline comments to D103578: [scudo] Add Scudo support for Trusty OS.
Fri, Jun 4, 8:00 AM · Restricted Project
cryptoad added inline comments to D103578: [scudo] Add Scudo support for Trusty OS.
Fri, Jun 4, 7:57 AM · Restricted Project

Thu, Jun 3

cryptoad committed rG868317b3fd76: [scudo] Rework Vector/String (authored by cryptoad).
[scudo] Rework Vector/String
Thu, Jun 3, 6:13 PM
cryptoad closed D103641: [scudo] Rework Vector/String.
Thu, Jun 3, 6:13 PM · Restricted Project
cryptoad added inline comments to D103641: [scudo] Rework Vector/String.
Thu, Jun 3, 3:03 PM · Restricted Project
cryptoad updated the diff for D103641: [scudo] Rework Vector/String.

Switching 2 lines around.

Thu, Jun 3, 3:02 PM · Restricted Project
cryptoad updated the diff for D103641: [scudo] Rework Vector/String.

Change type in vector test.

Thu, Jun 3, 2:46 PM · Restricted Project
cryptoad updated the diff for D103641: [scudo] Rework Vector/String.

and I forgot to copy over the test file.

Thu, Jun 3, 2:19 PM · Restricted Project
cryptoad updated the diff for D103641: [scudo] Rework Vector/String.

Remove InitialSize as suggested.

Thu, Jun 3, 2:17 PM · Restricted Project
cryptoad updated the diff for D103641: [scudo] Rework Vector/String.

Remove stray comment.

Thu, Jun 3, 2:11 PM · Restricted Project
cryptoad updated the diff for D103641: [scudo] Rework Vector/String.

Based on feedback, remove for now the CanGrow aspect of the vector.

Thu, Jun 3, 2:07 PM · Restricted Project
cryptoad added a comment to D103641: [scudo] Rework Vector/String.

CanGrow templated parameter that for now is
always true but would be set to false on Trusty.

Are we seeing OOMs on Trusty due to ScopedString? I had a quick look and it seemed like there's no non-ephemeral buffers (and really, this shouldn't be called very often at all), and it concerns me a little that push_back can silently fail on a ScopedErrorReport and truncate the text and it seems hard to test "all failure conditions log successfully".

Thu, Jun 3, 1:55 PM · Restricted Project
cryptoad updated the diff for D103641: [scudo] Rework Vector/String.

Corrected, with a test now.

Thu, Jun 3, 12:31 PM · Restricted Project
cryptoad updated the diff for D103641: [scudo] Rework Vector/String.

Correcting a couple of ScopedString constructors.

Thu, Jun 3, 12:13 PM · Restricted Project
cryptoad requested review of D103641: [scudo] Rework Vector/String.
Thu, Jun 3, 12:11 PM · Restricted Project
cryptoad added a comment to D103578: [scudo] Add Scudo support for Trusty OS.

Thank you Daniel, this looks like a good start!
I'll work on my side on reducing some of the memory footprint of some structures and whatnot (scoped strings come to mind).
As discussed, we can re-introduce the use-separate-class-for-batches distinction for Trusty, which can be a large win to get rid of a class size.
I still think the Primary32 might be better suited here, but we'll follow up.

Thu, Jun 3, 7:56 AM · Restricted Project

Thu, May 27

cryptoad added a comment to D102543: [Scudo] Make -fsanitize=scudo use standalone. Migrate tests..

sanitizer-x86_64-linux https://lab.llvm.org/buildbot/#/builders/37/builds/4244 FAIL: ScudoStandalone-x86_64 :: preinit.c (772 of 856)
ppc64be-clang-test https://lab.llvm.org/buildbot#builders/52/builds/7794 TEST 'ScudoStandalone-powerpc64 :: preinit.c' FAILED
among others

Thu, May 27, 12:28 PM · Restricted Project, Restricted Project

Wed, May 26

cryptoad added a comment to D102543: [Scudo] Make -fsanitize=scudo use standalone. Migrate tests..

I saw some bots failure for preinit.c:

FAIL: ScudoStandalone-i386 :: preinit.c (768 of 856)
******************** TEST 'ScudoStandalone-i386 :: preinit.c' FAILED ********************
Script:
--
: 'RUN: at line 1';      /b/sanitizer-x86_64-linux/build/clang_build/./bin/clang   -m32  -pthread -fPIE -pie -O0 -UNDEBUG -Wl,--gc-sections -resource-dir=/b/sanitizer-x86_64-linux/build/clang_build/./lib/clang/13.0.0/lib/linux/../../ -fsanitize=scudo /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test/scudo/standalone/preinit.c -o /b/sanitizer-x86_64-linux/build/clang_build/projects/compiler-rt/test/scudo/standalone/I386LinuxConfig/Output/preinit.c.tmp
: 'RUN: at line 2';    /b/sanitizer-x86_64-linux/build/clang_build/projects/compiler-rt/test/scudo/standalone/I386LinuxConfig/Output/preinit.c.tmp 2>&1
--
Exit Code: 139
Wed, May 26, 4:08 PM · Restricted Project, Restricted Project
cryptoad accepted D103200: [scudo] Build scudo_standalone on Android and Fuchsia..
Wed, May 26, 2:56 PM · Restricted Project
cryptoad abandoned D70860: [docs] Fixed incorrect build instructions for Scudo.

Docs have been udpated some time ago with new build instructions to reflect the migration to the standalone version. Abandoning this.

Wed, May 26, 2:04 PM · Restricted Project
cryptoad commandeered D70860: [docs] Fixed incorrect build instructions for Scudo.
Wed, May 26, 2:04 PM · Restricted Project
cryptoad added a comment to D103200: [scudo] Build scudo_standalone on Android and Fuchsia..

I think Fuchsia doesn't need COMPILER_RT_HAS_AUXV since it's only used in Linux parts.

Wed, May 26, 1:03 PM · Restricted Project
cryptoad committed rGa45877eea8c4: [scudo] Get rid of initLinkerInitialized (authored by cryptoad).
[scudo] Get rid of initLinkerInitialized
Wed, May 26, 9:54 AM
cryptoad closed D103119: [scudo] Get rid of initLinkerInitialized.
Wed, May 26, 9:54 AM · Restricted Project

Tue, May 25

cryptoad updated the diff for D103119: [scudo] Get rid of initLinkerInitialized.

Remove HybridMutex::init, remove stray ;.

Tue, May 25, 3:34 PM · Restricted Project
cryptoad accepted D103122: [NFC][SCUDO] Fix unittest for -gtest_repeat=10.
Tue, May 25, 3:29 PM · Restricted Project
cryptoad requested review of D103119: [scudo] Get rid of initLinkerInitialized.
Tue, May 25, 3:01 PM · Restricted Project
cryptoad committed rG1872283457fc: [scudo] Rework dieOnMapUnmapError (authored by cryptoad).
[scudo] Rework dieOnMapUnmapError
Tue, May 25, 8:28 AM
cryptoad closed D103034: [scudo] Rework dieOnMapUnmapError.
Tue, May 25, 8:28 AM · Restricted Project
cryptoad accepted D103061: [scudo] Consistent setting of SCUDO_DEBUG.
Tue, May 25, 7:55 AM · Restricted Project
cryptoad accepted D103060: [scudo] Fix CHECK implementation.

Right, that makes sense, thank you Vitaly!
FWIW, this was originally taken from CHECK_IMPL in sanitizer_common that seems to display the same behavior: https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h#L289

Tue, May 25, 7:55 AM · Restricted Project

Mon, May 24

cryptoad added a comment to D103042: [NFC][scudo] Add paramenters DCHECKs.

This might trigger sign-compare warnings like https://reviews.llvm.org/D103029?

Mon, May 24, 1:24 PM · Restricted Project
cryptoad requested review of D103034: [scudo] Rework dieOnMapUnmapError.
Mon, May 24, 9:26 AM · Restricted Project
cryptoad committed rG20c1f94220d9: [scudo] Separate Fuchsia & Default SizeClassMap (authored by cryptoad).
[scudo] Separate Fuchsia & Default SizeClassMap
Mon, May 24, 8:54 AM
cryptoad closed D102783: [scudo] Separate Fuchsia & Default SizeClassMap.
Mon, May 24, 8:54 AM · Restricted Project
cryptoad accepted D102994: [NFC][scudo] Small test cleanup.
Mon, May 24, 8:03 AM · Restricted Project

Sun, May 23

cryptoad added a reviewer for D102783: [scudo] Separate Fuchsia & Default SizeClassMap: vitalybuka.
Sun, May 23, 11:24 AM · Restricted Project
cryptoad added inline comments to D102979: [scudo] Use MAP_NORESERVE to release pages.
Sun, May 23, 10:04 AM · Restricted Project

Fri, May 21

cryptoad added a comment to D102783: [scudo] Separate Fuchsia & Default SizeClassMap.

ping please

Fri, May 21, 10:00 AM · Restricted Project
cryptoad accepted D102887: [scudo] Try to re-enabled the test on arm.
Fri, May 21, 7:59 AM · Restricted Project

Thu, May 20

cryptoad accepted D102886: [scudo] Fix EXPECT_DEATH tests.

Good point!

Thu, May 20, 4:36 PM · Restricted Project
cryptoad accepted D102874: [scudo] Add unmapTestOnly() to secondary..
Thu, May 20, 2:22 PM · Restricted Project
cryptoad updated the diff for D102783: [scudo] Separate Fuchsia & Default SizeClassMap.

Shuffle a couple of lines around.

Thu, May 20, 9:32 AM · Restricted Project

Wed, May 19

cryptoad accepted D102648: [scudo] Add supported architectures..
Wed, May 19, 1:49 PM · Restricted Project
cryptoad added a comment to D102783: [scudo] Separate Fuchsia & Default SizeClassMap.

I should say that this improves the QPS for rpc2 benchmark in g3:

  • New SCM: Finished measurement, throughput=438422.221084 reqs/sec.
  • Old SCM: Finished measurement, throughput=421669.366002 reqs/sec.
Wed, May 19, 9:23 AM · Restricted Project
cryptoad requested review of D102783: [scudo] Separate Fuchsia & Default SizeClassMap.
Wed, May 19, 9:13 AM · Restricted Project

May 18 2021

cryptoad accepted D102716: scudo: Test realloc on increasing size buffers..

Thank you Peter!

May 18 2021, 2:39 PM · Restricted Project

May 17 2021

cryptoad accepted D102543: [Scudo] Make -fsanitize=scudo use standalone. Migrate tests..

I think this is good, with the additional arch

May 17 2021, 10:48 AM · Restricted Project, Restricted Project
cryptoad added a comment to D102543: [Scudo] Make -fsanitize=scudo use standalone. Migrate tests..

I think one of the remaining things to address is the supported architectures:

set(ALL_SCUDO_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM32} ${ARM64} ${MIPS32} ${MIPS64} ${PPC64})
set(ALL_SCUDO_STANDALONE_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM64})

I am unclear as to whether or not there are consumers for the missing ones, but if there are we might break -fsanitize=scudo for them.
We have fallback for non-x86/arm processors in https://github.com/llvm/llvm-project/blob/e0921655b1ff8d4ba7c14be59252fe05b705920e/compiler-rt/lib/scudo/standalone/checksum.cpp#L79 , the rest looks fine so I think adding them should work.

May 17 2021, 8:35 AM · Restricted Project, Restricted Project

May 15 2021

cryptoad added a comment to D102543: [Scudo] Make -fsanitize=scudo use standalone. Migrate tests..

Thanks a lot Mitch! Couple of comments from a first look, I'll give it another go later.

May 15 2021, 7:59 AM · Restricted Project, Restricted Project

May 14 2021

cryptoad accepted D102529: [Scudo] Delete unused flag 'rss_limit_mb'..

I should implement that someday, but nobody seems to have a need for it.

May 14 2021, 1:40 PM · Restricted Project
cryptoad accepted D102349: [GWP-ASan] Migrate lit tests from old Scudo -> Standalone..
May 14 2021, 10:22 AM · Restricted Project

May 10 2021

cryptoad accepted D95884: [Scudo] Use GWP-ASan's aligned allocations and fixup postalloc hooks..
May 10 2021, 12:02 PM · Restricted Project
cryptoad accepted D101865: [scudo] [GWP-ASan] Add GWP-ASan variant of scudo benchmarks..
May 10 2021, 11:57 AM · Restricted Project

May 5 2021

cryptoad committed rG6fac34251d01: [scudo] Add initialization for TSDRegistrySharedT (authored by cferris).
[scudo] Add initialization for TSDRegistrySharedT
May 5 2021, 7:01 PM
cryptoad closed D101951: [scudo] Add initialization for TSDRegistrySharedT.
May 5 2021, 7:01 PM · Restricted Project
cryptoad accepted D101951: [scudo] Add initialization for TSDRegistrySharedT.
May 5 2021, 3:59 PM · Restricted Project

May 3 2021

cryptoad added a comment to D95884: [Scudo] Use GWP-ASan's aligned allocations and fixup postalloc hooks..

Update to provide correct statistics for live/free allocations in GWP-ASan. Use the slot size in the calculation rather than actual size, as there's no way to pre-calculate the size of the allocations that can land in the guarded pool.

@cryptoad - Just wanted to double check with you that this schema SGTY. mallinfo.uordblks and mallinfo.fordblks for malloc(1) moves up and down by a page if it lands in the GWP-ASan region, which is a little strange, but it's better than the alternative of having 4095 bytes "available" as noted by fordblks that aren't actually allocable.

May 3 2021, 2:01 PM · Restricted Project
cryptoad accepted D101653: [scudo] Don't track free/use stats for transfer batches..
May 3 2021, 11:17 AM · Restricted Project

Apr 30 2021

cryptoad added a comment to D101653: [scudo] Don't track free/use stats for transfer batches..

ClassSize is only used for the stats in the local cache, what about initializing it to 0 for BatchClassId in initCache?

Apr 30 2021, 6:05 PM · Restricted Project
cryptoad accepted D101653: [scudo] Don't track free/use stats for transfer batches..

Right, it makes sense to not account for the batches. I don't think this should have a meaningful impact, but have you noticed any perf changes adding this since it's in the fastpath?

Apr 30 2021, 11:07 AM · Restricted Project

Apr 29 2021

cryptoad added inline comments to D101514: [scudo] Use require_constant_initialization.
Apr 29 2021, 8:16 AM · Restricted Project
cryptoad added a comment to D101514: [scudo] Use require_constant_initialization.

Is this related to the GWP-ASan initialization issues?
I am unclear if we need to rework the init() vs initLinkInitialized() functions as well?

Apr 29 2021, 7:53 AM · Restricted Project

Apr 27 2021

cryptoad accepted D101407: [gwp_asan] Use __sanitizer_fast_backtrace on Fuchsia.

Adding @hctim for visibility.

Apr 27 2021, 3:50 PM · Restricted Project

Apr 23 2021

cryptoad accepted D101137: scudo: Store header on deallocation before retagging memory..
Apr 23 2021, 8:06 AM · Restricted Project

Apr 22 2021

cryptoad accepted D101018: scudo: Use a table to look up the LSB for computing the odd/even mask. NFCI..
Apr 22 2021, 8:07 AM · Restricted Project
cryptoad accepted D101031: [scudo] Check if MADV_DONTNEED zeroes memory.

Thank you for finding this!
LGTM with a couple of nits

Apr 22 2021, 8:04 AM · Restricted Project

Apr 21 2021

cryptoad accepted D101014: scudo: Obtain tag from pointer instead of loading it from memory. NFCI..
Apr 21 2021, 8:44 PM · Restricted Project

Apr 17 2021

cryptoad abandoned D100683: [scudo][standalone] Fix -Werror=empty-body error on Fuchsia.

https://reviews.llvm.org/D100693 is better.

Apr 17 2021, 8:08 AM · Restricted Project

Apr 16 2021

cryptoad accepted D100693: [scudo] Avoid empty statement warnings.

Well I wrote up https://reviews.llvm.org/D100683, but I guess yours makes more sense!

Apr 16 2021, 7:11 PM · Restricted Project
cryptoad requested review of D100683: [scudo][standalone] Fix -Werror=empty-body error on Fuchsia.
Apr 16 2021, 1:38 PM · Restricted Project

Apr 15 2021

cryptoad committed rG3f97c66b0040: [scudo][standalone] Fuchsia related fixes (authored by cryptoad).
[scudo][standalone] Fuchsia related fixes
Apr 15 2021, 1:27 PM
cryptoad closed D100524: [scudo][standalone] Fuchsia related fixes.
Apr 15 2021, 1:27 PM · Restricted Project
cryptoad added inline comments to D100524: [scudo][standalone] Fuchsia related fixes.
Apr 15 2021, 8:59 AM · Restricted Project

Apr 14 2021

cryptoad requested review of D100524: [scudo][standalone] Fuchsia related fixes.
Apr 14 2021, 9:21 PM · Restricted Project
cryptoad accepted D100426: [scudo] Restore zxtest compatibility.

As far as I can tell this works.
With a few extra fixes:

[==========] 183 tests from 87 test cases ran (10944 ms total).
[  PASSED  ] 183 tests
Apr 14 2021, 3:42 PM · Restricted Project
cryptoad added a comment to D100426: [scudo] Restore zxtest compatibility.

I assume D99766 should not be relevant because DefaultConfig is not tested on SCUDO_FUCHSIA ?

Apr 14 2021, 12:37 PM · Restricted Project