fjricci (Francis Ricci)
User

Projects

User does not belong to any projects.

User Details

User Since
Dec 4 2015, 11:34 AM (84 w, 6 d)

Recent Activity

Yesterday

fjricci committed rL308676: Revert "Add MemoryMappedSection struct for two-level memory map iteration".
Revert "Add MemoryMappedSection struct for two-level memory map iteration"
Thu, Jul 20, 2:24 PM
fjricci added inline comments to D35422: Add MemoryMappedSection struct for two-level memory map iteration.
Thu, Jul 20, 11:57 AM
fjricci added inline comments to D35422: Add MemoryMappedSection struct for two-level memory map iteration.
Thu, Jul 20, 11:15 AM
fjricci committed rL308644: Add MemoryMappedSection struct for two-level memory map iteration.
Add MemoryMappedSection struct for two-level memory map iteration
Thu, Jul 20, 11:09 AM
fjricci closed D35422: Add MemoryMappedSection struct for two-level memory map iteration by committing rL308644: Add MemoryMappedSection struct for two-level memory map iteration.
Thu, Jul 20, 11:09 AM
fjricci updated the diff for D35432: Only scan global sections containing data in LSan on darwin.

clang-format

Thu, Jul 20, 10:51 AM
fjricci requested review of D35432: Only scan global sections containing data in LSan on darwin.

This had to be reverted due to a masking error in its dependency which caused failures on 10.11 buildbots. Since it got reverted anyway, I took the opportunity to look into some false positives in swift, and found that they were caused by this patch. In order to prevent false positives, this now uses a blacklist of sections which definitely do not store pointers instead of a whitelist of sections which do (to avoid missing sections). Would appreciate a re-review.

Thu, Jul 20, 10:38 AM
fjricci updated the diff for D35432: Only scan global sections containing data in LSan on darwin.

Use blacklist instead of whitelist to avoid false positives

Thu, Jul 20, 10:36 AM
fjricci reopened D35432: Only scan global sections containing data in LSan on darwin.

Dependency was reverted.

Thu, Jul 20, 10:36 AM
fjricci updated the diff for D35422: Add MemoryMappedSection struct for two-level memory map iteration.

Remove accidental arcanist squash

Thu, Jul 20, 10:33 AM
fjricci updated the diff for D35422: Add MemoryMappedSection struct for two-level memory map iteration.

Use blacklist instead of whitelist

Thu, Jul 20, 10:32 AM

Wed, Jul 19

fjricci added inline comments to D35551: Introduce sanitizer_procmaps_netbsd.cc.
Wed, Jul 19, 3:32 PM · Restricted Project
fjricci requested review of D35422: Add MemoryMappedSection struct for two-level memory map iteration.
Wed, Jul 19, 10:37 AM
fjricci updated the diff for D35422: Add MemoryMappedSection struct for two-level memory map iteration.

Fix 10.11 buildbots by masking dyld section addresses

Wed, Jul 19, 10:37 AM

Tue, Jul 18

fjricci committed rL308395: Revert "Add MemoryMappedSection struct for two-level memory map iteration".
Revert "Add MemoryMappedSection struct for two-level memory map iteration"
Tue, Jul 18, 4:54 PM
fjricci committed rL308394: Revert "Only scan global sections containing data in LSan on darwin".
Revert "Only scan global sections containing data in LSan on darwin"
Tue, Jul 18, 4:54 PM
fjricci reopened D35422: Add MemoryMappedSection struct for two-level memory map iteration.

Reverted as rL308394 and rL308395

Tue, Jul 18, 4:54 PM
fjricci added a comment to D35422: Add MemoryMappedSection struct for two-level memory map iteration.

Interesting to me that this would cause timeouts, but yeah, I'll revert and look into it.

Tue, Jul 18, 4:26 PM
fjricci added a comment to D35443: Enable 64-bit Darwin LeakSanitizer by default on AddressSanitizer builds.

I'm running the swift stdlib test suite with LSan, and it complains about the dyld insertion:

