vitalybuka added a reviewer: rnk.
vitalybuka added a subscriber: rnk.
vitalybuka added a comment.
LGTM in general, but it can be cut into smaller patches.
+ @rnk for Windows suff
================
Comment at: compiler-rt/lib/lsan/lsan.h:20
+#elif SANITIZER_WINDOWS
+# include "lsan_win.h"
#endif
----------------
MyDeveloperDay wrote:
> clang-format?
@clemenswasser Can you please a separate tiny patch which clang-format nearby
lines
================
Comment at: compiler-rt/lib/lsan/lsan_allocator.cpp:30
#if defined(__i386__) || defined(__arm__)
-static const uptr kMaxAllowedMallocSize = 1UL << 30;
+static const uptr kMaxAllowedMallocSize = static_cast<uptr>(1) << 30;
#elif defined(__mips64) || defined(__aarch64__)
----------------
This also can go into a separate patch
BTW I would slightly prefer just UL -> ULL
================
Comment at: compiler-rt/lib/lsan/lsan_common.cpp:411
+# if !SANITIZER_WINDOWS
static void ProcessRootRegion(Frontier *frontier,
----------------
I guess do needs this as well on windows
The problem is that we have no MemoryMappingLayout implementation for Windows.
I guess it can be done with VirtualQueryEx
for now can you just ifdef only body of the ProcessRootRegion with TODO?
================
Comment at: compiler-rt/lib/lsan/lsan_common.h:48
+#elif SANITIZER_NETBSD || SANITIZER_FUCHSIA || SANITIZER_WINDOWS
+# define CAN_SANITIZE_LEAKS 1
#else
----------------
MyDeveloperDay wrote:
> you probably don't want to change this do you?
I am fine like this, fighting clang-format is not useful
however it would be nice if you clang-format relevant block in a separate patch
then then rebase this patch on top of it
================
Comment at: compiler-rt/lib/lsan/lsan_common_win.cpp:48
+
+void ProcessGlobalRegions(Frontier *frontier) {}
+void ProcessPlatformSpecificAllocations(Frontier *frontier) {}
----------------
how this can work without ProcessGlobalRegions?
================
Comment at: compiler-rt/lib/lsan/lsan_interceptors.cpp:506
+ INTERCEPT_FUNCTION(malloc);
+ INTERCEPT_FUNCTION(free);
+ INTERCEPT_FUNCTION(calloc);
----------------
Can you please just convert unneeded stuff from INTERCEPT_FUNCTION into
LSAN_MAYBE_INTERCEPT
In a separate patch
================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_win.cpp:4
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
----------------
sanitizer_stoptheworld_test.cpp needs to be ported to windows, probably with
std::
can you please move this work and sanitizer_stoptheworld_win.cpp into separate
patch/patches
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D115103/new/
https://reviews.llvm.org/D115103
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
