This is an archive of the discontinued LLVM Phabricator instance.

Reduce the size of quarantine cache in ASAN_LOW_MEMORY configuration.
ClosedPublic

Authored by alekseyshl on Dec 16 2016, 5:27 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

alekseyshl updated this revision to Diff 81829.Dec 16 2016, 5:27 PM
alekseyshl retitled this revision from to Reduce the size of quarantine cache in ASAN_LOW_MEMORY configuration..
alekseyshl updated this object.
alekseyshl added reviewers: eugenis, kcc.
alekseyshl added a subscriber: llvm-commits.
eugenis accepted this revision.Dec 20 2016, 2:34 PM
eugenis edited edge metadata.

LGTM

lib/asan/asan_allocator.cc
235 ↗(On Diff #81829)

1 << 16

lib/asan/asan_internal.h
39 ↗(On Diff #81829)

Please remove a line in lib/asan/CMakeLists.txt that sets ASAN_LOW_MEMORY for Android.

This revision is now accepted and ready to land.Dec 20 2016, 2:34 PM

Btw ASAN_LOW_MEMORY was set for all 32-bit targets in r171052 by kcc.
I don't understand why.
IMHO, it should be a property of environment (i.e. OS), not architecture. If necessary, we can always test for 32-bit platform directly. And we do, see all the FIRST_32_SECOND_64 macros!

I'd support removing "|| (SANITIZER_WORDSIZE == 32)" from this definition.

alekseyshl updated this revision to Diff 82169.Dec 20 2016, 3:28 PM
alekseyshl edited edge metadata.
  • Define ASAN_LOW_MEMORY based on the target platform, not bitness.
alekseyshl marked an inline comment as done.Dec 20 2016, 3:29 PM
alekseyshl added inline comments.
lib/asan/asan_allocator.cc
235 ↗(On Diff #81829)

Sure, but doesn't it make it a bit less cryptic using 64 and Kb (1 << 10) than 1 << 16?

eugenis added inline comments.Dec 20 2016, 3:51 PM
lib/asan/asan_allocator.cc
235 ↗(On Diff #81829)

it's a matter of taste, I guess
It's also consistent with other constants in this file, like 1<<18 and 1<<20.

alekseyshl updated this revision to Diff 82173.Dec 20 2016, 4:11 PM
alekseyshl marked an inline comment as done.
  • Addressing comments.
This revision was automatically updated to reflect the committed changes.

The Darwin bots are failing the ThreadedQuarantineTest test: http://lab.llvm.org:8080/green/job/clang-stage1-configure-RA_check/25044/console

Can you please take a look?