Tue, Jul 18, 2:57 PM
fjricci committed rL308353: Don't call exit() from atexit handlers on Darwin.
Don't call exit() from atexit handlers on Darwin
Tue, Jul 18, 1:19 PM
fjricci closed D35513: Don't call exit() from atexit handlers on Darwin by committing rL308353: Don't call exit() from atexit handlers on Darwin.
Tue, Jul 18, 1:19 PM
fjricci added a comment to D35443: Enable 64-bit Darwin LeakSanitizer by default on AddressSanitizer builds.

Still working on swift, but looks like bootstrapped llvm-tblgen has a few leaks. They don't show up on Linux, but they also don't really look like false positives:

Tue, Jul 18, 11:20 AM
fjricci updated the diff for D35513: Don't call exit() from atexit handlers on Darwin.

Address comments

Tue, Jul 18, 8:16 AM

Mon, Jul 17

fjricci added a comment to D35443: Enable 64-bit Darwin LeakSanitizer by default on AddressSanitizer builds.

DYLD_INSERT_LIBRARIES=<path_to_lsan_library> ./an-already-built-binary
should do the trick.

Mon, Jul 17, 5:47 PM
fjricci committed rL308231: Only scan global sections containing data in LSan on darwin.
Only scan global sections containing data in LSan on darwin
Mon, Jul 17, 4:03 PM
fjricci closed D35432: Only scan global sections containing data in LSan on darwin by committing rL308231: Only scan global sections containing data in LSan on darwin.
Mon, Jul 17, 4:03 PM
fjricci added a comment to D35443: Enable 64-bit Darwin LeakSanitizer by default on AddressSanitizer builds.

re check-llvm, turns out it was a KP with libedit, re-running with that disabled using LLVM_USE_SANITIZER instead of manual cflags

Mon, Jul 17, 3:57 PM
fjricci added a comment to D35443: Enable 64-bit Darwin LeakSanitizer by default on AddressSanitizer builds.

I spoke too soon, looks like check-clang is clean but check-llvm has a few leaks. I'll look into whether those seem like false positives.

Mon, Jul 17, 3:47 PM
fjricci added a comment to D35443: Enable 64-bit Darwin LeakSanitizer by default on AddressSanitizer builds.

I ran bootstrapped check-clang with -fsanitize=address and this patch, all tests passed. (Verified that it was running LSan as well by making sure tests failed with a CHECK(0) in DoLeakCheck()).

Mon, Jul 17, 3:38 PM
fjricci created D35513: Don't call exit() from atexit handlers on Darwin.
Mon, Jul 17, 3:09 PM
fjricci updated the diff for D35432: Only scan global sections containing data in LSan on darwin.

sec_names -> kGlobalVarSecNames

Mon, Jul 17, 1:44 PM
fjricci updated the diff for D35432: Only scan global sections containing data in LSan on darwin.

Address comments

Mon, Jul 17, 1:38 PM
fjricci retitled D35432: Only scan global sections containing data in LSan on darwin from Only scan global sections containing data in LSan on darin to Only scan global sections containing data in LSan on darwin.
Mon, Jul 17, 1:13 PM
fjricci committed rL308210: Add MemoryMappedSection struct for two-level memory map iteration.
Add MemoryMappedSection struct for two-level memory map iteration
Mon, Jul 17, 1:09 PM
fjricci closed D35422: Add MemoryMappedSection struct for two-level memory map iteration by committing rL308210: Add MemoryMappedSection struct for two-level memory map iteration.
Mon, Jul 17, 1:09 PM

Fri, Jul 14

fjricci added inline comments to D35422: Add MemoryMappedSection struct for two-level memory map iteration.
Fri, Jul 14, 5:11 PM
fjricci added a comment to D35443: Enable 64-bit Darwin LeakSanitizer by default on AddressSanitizer builds.

Please wait on enabling this on-by-default until we perform some more testing on our side.

Fri, Jul 14, 4:43 PM
fjricci added inline comments to D35422: Add MemoryMappedSection struct for two-level memory map iteration.
Fri, Jul 14, 4:21 PM
fjricci created D35443: Enable 64-bit Darwin LeakSanitizer by default on AddressSanitizer builds.
Fri, Jul 14, 4:17 PM
fjricci updated the diff for D35432: Only scan global sections containing data in LSan on darwin.

Rebase

Fri, Jul 14, 3:45 PM
fjricci updated the diff for D35422: Add MemoryMappedSection struct for two-level memory map iteration.

Fix whitespace

