Skip to content

Conversation

rapidsna
Copy link
Contributor

-fbounds-safety doesn't allow taking address of a variable referred to by __counted_by, in order to prevent code from using the pointer to update the variable without necessary checks to keep the invariant of __counted_by.

As shown in the example below, with -fbounds-safety the compiler ensures that the __counted_by pointer/or array has at least as many as elements that the attribute indicates, by requiring the count and the buf are always updated side by side and emitting run-time checks to ensure the new values are valid.

struct counted_buf {
  sized_t count;
  int *__counted_by(count) buf;
};

// BEFORE FIX
void foo(struct counted_buf *p) {
  p->count = 10; // error: assignment to 'count' requires corresponding assignment to 'int *__counted_by(count) buf')
}

// FIXED by adding assignment to `p->buf`
void foo(struct counted_buf *p) {
  // The compiler is now happy because  
  p->buf = (int *)malloc(sizeof(int) *5); // run-time checks are emitted to make sure new buf has at least `10` elements. 
  p->count = 10;
}

Consequently, -fbounds-safety prevents taking address of a variable referred to by __counted_by, because otherwise, the compiler cannot check the updates through the pointer pointing to the count:

void foo(struct counted_buf *p) {
  int *count_p = &p->count; // error: variable referred to by '__counted_by' cannot be pointed to by any other variable
 *count_p = 10; // without the above error, `count_p` is a normal `int *` so the compiler cannot check the value it updates against `__counted_by`
}

This PR is to explicitly specify this restriction and avoid future conflicts.

@rapidsna rapidsna added the clang:bounds-safety Issue/PR relating to the experimental -fbounds-safety feature in Clang label Aug 26, 2024
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Aug 26, 2024
@rapidsna rapidsna changed the title Specify taking address of a variable referred to by '__counted_by' is forbidden [BoundsSafety][NFC] Specify taking address of a variable referred to by '__counted_by' is forbidden Aug 26, 2024
@llvmbot
Copy link
Member

llvmbot commented Aug 26, 2024

@llvm/pr-subscribers-clang

Author: Yeoul Na (rapidsna)

Changes

-fbounds-safety doesn't allow taking address of a variable referred to by __counted_by, in order to prevent code from using the pointer to update the variable without necessary checks to keep the invariant of __counted_by.

As shown in the example below, with -fbounds-safety the compiler ensures that the __counted_by pointer/or array has at least as many as elements that the attribute indicates, by requiring the count and the buf are always updated side by side and emitting run-time checks to ensure the new values are valid.

struct counted_buf {
  sized_t count;
  int *__counted_by(count) buf;
};

// BEFORE FIX
void foo(struct counted_buf *p) {
  p->count = 10; // error: assignment to 'count' requires corresponding assignment to 'int *__counted_by(count) buf')
}

// FIXED by adding assignment to `p->buf`
void foo(struct counted_buf *p) {
  // The compiler is now happy because  
  p->buf = (int *)malloc(sizeof(int) *5); // run-time checks are emitted to make sure new buf has at least `10` elements. 
  p->count = 10;
}

Consequently, -fbounds-safety prevents taking address of a variable referred to by __counted_by, because otherwise, the compiler cannot check the updates through the pointer pointing to the count:

void foo(struct counted_buf *p) {
  int *count_p = &p->count; // error: variable referred to by '__counted_by' cannot be pointed to by any other variable
 *count_p = 10; // without the above error, `count_p` is a normal `int *` so the compiler cannot check the value it updates against `__counted_by`
}

This PR is to explicitly specify this restriction and avoid future conflicts.


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

1 Files Affected:

  • (modified) clang/docs/BoundsSafety.rst (+18-1)
diff --git a/clang/docs/BoundsSafety.rst b/clang/docs/BoundsSafety.rst
index 8fd655663edb00..e4ddd3c62db65d 100644
--- a/clang/docs/BoundsSafety.rst
+++ b/clang/docs/BoundsSafety.rst
@@ -759,7 +759,24 @@ relationship must hold even after any of these related variables are updated. To
 this end, the model requires that assignments to ``buf`` and ``count`` must be
 side by side, with no side effects between them. This prevents ``buf`` and
 ``count`` from temporarily falling out of sync due to updates happening at a
-distance.
+distance. In addition, taking address of ``count`` is not allowed in order to 
+prevent the programmers from updating the ``count`` through the pointer, which
+will evade the necessary checks to make ``count`` and ``buf`` in sync.
+
+.. code-block:: c
+
+   struct counted_buf {
+      int *__counted_by(count) buf;
+      size_t count;
+   };
+
+   void foo(struct counted_buf *p) {
+      int *pointer_to_count = &p->count; // error: variable referred to by
+      // '__counted_by' cannot be pointed to by any other variable; exception is
+      // when the pointer is passed as a compatible argument to a function.
+      *pointer_to_count = SIZE_MAX; // Without reporting the error above, the
+      // compiler cannot prevent count from getting an invalid value.   
+   }
 
 The example below shows a function ``alloc_buf`` that initializes a struct that
 members that use the ``__counted_by`` annotation. The compiler allows these

Copy link
Member

@hnrklssn hnrklssn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good, just a small clarification

void foo(struct counted_buf *p) {
int *pointer_to_count = &p->count; // error: variable referred to by
// '__counted_by' cannot be pointed to by any other variable; exception is
// when the pointer is passed as a compatible argument to a function.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should define what a compatible argument is

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:bounds-safety Issue/PR relating to the experimental -fbounds-safety feature in Clang clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants