[PATCH] D136811: [-Wunsafe-buffer-usage] WIP: RFC: NFC: User documentation.

Tue Nov 22 00:29:50 PST 2022

steakhal added a comment.

As I was reading I'll highlighted some typos.
`compile time` -> `compile-time` /g

By the looks of it, this document is not referenced anywhere.
I believe `clang/docs/index.rst` should refer to this document in some place.

Thanks for the huge effort driving this you all!

================
Comment at: clang/docs/SafeBuffers.rst:27
+    annotate sections of code as either already-hardened or not-to-be-hardened,
+    enabling incremental adoption and and providing “escape hatches”
+    when unsafety is necessary;
----------------

================
Comment at: clang/docs/SafeBuffers.rst:38
+    at runtime against buffer overflows. This changes buffer overflows from
+    extremely dangerous undefined behavior to predictable runtime crash.
+  - Finally, in order to avoid bugs in newly converted code, the
----------------

================
Comment at: clang/docs/SafeBuffers.rst:45
+hardened libc++ can be used as a “sanitizer” to find buffer overflow bugs in
+existing code. The static analyzer checker is also universally useful,
+not tied to this programming model. The warning may be desired independently,
----------------

================
Comment at: clang/docs/SafeBuffers.rst:56
+
+At a glance it seems reasonable to ask the compiler to make raw pointer
+operations crash on out-of-bounds accesses at runtime. And in case of C, this
----------------

================
Comment at: clang/docs/SafeBuffers.rst:63
+either in a system of attributes significantly more advanced than
+the current solution, or a significant change in runtime representation
+of all pointers.
----------------
aaron.ballman wrote:
> It would be helpful to have some details here about why a user would pick these proposed features over use of a sanitizer like asan or hwasan, which also finds out of bounds accesses.

================
Comment at: clang/docs/SafeBuffers.rst:88
+   allocated with ``new[]``; it doesn’t assume unique ownership so you can copy
+   such span if you want, just like a raw pointer, but of course you’ll have to
+   manually ``delete[]`` it when appropriate.
----------------

================
Comment at: clang/docs/SafeBuffers.rst:108
+   Say, ``std::vector`` or ``std::span`` may turn out to be poorly suitable
+   for your needs, and this is fine. In such cases the same guidelines apply.
+   You may have to use the opt-out pragma on the implementation of
----------------

================
Comment at: clang/docs/SafeBuffers.rst:127
+  - Addition or subtraction of a number to/from a raw pointer with operators
+  ``+``, ``-``, ``+=``, ``-=``,
+      - unless that number is a compile time constant ``0``;
----------------
Sphinx is picky about indentation, and it also complains about the missing new-line prior to the sub-list.
The same also applies to the very next sub-list.

================
Comment at: clang/docs/SafeBuffers.rst:134
+      - a number of C/C++ standard library buffer manipulation functions
+        (such as std::memcpy() or std::next()) are hardcoded to act
+        as if they had the attribute.
----------------

================
Comment at: clang/docs/SafeBuffers.rst:138
+The warning doesn’t warn on single pointer use in general, such as
+dereference with operations like * and -> or [0]. If such operation causes
+a buffer overflow, there’s probably another unsafe operation nearby,
----------------
aaron.ballman wrote:
> 

================
Comment at: clang/docs/SafeBuffers.rst:148
+but only when the function is annotated with the attribute. The attribute gets
+auto-attached by automatic fixits, which let the warning and the fixes propagate
+across function boundaries, and the users are encouraged to annotate their
----------------
What does //auto-attached// mean?

================
Comment at: clang/docs/SafeBuffers.rst:176
+but also provide essential “escape hatches” when the programming model
+is undesirable for a section of code (such as tight loop in performance-critical
+code, or implementation of a custom container). In such cases sections of code
----------------

================
Comment at: clang/docs/SafeBuffers.rst:216
+outside the function are well-formed. For example, function ``bar()`` below
+doesn’t need an attribute, because assuming buf is well-formed (has size that
+fits the original buffer it refers to), hardened libc++ protects this function
----------------
aaron.ballman wrote:
> 

================
Comment at: clang/docs/SafeBuffers.rst:321
+
+In spirit of our guidelines, the automatic fixit would prefer this function
+to accept a span instead of raw buffer, so that the span didn't need to be
----------------

================
Comment at: clang/docs/SafeBuffers.rst:362
+of bounds checks. Variable ``size`` does not *have* to carry the size of the
+buffer (or the size of *that* buffer) just becaused it's named "size".
+The compiler will avoid making guesses about that.
----------------

================
Comment at: clang/docs/SafeBuffers.rst:369
+will demonstrate incorrect runtime behavior compared to the original code.
+In an otherwise well-formed program it is always possible (and usually easy)
+to resolve the placeholder correctly.
----------------

================
Comment at: clang/docs/SafeBuffers.rst:384
+will remain operational. A warning will be emitted if the replacement function's
+prototype diverges from the original prototype beyond recognition. In such cases
+an attribute can be either updated to give more manual hints to the compiler, or
----------------

Repository:
  rC Clang

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

https://reviews.llvm.org/D136811