Fri, Jul 14, 3:42 PM
fjricci abandoned D35423: Add basic section information to darwin procmaps.

Merged into D35422

Fri, Jul 14, 3:34 PM
fjricci updated the diff for D35422: Add MemoryMappedSection struct for two-level memory map iteration.

Address comments and remove MemoryMappedSection class

Fri, Jul 14, 3:33 PM
fjricci added inline comments to D35422: Add MemoryMappedSection struct for two-level memory map iteration.
Fri, Jul 14, 2:27 PM
fjricci added inline comments to D35422: Add MemoryMappedSection struct for two-level memory map iteration.
Fri, Jul 14, 2:15 PM
fjricci updated the summary of D35432: Only scan global sections containing data in LSan on darwin.
Fri, Jul 14, 12:21 PM
fjricci updated the summary of D35432: Only scan global sections containing data in LSan on darwin.
Fri, Jul 14, 12:21 PM
fjricci added a dependent revision for D35423: Add basic section information to darwin procmaps: D35432: Only scan global sections containing data in LSan on darwin.
Fri, Jul 14, 12:16 PM
fjricci added a dependency for D35432: Only scan global sections containing data in LSan on darwin: D35423: Add basic section information to darwin procmaps.
Fri, Jul 14, 12:16 PM
fjricci created D35432: Only scan global sections containing data in LSan on darwin.
Fri, Jul 14, 12:15 PM
fjricci updated the diff for D35423: Add basic section information to darwin procmaps.

Fix backwards conditional

Fri, Jul 14, 11:04 AM
fjricci added a dependent revision for D35422: Add MemoryMappedSection struct for two-level memory map iteration: D35423: Add basic section information to darwin procmaps.
Fri, Jul 14, 10:27 AM
fjricci added a dependency for D35423: Add basic section information to darwin procmaps: D35422: Add MemoryMappedSection struct for two-level memory map iteration.
Fri, Jul 14, 10:27 AM
fjricci created D35423: Add basic section information to darwin procmaps.
Fri, Jul 14, 10:27 AM
fjricci updated the diff for D35422: Add MemoryMappedSection struct for two-level memory map iteration.

Fix a typo

Fri, Jul 14, 10:25 AM
fjricci created D35422: Add MemoryMappedSection struct for two-level memory map iteration.
Fri, Jul 14, 10:24 AM

Wed, Jul 12

fjricci abandoned D35085: Respect exitcode sanitizer option in UBSan.

Looking further into it, _exit family functions don't get called on linux when return from main is used (GIexit in libc calls the exit syscall directly), so interception won't work. TSan and LSan get around this by calling exit() from their atexit() handlers, but we've established that we don't want to do this because of dlclose() issues. I think we're stuck with the current behavior, at least on Linux.

Wed, Jul 12, 2:35 PM
fjricci added a comment to D35085: Respect exitcode sanitizer option in UBSan.

Hmmm, it actually appears that even TSan doesn't successfully intercept _exit on Linux, only on Darwin (a CHECK(0) statement in OnExit() is never reached on Linux). If we can't successfully intercept _exit, and we can't exit from atexit, I'm not sure that there's a good way to do this.

Wed, Jul 12, 12:47 PM
fjricci added a comment to D35085: Respect exitcode sanitizer option in UBSan.

Only adding this for non-standalone builds makes testing hard, since you don't have consistent behavior between standalone and non-standalone ubsan builds. I'm debating leaving UBSan be, and just fixing this for the other sanitizers (ie making sure ASan returns failure when halt_on_error=0).

Wed, Jul 12, 11:52 AM
fjricci added a comment to D35085: Respect exitcode sanitizer option in UBSan.

I'll put up the partial solution where we respect exitcode in all sanitizers that already use interceptors when halt_on_error=0. I won't add interception to UBSan. It's unfortunate for me personally, since my codebases only use standalone UBSan, but I can understand why we wouldn't want to add interception to UBSan just for this one feature.

Wed, Jul 12, 9:57 AM

Tue, Jul 11

fjricci added a comment to D35085: Respect exitcode sanitizer option in UBSan.

There's a further complication with intercepting in UBSan - there doesn't seem to be a way to tell whether you're building standalone at compile time (although I could add a cmake define for that), so the UBSan interceptor will conflict with the TSan/ASan interceptor.

