[libcxx-commits] [PATCH] D153902: [libc++][hardening][NFC] Add macros to enable hardened mode.

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const added inline comments.


================
Comment at: libcxx/include/__config:231
+// Hardened mode checks.
+#if _LIBCPP_ENABLE_HARDENED_MODE
+
----------------
ldionne wrote:
> Mordante wrote:
> > The comment on line 238 `// TODO(hardening): more checks to be added here...` seem to suggest we get quite a bit of duplication in the `#if` and `#elif` branch. So I propose
> > 
> > ```
> > #if _LIBCPP_ENABLE_HARDENED_MODE || _LIBCPP_ENABLE_HARDENED_DEBUG_MODE
> > // do stuff for hardened and hardened debug
> > #endif // _LIBCPP_ENABLE_HARDENED_MODE || _LIBCPP_ENABLE_HARDENED_DEBUG_MODE
> > 
> > #if _LIBCPP_ENABLE_HARDENED_DEBUG_MODE
> > // do stuff only for hardened debug
> > #endif // _LIBCPP_ENABLE_HARDENED_DEBUG_MODE
> > ```
> Considering there will be only a small number of categories (at least at first), IMO it will end up being easier to read by listing all of them explicitly in each mode. We can also cross that bridge once we get to it in an upcoming patch.
I'm open to doing that in the future if we have enough categories that duplication becomes a concern. For now, I'd prefer to have each mode separate (I find it convenient to be able to focus on one mode in isolation).


================
Comment at: libcxx/include/__config:258
+
+// TODO: more checks to be added here...
+
----------------
ldionne wrote:
> Mordante wrote:
> > Do we really want to disable things? If I only want to define `_LIBCPP_ABI_BOUNDED_ITERATORS` is that prohibited?
> > If I only want to define `_LIBCPP_ABI_BOUNDED_ITERATORS` is that prohibited?
> 
> No, that's allowed. ABI flags are disjoint from the hardening checks, so you could enable bounded iterators but if you don't enable any hardening, the in-bounds assertions inside `__bounded_iter` would be disabled. The intent is that vendors would decide on those ABI flags at configuration time, since users can't change them on a per-TU basis.
@Mordante I'm doing this only to avoid compiler warnings -- it's to make sure every macro is defined and set to either `0` or `1`. Otherwise, the compiler would issue a warning each time one of these macros is referred to. I can't set them all to `0` beforehand since that would produce a warning about redefining macros.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153902