https://github.com/barcharcraz closed
https://github.com/llvm/llvm-project/pull/81677
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/barcharcraz edited
https://github.com/llvm/llvm-project/pull/81677
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -35,6 +35,9 @@
// RUN: %p/../../../../lib/sanitizer_common/sanitizer_coverage_interface.inc
\
// RUN: | grep -e "INTERFACE_\(WEAK_\)\?FUNCTION"
\
// RUN: | grep -v "__sanitizer_weak_hook"
\
+//
MaskRay wrote:
> It's LGTM, but it's going to be annoying to merge. I bet this patch has
> several reasons to be reverted, and will be reverted multiple times. It would
> be nice to try to split into smaller parts.
I agree.
It'd be nice if Chromium folks can test this before this is merged.
vitalybuka wrote:
It's LGTM, but it's going to be annoying to merge.
I bet this patch has several reasons to be reverted, and will be reverted
multiple times.
It would be nice to try to split into smaller parts.
https://github.com/llvm/llvm-project/pull/81677
@@ -35,6 +35,9 @@
// RUN: %p/../../../../lib/sanitizer_common/sanitizer_coverage_interface.inc
\
// RUN: | grep -e "INTERFACE_\(WEAK_\)\?FUNCTION"
\
// RUN: | grep -v "__sanitizer_weak_hook"
\
+//
barcharcraz wrote:
@rnk @vitalybuka
This should be ready for a more in-depth review (I'll stop force-pushing)
The remaining commits here all depend on each-other in some way. Splitting off
any more commits would mean introducing bugs into main until other parts of
this are merged.
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff ce37a7131ff33bbd04bd1893de78c1bd3a1f0897
7affdb487a88f0c7b31ec20c027ceeaa92f2fe4a --
https://github.com/barcharcraz edited
https://github.com/llvm/llvm-project/pull/81677
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
barcharcraz wrote:
> > @vitalybuka Here's a more detailed explination on the motivations behind
> > this change from @amyw-msft, who is the original author of these changes on
> > our side. (And who I'll add as a co-author using fixup commits)
> >
vitalybuka wrote:
> @vitalybuka Here's a more detailed explination on the motivations behind this
> change from @amyw-msft, who is the original author of these changes on our
> side. (And who I'll add as a co-author using fixup commits)
>
>
barcharcraz wrote:
@vitalybuka Here's a more detailed explination on the motivations behind this
change from @amyw-msft, who is the original author of these changes on our
side. (And who I'll add as a co-author using fixup commits)
barcharcraz wrote:
> Is it feasible to create a migration path from the static runtime to the
> dynamic runtime? As written, this requires users to update their ASan builds
> at the same time that they take this update. Is it possible to leave the
> static runtime behind, create a migration
https://github.com/rnk edited https://github.com/llvm/llvm-project/pull/81677
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -0,0 +1,81 @@
+//===-- sanitizer_win_thunk_interception.h -
-===//
+//
+// 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:
https://github.com/rnk commented:
Is it feasible to create a migration path from the static runtime to the
dynamic runtime? As written, this requires users to update their ASan builds at
the same time that they take this update. Is it possible to leave the static
runtime behind, create a
https://github.com/rnk edited https://github.com/llvm/llvm-project/pull/81677
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rnk wrote:
I'm short on time, so I can't respond to all the discussion here, but at a high
level, I agree with this direction and it's something I discussed with
Microsoft folks working on ASan in the past, so I'm happy to see it come
together.
There is a great deal of build configuration
barcharcraz wrote:
The buildkite failure is just windows defender freaking out about one of
cmake's test exes. Presumably it'll get sorted out next time it runs.
https://github.com/llvm/llvm-project/pull/81677
___
cfe-commits mailing list
https://github.com/barcharcraz ready_for_review
https://github.com/llvm/llvm-project/pull/81677
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
mstorsjo wrote:
> > Not to distract from the direction taken here (which I do agree seems
> > reasonable, even if I haven't had time to look closer at the PR yet) - but,
> > when using the static CRT in general, doesn't the same concern apply there
> > as well? I.e. when using the static CRT,
barcharcraz wrote:
It occurs to me that this probably requires changes to the gyp build files.
https://github.com/llvm/llvm-project/pull/81677
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
barcharcraz wrote:
> > The core reasoning is that asan is a "only one allowed per process" type
> > thing (you can't have one copy of the asan runtime handling a malloc while
> > a different one handles the corresponding free).
>
> Not to distract from the direction taken here (which I do
mstorsjo wrote:
> The core reasoning is that asan is a "only one allowed per process" type
> thing (you can't have one copy of the asan runtime handling a malloc while a
> different one handles the corresponding free).
Not to distract from the direction taken here (which I do agree seems
barcharcraz wrote:
> I think @rnk has the deepest knowledge here.
>
> Just to double check, when you say "Eliminate the static asan runtime", you
> mean the use case it was supporting will still keep working, but in a
> different way right?
Yes. The static asan runtime didn't work very well
zmodem wrote:
I think @rnk has the deepest knowledge here.
Just to double check, when you say "Eliminate the static asan runtime", you
mean the use case it was supporting will still keep working, but in a different
way right?
https://github.com/llvm/llvm-project/pull/81677
barcharcraz wrote:
> This is long description, but it does not explain WHY. Could you please add
> some explanation there?
Sure, the reason is to greatly simplify how asan works on windows, and to fix a
bunch of bugs with asan+static CRT. Before this change there was a bunch of
code that
tru wrote:
cc @sylvain-audi
https://github.com/llvm/llvm-project/pull/81677
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/vitalybuka commented:
This is long description, but it does not explain WHY.
Could you please add some explanation there?
https://github.com/llvm/llvm-project/pull/81677
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
vitalybuka wrote:
Added know some folks who may know how Asan in used by Chromium on Windows.
https://github.com/llvm/llvm-project/pull/81677
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
github-actions[bot] wrote:
:warning: Python code formatter, darker found issues in your code. :warning:
You can test this locally with the following command:
``bash
darker --check --diff -r
4af24d4ab76539706bfbceec4b3923426fb1b9e7...c7eaa51d14fca4fac23ad8e5a937a8494f031eeb
llvmbot wrote:
@llvm/pr-subscribers-compiler-rt-sanitizer
Author: Charlie Barto (barcharcraz)
Changes
This is one of the major changes we (Microsoft) have made in the version of
asan we ship with Visual Studio.
Here's the description of these changes from our internal PR
1. Build one
32 matches
Mail list logo