Tue, Jul 11, 4:08 PM
fjricci added a comment to D35085: Respect exitcode sanitizer option in UBSan.

The only issue I have come across is that standalone UBSan currently doesn't link against interception. If we want to intercept _exit, we'll need to add interception as a dependency. Is that something we're okay with?

Tue, Jul 11, 3:42 PM
fjricci committed rL307696: Use internal_strncpy to copy filename in linux procmaps.
Use internal_strncpy to copy filename in linux procmaps
Tue, Jul 11, 12:41 PM
fjricci closed D35136: Use internal_strncpy to copy filename in linux procmaps by committing rL307696: Use internal_strncpy to copy filename in linux procmaps.
Tue, Jul 11, 12:41 PM
fjricci committed rL307695: Inline function to get mac segment address range.
Inline function to get mac segment address range
Tue, Jul 11, 12:41 PM
fjricci closed D35270: Inline function to get mac segment address range by committing rL307695: Inline function to get mac segment address range.
Tue, Jul 11, 12:41 PM
fjricci committed rL307688: Refactor MemoryMappingLayout::Next to use a single struct instead of output….
Refactor MemoryMappingLayout::Next to use a single struct instead of output…
Tue, Jul 11, 11:54 AM
fjricci closed D35135: Refactor MemoryMappingLayout::Next to use a single struct instead of output parameters. NFC. by committing rL307688: Refactor MemoryMappingLayout::Next to use a single struct instead of output….
Tue, Jul 11, 11:54 AM
fjricci added inline comments to D35135: Refactor MemoryMappingLayout::Next to use a single struct instead of output parameters. NFC..
Tue, Jul 11, 11:10 AM
fjricci added inline comments to D35135: Refactor MemoryMappingLayout::Next to use a single struct instead of output parameters. NFC..
Tue, Jul 11, 11:09 AM
fjricci updated the diff for D35135: Refactor MemoryMappingLayout::Next to use a single struct instead of output parameters. NFC..

Address comments

Tue, Jul 11, 11:08 AM
fjricci created D35270: Inline function to get mac segment address range.
Tue, Jul 11, 11:08 AM

Mon, Jul 10

fjricci added inline comments to D35085: Respect exitcode sanitizer option in UBSan.
Mon, Jul 10, 1:19 PM
fjricci updated subscribers of D35085: Respect exitcode sanitizer option in UBSan.

It might be possible to change the default exitcode for UBSan, but I still think it should be possible for the user to make UBSan errors to return failure. Two primary reasons:

  • Consistency: As far as I'm aware, all of the other sanitizers return a failing exit code when they detect an error in a program, and I'm not sure why UBSan should be different in that respect.
  • Utility: A common use case I could imagine would be the case where a user wants to prevent UB regressions in their codebase by running a unit test suite with UBSan enabled. If it's not possible to return a failing exit code (either by default or otherwise), the test suite will continue passing just fine when UB is introduced.
Mon, Jul 10, 1:18 PM
fjricci added a comment to D34210: Add __has_feature(leak_sanitizer).

COFF supports weak externals: https://stackoverflow.com/a/11529277/382079. Would it suffice here?

Mon, Jul 10, 12:40 PM
fjricci added a comment to D35173: [lsan] Add _os_trace into LSan's suppression list.

Yes, I'm going to look into handling this via memory region scanning, but it's possible (even likely) that the performance hit will be prohibitive.

Mon, Jul 10, 12:38 PM · Restricted Project
fjricci added inline comments to D35085: Respect exitcode sanitizer option in UBSan.
Mon, Jul 10, 12:35 PM
fjricci added a comment to D34210: Add __has_feature(leak_sanitizer).

Yeah, I think that makes sense. Life will get a bit tricky when Windows support gets added and we can't use weak hooks to determine whether LSan is running, but I'm sure there's a way to handle that.

Mon, Jul 10, 9:58 AM
fjricci abandoned D34210: Add __has_feature(leak_sanitizer).
Mon, Jul 10, 9:47 AM

Sun, Jul 9

