Not a known issue - no, but MSan doesn't play nice with uninistrumented libraries (including things like libcxx) - and so it can be tricky to ensure your build is properly sanitized, which is why I'd recommend the build script :).

Looks like this patch broke the MSan buildbots, PTAL (repro instructions https://github.com/google/sanitizers/wiki/SanitizerBotReproduceBuild):

LGTM

Mostly just some nits - other than that LGTM.

LGTM (after the keep_seed patch lands)

Still LGTM - can't validate the Fuchsia change on my end but Linux still seems fine.

Is it possible to fork the cross_over_uniformdist changes into a supplementary patchset easily? Seems like an related but indepentent change (prefer one patchset per feature if possible please -- reviewing is O(n^3) in lines-of-code-per-patch and it's also nice to have different commits for different features).

In D86399#2233735, @cryptoad wrote:

In D86399#2232864, @vitalybuka wrote:

Interesting that GCC does not inline capture by ref https://godbolt.org/z/exfP69

Would you recommend some change to ensure inlining?

Hi folks, looks like this patch broke check-clang under ASan/MSan, would you mind taking a look? Thanks.

In rG219f32f4b68679563443cdaae7b8174c9976409a#938914, @RKSimon wrote:

In rG219f32f4b68679563443cdaae7b8174c9976409a#938901, @hctim wrote:

Hi Simon, looks like this patch breaks bots that use -Wsign-compare (http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/28842/steps/ninja%20check-asan/logs/stdio).

I've reverted in 66e7dce714fabd3ddb1aed635e4b826476d4f1a2 to bring the bots back online

np, cheers - I don't appear to have received any buildbot failure emails which is a bit odd. I'll resubmit in the morning.

Hi Simon, looks like this patch breaks bots that use -Wsign-compare (http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/28842/steps/ninja%20check-asan/logs/stdio).

Fix an OOB operand problem. ISel is smart enough to figure out the MO_G3
relocation should use a 48-bit shift, but assert()'s internally if the third
operand isn't specified.

• Fix up 'dealloc' typo.

Apparently static constexpr declarations and initializers go in headers,
but they still require declarations to avoid link-time undefined
reference complaints (except after c++17). Learn something new every day, I guess.

Revert of the original changes was already done in 540b92147da0e9a2c2d096505eb0785ec79f2024 :) - a rebase should fix it up.

• Refactor the backtrace code into a common core.

• And patch in the changes to scudo/standalone.

• Remove vestigal changes to sanitizer_common.

+@cryptoad as this touches non-standalone scudo, but to a very limited degree.

Correct diff.

LGTM

LGTM w/ nit

LGTM

LGTM

In D83494#2148180, @morehouse wrote:

In D83494#2148164, @dokyungs wrote:

Right. Apparently with -O2 many calls to memcmp-like functions are removed. I just wondered, though, what makes more sense: disabling such optimization when building (i) with sancov, or (ii) with -fsanitize=fuzzer? If we go for (i), would it make sense to do it in the SanitizerCoverage module pass like other sanitizers do? What do you think? Also, can it be addressed in a follow-up patch?

My opinion is to make it part of -fsanitize=fuzzer, or maybe disable parts of the builtin optimization pass for functions with the OptForFuzzing attribute.

And I think a follow-up patch is easier to review.

In D83494#2147608, @dokyungs wrote:

Builtin libfunc optimizations may transform memcmp and strcmp-like functions. To disable such optimizations, -fno-builtin= flag was additionally added in compiling new test cases. FWIW, the original test cases didn't require such flags since other sanitizers including ASan disables those optimizations in their LLVM pass by dropping libfunc attribute in the call instructions.

Thanks!

So - the FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION flag is a property of the build system and not that of the compiler. There are some places (android) where enabling FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION globally changes the behaviour of large amounts of libraries in ways that break the build system.

Changed the filename and fixed up the inverted test.

Hi @arsenm - does this patch LGTY? Still unsure about whether we need to notify the CO here, if you could help advise that would be much appreciated. Thanks.

In D82615#2117979, @aemerson wrote:

I'm not familiar with these types globals. Do you think there's any issue with the current design that would make it difficult to generate optimal code in future?

• Remove vestigal operand and set regclass properly.

• Reformat exported-tagged-globalc.

Abandoning - fixing the underlying issue at D82615.

First patchset was the wrong diff. Should be the right one now.

Reverted in 10045cbe01928d9281723c640c03984d540f9012 to bring the ASan bots green again.

In D82249#2110010, @arsenm wrote:

I don't follow. It no longer falls back, so what is the problem?

In D82249#2109920, @eugenis wrote:

I'm OK with this as a workaround, but it would be more natural to detect the unsupported IR pattern in globalisel and fall back instead of disabling it entirely. Is it difficult to do for some reason?

In D82249#2105036, @arsenm wrote:

Is the fallback not working correctly in this case for some reason?

In D82031#2101508, @cferris wrote:

In D82031#2101385, @hctim wrote:

How does this change look w.r.t. a memory replay test (https://android.googlesource.com/platform/bionic/+/master/docs/native_allocator.md#memory-replay-benchmarks)?

Is it possible to get a before/after of RSS/USS and CPU? This change seems like a very reasonable strategy to me, but I feel like I'm flying blind.

I'm planning to run perf tests on this. Unfortunately, I lost the device I was using last night, so I couldn't run the data properly. I can put up the data here when I'm done.

How does this change look w.r.t. a memory replay test (https://android.googlesource.com/platform/bionic/+/master/docs/native_allocator.md#memory-replay-benchmarks)?

LGTM, thanks!

LGTM - thanks.

Looks like D80599 didn't squash properly before being upstreamed, this commit is still unreviewed and for all intents and purposes unsubmitted (and still needs review).

Looks like my local commits didn't get squashed before upstream, but the net impact was [base .. a744142] (i.e. the last version before commit).