[GWP-ASan] Respect GWP_ASAN_DEFAULT_ENABLED compile-time macro

[mcgrathr]

Summary

If the containing allocator build uses -DGWP_ASAN_DEFAULT_ENABLED=false
then the option will default to false. For e.g. Scudo, this is simpler
and more efficient than using -DSCUDO_DEFAULT_OPTIONS=... to set gwp-asan
options that have to be parsed from the string at startup.

[hctim]

I'm not sure adding this complexity here is warranted, why is -DGWP_ASAN_DEFAULT_ENABLED=false better than -DSCUDO_DEFAULT_OPTIONS=GWP_ASAN_Enabled=false?

This does have some niceties that it patches up the text description, but nothing else in Scudo does that if you use -DSCUDO_DEFAULT_OPTIONS, and the inconsistency might be misleading.

[mcgrathr]

In D91463#2395332, @hctim wrote:

I'm not sure adding this complexity here is warranted, why is -DGWP_ASAN_DEFAULT_ENABLED=false better than -DSCUDO_DEFAULT_OPTIONS=GWP_ASAN_Enabled=false?

The complexity of one optional macro with a Boolean value seems quite minimal to me. The benefit is having the value simply linker-initialized rather than having every process on every device repeat the same runtime work at startup to parse a string and poke a variable (possibly causing an otherwise unnecessary COW page).

This does have some niceties that it patches up the text description, but nothing else in Scudo does that if you use -DSCUDO_DEFAULT_OPTIONS, and the inconsistency might be misleading.

I don't particularly care about the description text, it just seemed easy to do. If you'd prefer to leave the description text fixed (and thus inconsistent with non-default defaults but consistent with other means of changing the defaults) that's fine with me.

Thanks for the review!

[hctim]

In D91463#2395388, @mcgrathr wrote:

In D91463#2395332, @hctim wrote:

I'm not sure adding this complexity here is warranted, why is -DGWP_ASAN_DEFAULT_ENABLED=false better than -DSCUDO_DEFAULT_OPTIONS=GWP_ASAN_Enabled=false?

The complexity of one optional macro with a Boolean value seems quite minimal to me. The benefit is having the value simply linker-initialized rather than having every process on every device repeat the same runtime work at startup to parse a string and poke a variable (possibly causing an otherwise unnecessary COW page).

Sure, this sounds like a good enough reason to me.

compiler-rt/lib/gwp_asan/options.inc

16

Nit: Remove -DGWP_ASAN_DEFAULT_ENABLED_STRING and use a compiler-rt-like stringify:

#define GWP_ASAN_STRINGIFY_(S) #S
#define GWP_ASAN_STRINGIFY(S) GWP_ASAN_STRINGIFY_(S)

#ifndef GWP_ASAN_DEFAULT_ENABLED
#define GWP_ASAN_DEFAULT_ENABLED true
#endif
GWP_ASAN_OPTION(bool, Enabled, GWP_ASAN_DEFAULT_ENABLED,
                "Is GWP-ASan enabled? Defaults to " GWP_ASAN_STRINGIFY(
                    GWP_ASAN_DEFAULT_ENABLED) ".")

[mcgrathr]

macro nit

[mcgrathr]

updated macro

compiler-rt/lib/gwp_asan/options.inc

 //===-- options.inc ---------------------------------------------*- C++ -*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef GWP_ASAN_OPTION
 #error "Define GWP_ASAN_OPTION prior to including this file!"
 #endif
 
-GWP_ASAN_OPTION(bool, Enabled, true, "Is GWP-ASan enabled? Defaults to true.")
+#ifndef GWP_ASAN_DEFAULT_ENABLED
+#define GWP_ASAN_DEFAULT_ENABLED true
+#endif
+
+#ifndef GWP_ASAN_STRINGIFY
+#define GWP_ASAN_STRINGIFY(S) GWP_ASAN_STRINGIFY_(S)
+#define GWP_ASAN_STRINGIFY_(S) #S
+#endif
+
+GWP_ASAN_OPTION(bool, Enabled, GWP_ASAN_DEFAULT_ENABLED,
+                "Is GWP-ASan enabled? Defaults to " GWP_ASAN_STRINGIFY(
+                    GWP_ASAN_DEFAULT_ENABLED) ".")
 
 GWP_ASAN_OPTION(
     bool, PerfectlyRightAlign, false,
     "When allocations are right-aligned, should we perfectly align them up to "
     "the page boundary? By default (false), we round up allocation size to the "
     "nearest power of two (1, 2, 4, 8, 16) up to a maximum of 16-byte "
     "alignment for performance reasons. For Bionic, we use 8-byte alignment by "
     "default. Setting this to true can find single byte buffer-overflows for "