[libcxx-commits] [PATCH] D158823: [libc++][hardening] Add back the safe mode.

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added inline comments.


================
Comment at: libcxx/CMakeLists.txt:53
 option(LIBCXX_ENABLE_STATIC "Build libc++ as a static library." ON)
 option(LIBCXX_ENABLE_FILESYSTEM
   "Whether to include support for parts of the library that rely on a filesystem being
----------------
ldionne wrote:
> ldionne wrote:
> > @thakis We brainstormed on some names here:
> > 
> > ```
> > _LIBCPP_ENABLE_HARDENED_PLUS_MODE
> > _LIBCPP_ENABLE_EXTENDED_HARDENED_MODE
> > _LIBCPP_ENABLE_STRONG_HARDENED_MODE
> > _LIBCPP_ENABLE_STRICT_MODE
> > _LIBCPP_ENABLE_PARANOID_MODE
> > _LIBCPP_ENABLE_FORTIFIED_MODE
> > _LIBCPP_ENABLE_SAFE_MODE
> > ```
> > 
> > Do you have any thoughts? Our thoughts so far:
> > 
> > ```
> > HARDENED_PLUS, EXTENDED_HARDENED, STRONG_HARDENED // those are kind of heavyweight names
> > PARANOID // has negative connotation and doesn't make it clear whether it is stronger than DEBUG
> > STRICT // could be confused with the notion of not having non-standard extensions
> > FORTIFIED // not clear whether it is stronger than HARDENED or not
> > SAFE // our current preference
> > ```
> > 
> > In fact, before LLVM 17 we had something called the `SAFE` mode, and I think what we discovered with Chromium's use case is that it still had its place. Hence, I think what we should do is call this the `SAFE` mode, backport this change, and rework the way we announced our 17 release notes not to say that we "replaced' the safe mode, but instead that we added new modes and that we changed how the safe mode is enabled. This is IMO a superior design and a superior way of rolling it out based on our experience so far.
> @Mordante Those are the other names we considered.
Thanks. If this list SAFE sounds the best to me too.


================
Comment at: libcxx/utils/libcxx/test/params.py:301-303
                 AddCompileFlag("-D_LIBCPP_ENABLE_HARDENED_MODE=1") if hardening_mode == "hardened" else None,
+                AddCompileFlag("-D_LIBCPP_ENABLE_SAFE_MODE=1")     if hardening_mode == "safe" else None,
                 AddCompileFlag("-D_LIBCPP_ENABLE_DEBUG_MODE=1")    if hardening_mode == "debug" else None,
----------------
ldionne wrote:
> Mordante wrote:
> > I start to feel slightly uncomfortable with these names. To me they are not very descriptive and they are now 4 options. I don't directly have better suggestions, but I think we should spend a bit of time on this.
> > 
> > For example, which is more expensive "safe" or "hardened"?
> I agree, this is definitely not perfect. This is user facing too, so we need to find something good.
> 
> We thought that `safe` was good given that it is the name we used for this mode historically (in LLVM 15 and LLVM 16). We also thought that it was "reasonably" clear that it was more expensive than `hardened`, but maybe it isn't. I'll CC you on the comment above that discusses naming.
Thanks. As said I don't have a better suggestion either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158823