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

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

In general quite happy with the patch, but some minor issues.



================
Comment at: libcxx/docs/HardenedMode.rst:2
+=============
+Hardened Mode
+=============
----------------
Is this name future-proof with all the other pre-condition violation checks you want to do?


================
Comment at: libcxx/docs/HardenedMode.rst:10-11
+
+Using the hardened mode
+====================
+
----------------



================
Comment at: libcxx/docs/HardenedMode.rst:15-16
+undefined behavior caused by violating preconditions of the standard library.
+These assertions can be done with relatively little overhead in constant time
+and are intended to be used in production by security-conscious projects.
+
----------------
I feel "by *security-conscious projects" is opinionated.
Even if I would not care about security since my device has no connection with the outside world I still may want to avoid undefined behavior.

I think the text is better without the last part. It already mentions "security" and "undefined behavior" before giving the reader enough information to whether or not they like this feature.



================
Comment at: libcxx/docs/HardenedMode.rst:25
+``LIBCXX_ENABLE_HARDENED_MODE`` option, and similarly enable the debug mode by
+building with the ``LIBCXX_ENABLE_DEBUG_MODE`` option.
+
----------------
Maybe list the CMake options with a short description of what they do.


================
Comment at: libcxx/docs/ReleaseNotes.rst:75
+- The library now provides a hardened mode under which common cases of library undefined behavior will be turned into
+  a reliable program crash. Vendors can configure whether the hardened mode is enabled by default with the
+  ``LIBCXX_HARDENING_MODE`` variable at CMake configuration time. Users can control whether the hardened mode is
----------------
crash has a negative meaning.


================
Comment at: libcxx/docs/index.rst:42
    Contributing
    Modules
    ReleaseProcedure
----------------
Please add HardenedMode here.


================
Comment at: libcxx/docs/index.rst:187
    DesignDocs/FileTimeType
+   DesignDocs/HardenedMode
    DesignDocs/HeaderRemovalPolicy
----------------
This file is not in design docs, so it does not belong here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153902