fjricci accepted D35173: [lsan] Add _os_trace into LSan's suppression list.
Sun, Jul 9, 6:34 AM · Restricted Project
fjricci added inline comments to D35085: Respect exitcode sanitizer option in UBSan.
Sun, Jul 9, 6:13 AM
fjricci created D35136: Use internal_strncpy to copy filename in linux procmaps.
Sun, Jul 9, 6:13 AM
fjricci created D35135: Refactor MemoryMappingLayout::Next to use a single struct instead of output parameters. NFC..
Sun, Jul 9, 6:13 AM

Fri, Jul 7

fjricci added a reviewer for D35085: Respect exitcode sanitizer option in UBSan: rsmith.
Fri, Jul 7, 8:17 AM

Thu, Jul 6

fjricci created D35085: Respect exitcode sanitizer option in UBSan.
Thu, Jul 6, 1:54 PM

Wed, Jun 28

fjricci abandoned D34772: Treat allocations within dispatch_once blocks as global allocations.

This doesn't provide as much of a performance benefit as I expected. In addition, the fact that a leak comes from a system library doesn't make it less of a leak. It's not a great user experience to see system library leaks in your program output, but I don't think there is much we can/should do about it.

Wed, Jun 28, 2:16 PM
fjricci added a comment to D34772: Treat allocations within dispatch_once blocks as global allocations.

An alternative solution here would be to add a supression for dispatch_once instead, but that would be less configurable (unless we required the user to provide it) and slightly less performant.

Wed, Jun 28, 12:40 PM
fjricci created D34772: Treat allocations within dispatch_once blocks as global allocations.
Wed, Jun 28, 12:38 PM

Tue, Jun 27

fjricci committed rL306463: Don't build tsan/dd when COMPILER_RT_HAS_TSAN is false.
Don't build tsan/dd when COMPILER_RT_HAS_TSAN is false
Tue, Jun 27, 2:11 PM
fjricci committed rL306455: Don't double-include cfi tests on linux.
Don't double-include cfi tests on linux
Tue, Jun 27, 12:52 PM
fjricci committed rL306453: Loop directly over sanitizers to build in cmake.
Loop directly over sanitizers to build in cmake
Tue, Jun 27, 12:33 PM
fjricci closed D34693: Loop directly over sanitizers to build in cmake by committing rL306453: Loop directly over sanitizers to build in cmake.
Tue, Jun 27, 12:32 PM
fjricci committed rL306450: Only test sanitizers that are built when COMPILER_RT_SANITIZERS_TO_BUILD is used.
Only test sanitizers that are built when COMPILER_RT_SANITIZERS_TO_BUILD is used
Tue, Jun 27, 12:18 PM
fjricci closed D34644: Only test sanitizers that are built when COMPILER_RT_SANITIZERS_TO_BUILD is used by committing rL306450: Only test sanitizers that are built when COMPILER_RT_SANITIZERS_TO_BUILD is used.
Tue, Jun 27, 12:18 PM
fjricci updated the diff for D34644: Only test sanitizers that are built when COMPILER_RT_SANITIZERS_TO_BUILD is used.

Add back cfi comment

Tue, Jun 27, 11:07 AM
fjricci added a comment to D34644: Only test sanitizers that are built when COMPILER_RT_SANITIZERS_TO_BUILD is used.

As far as I'm aware, the feature exists both to save time building and testing sanitizers you don't need, and to get around differing build requirements across the sanitizers (it's possible, especially when cross-compiling, that some sanitizers build in a given configuration and others do not).

Tue, Jun 27, 11:06 AM
fjricci added reviewers for D34644: Only test sanitizers that are built when COMPILER_RT_SANITIZERS_TO_BUILD is used: rnk, pcc, eugenis.
Tue, Jun 27, 10:29 AM
fjricci updated the diff for D34644: Only test sanitizers that are built when COMPILER_RT_SANITIZERS_TO_BUILD is used.

Test cfi even when cfi runtime isn't built

Tue, Jun 27, 10:28 AM
fjricci reopened D34644: Only test sanitizers that are built when COMPILER_RT_SANITIZERS_TO_BUILD is used.

Broke cfi in cases when the cfi runtime isn't built.

Tue, Jun 27, 10:28 AM
fjricci committed rL306431: Revert "Only test sanitizers that are built when….
Revert "Only test sanitizers that are built when…
Tue, Jun 27, 10:24 AM