[PATCH] D60593: [GwpAsan] Introduce GWP-ASan.

[Mitch Phillips via Phabricator via llvm-commits]

hctim added inline comments.


================
Comment at: compiler-rt/lib/gwp_asan/optional/runtime_env_flag_parser.cpp:26
+  va_end(args);
+  return 0;
+}
----------------
morehouse wrote:
> May as well make this return `void` if we're always returning 0.
fn is removed.


================
Comment at: compiler-rt/lib/gwp_asan/optional/runtime_env_flag_parser.cpp:48
+  return (&__gwp_asan_default_options) ? __gwp_asan_default_options() : "";
+}
+
----------------
morehouse wrote:
> Unnamed namespace would allow us to remove all these `statics` on variables and functions.
Removed them without an unnamed namespace. This okay?


================
Comment at: compiler-rt/lib/gwp_asan/optional/runtime_env_flag_parser.cpp:58
+    OverrideCommonFlags(cf);
+  }
+  Options *o = getOptions();
----------------
morehouse wrote:
> Do we care about any of the `sanitizer_common` flags?
Nope, removed.


================
Comment at: compiler-rt/lib/gwp_asan/optional/runtime_env_flag_parser.cpp:99
+
+#if !SANITIZER_SUPPORTS_WEAK_HOOKS
+SANITIZER_INTERFACE_WEAK_DEF(const char *, __gwp_asan_default_options, void) {
----------------
morehouse wrote:
> Why the `#if`?
I could have sworn that this failed to build without it, but it appears to be a non-issue now. Removed.


================
Comment at: compiler-rt/lib/gwp_asan/options.h:27
+#include "gwp_asan/options.inc"
+#undef GWP_ASAN_OPTION
+
----------------
morehouse wrote:
> Seems we have potential double-initialization of the global options:
> 
> # We call `initOptions` before the `GwpAsanFlags` constructor runs.
> # `GwpAsanFlags` constructor runs and sets back to defaults.
Yep, fixed the init-order-fiasco with moving to on-demand instantiation inside of `getOptions()`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60593/new/

https://reviews.llvm.org/D60593