[BoundsSafety][Doc] Add BoundsSafetyAdoptionGuide.rst #120674

rapidsna · 2024-12-20T03:27:15Z

This adds an instruction to adopt -fbounds-safety using the preview implementation available in the fork of llvm-project.

llvmbot · 2024-12-20T03:27:49Z

@llvm/pr-subscribers-clang

Author: Yeoul Na (rapidsna)

Changes

This adds an instruction to adopt -fbounds-safety using the preview implementation available in the fork of llvm-project.

Full diff: https://github.com/llvm/llvm-project/pull/120674.diff

3 Files Affected:

(modified) clang/docs/BoundsSafety.rst (+8-1)
(added) clang/docs/BoundsSafetyAdoptionGuide.rst (+87)
(modified) clang/docs/index.rst (+1)

diff --git a/clang/docs/BoundsSafety.rst b/clang/docs/BoundsSafety.rst
index 8fd655663edb00..e24c69d8c7855f 100644
--- a/clang/docs/BoundsSafety.rst
+++ b/clang/docs/BoundsSafety.rst
@@ -996,4 +996,11 @@ and the soundness of the type system. This may incur significant code size
 overhead in unoptimized builds and leaving some of the adoption mistakes to be
 caught only at run time. This is not a fundamental limitation, however, because
 incrementally adding necessary static analysis will allow us to catch issues
-early on and remove unnecessary bounds checks in unoptimized builds.
\ No newline at end of file
+early on and remove unnecessary bounds checks in unoptimized builds.
+
+Try it out
+==========
+
+Your feedback on the programming model is valuable. You may want to follow the
+instruction in :doc:`BoundsSafetyAdoptionGuide` to play with ``-fbounds-safety``
+and please send your feedback to `Yeoul Na <mailto:[email protected]>`_.
\ No newline at end of file
diff --git a/clang/docs/BoundsSafetyAdoptionGuide.rst b/clang/docs/BoundsSafetyAdoptionGuide.rst
new file mode 100644
index 00000000000000..b19d9188fe26a2
--- /dev/null
+++ b/clang/docs/BoundsSafetyAdoptionGuide.rst
@@ -0,0 +1,87 @@
+======================================
+Adoption Guide for ``-fbounds-safety``
+======================================
+
+.. contents::
+   :local:
+
+Where to get ``-fbounds-safety``
+================================
+
+The open sourcing to llvm.org's ``llvm-project`` is still on going and the
+feature is not available yet. In the mean time, the preview implementation is
+available
+`here <https://github.com/swiftlang/llvm-project/tree/stable/20240723>`_ in a
+fork of ``llvm-project``. Please follow
+`Building LLVM with CMake <https://llvm.org/docs/CMake.html>`_ to build the
+compiler.
+
+Feature flag
+============
+
+Pass ``-fbounds-safety`` as a Clang compilation flag for the C file that you
+want to adopt. We recommend adopting the model file by file, because adoption
+requires some effort to add bounds annotations and fix compiler diagnostics.
+
+Include ``ptrcheck.h``
+======================
+
+``ptrcheck.h`` is a Clang toolchain header to provide definition of the bounds
+annotations such as ``__counted_by``, ``__counted_by_or_null``, ``__sized_by``,
+etc. In the LLVM source tree, the header is located in
+``llvm-project/clang/lib/Headers/ptrcheck.h``.
+
+
+Add bounds annotations on pointers as necessary
+===============================================
+
+Annotate pointers on struct fields and function parameters if they are pointing
+to an array of object, with appropriate bounds annotations. Please see
+:doc:`BoundsSafety` to learn what kind of bounds annotations are available and
+their semantics. Note that local pointer variables typically don't need bounds
+annotations because they are implicitely a wide pointer (``__bidi_indexable``)
+that automatically carries the bounds information.
+
+Address compiler diagnostics
+============================
+
+Once you pass ``-fbounds-safety`` to compiler a C file, you will see some new
+compiler warnings and errors, which will lead you to adopt the feature and
+to remove unsafeness in the code. Consider the following example:
+
+.. code-block:: c
+
+   #include <ptrcheck.h>
+
+   void init_buf(int *p, int n) {
+      for (int i = 0; i < n; ++i)
+         p[i] = 0; // error: array subscript on single pointer 'p' must use a constant index of 0 to be in bounds
+   }
+
+The parameter ``int *p`` doesn't have a bounds annotation, so the compiler will
+complain about the code indexing into it (``p[i]``). To address the diagnostics,
+you should add a bounds annotation on ``int *p`` so that the compiler can reason
+about the safety of the array subscript. In the following example, ``p`` is now
+``int *__counted_by(n)``, so the compiler will allow the array subscript with
+additional run-time checks as necessary.
+
+.. code-block:: c
+
+   #include <ptrcheck.h>
+
+   void init_buf(int *__counted_by(n) p, int n) {
+      for (int i = 0; i < n; ++i)
+         p[i] = 0; // ok; `p` is now has a type with bounds annotation.
+   }
+
+Run unit tests to fix new run-time traps
+========================================
+
+Adopting ``-fbounds-safety`` may cause your program to trap if it violates
+bounds safety or it has incorrect adoption.
+
+Repeat the process for each remaining file
+==========================================
+
+Once you've done with adopting a single C file, please repeat the same process
+for each remaining C file that you want to adopt.
\ No newline at end of file
diff --git a/clang/docs/index.rst b/clang/docs/index.rst
index cc070059eede5d..349378b1efa214 100644
--- a/clang/docs/index.rst
+++ b/clang/docs/index.rst
@@ -40,6 +40,7 @@ Using Clang as a Compiler
    SanitizerStats
    SanitizerSpecialCaseList
    BoundsSafety
+   BoundsSafetyAdoptionGuide
    BoundsSafetyImplPlans
    ControlFlowIntegrity
    LTOVisibility

hnrklssn · 2024-12-20T11:09:19Z

Given the number of small changes that need to be made, I think it's worth mentioning that fix-its exist and how their fixes can be applied, since that's not always obvious when building in the terminal. Perhaps we also want to mention that we'd appreciate reports of snippets that compile without errors but are optimised to unconditional traps?

rapidsna · 2024-12-22T22:09:48Z

Given the number of small changes that need to be made, I think it's worth mentioning that fix-its exist and how their fixes can be applied, since that's not always obvious when building in the terminal. Perhaps we also want to mention that we'd appreciate reports of snippets that compile without errors but are optimised to unconditional traps?

Sounds good. Let's add those in a follow up since we also need to add debugger support.

hnrklssn

If nobody else has any comments, I'd say LGTM

delcypher

Seems reasonable and something we can expand on. The main issue is the fact -fbounds-safety is a cc1 option while this doc is written assuming its a driver option.

delcypher · 2025-01-16T02:04:49Z