On 2025-06-16 18:08, Qing Zhao wrote:
gcc/ChangeLog:

        * tree-object-size.cc (access_with_size_object_size): Handle pointers
        with counted_by.

This should probably just say "Update comment for .ACCESS_WITH_SIZE.".

        (collect_object_sizes_for): Likewise.

gcc/testsuite/ChangeLog:

        * gcc.dg/pointer-counted-by-4.c: New test.
        * gcc.dg/pointer-counted-by-5.c: New test.
        * gcc.dg/pointer-counted-by-6.c: New test.
        * gcc.dg/pointer-counted-by-7.c: New test.
---

I can't approve, but here's a review. In summary, I've suggested some modifications to the tests, the tree-object-size change itself looks fine to me.

  gcc/testsuite/gcc.dg/pointer-counted-by-4.c | 63 +++++++++++++++++++++
  gcc/testsuite/gcc.dg/pointer-counted-by-5.c | 48 ++++++++++++++++
  gcc/testsuite/gcc.dg/pointer-counted-by-6.c | 47 +++++++++++++++
  gcc/testsuite/gcc.dg/pointer-counted-by-7.c | 30 ++++++++++
  gcc/tree-object-size.cc                     | 12 +++-
  5 files changed, 197 insertions(+), 3 deletions(-)
  create mode 100644 gcc/testsuite/gcc.dg/pointer-counted-by-4.c
  create mode 100644 gcc/testsuite/gcc.dg/pointer-counted-by-5.c
  create mode 100644 gcc/testsuite/gcc.dg/pointer-counted-by-6.c
  create mode 100644 gcc/testsuite/gcc.dg/pointer-counted-by-7.c

diff --git a/gcc/testsuite/gcc.dg/pointer-counted-by-4.c 
b/gcc/testsuite/gcc.dg/pointer-counted-by-4.c
new file mode 100644
index 00000000000..11e9421c819
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pointer-counted-by-4.c
@@ -0,0 +1,63 @@
+/* Test the attribute counted_by for pointer field and its usage in
+ * __builtin_dynamic_object_size.  */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+#include "builtin-object-size-common.h"
+
+struct pointer_array {
+  int b;
+  int *c;
+} *p_array;
+
+struct annotated {
+  int *c __attribute__ ((counted_by (b)));
+  int b;
+} *p_array_annotated;
+
+struct nested_annotated {
+  int *c __attribute__ ((counted_by (b)));
+  struct {
+    union {
+      int b;
+      float f; 
+    };
+    int n;
+  };
+} *p_array_nested_annotated;
+
+void __attribute__((__noinline__)) setup (int normal_count, int attr_count)
+{
+  p_array
+    = (struct pointer_array *) malloc (sizeof (struct pointer_array));
+  p_array->c = (int *) malloc (sizeof (int) * normal_count);
+  p_array->b = normal_count;
+
+  p_array_annotated
+    = (struct annotated *) malloc (sizeof (struct annotated));
+  p_array_annotated->c = (int *) malloc (sizeof (int) * attr_count);
+  p_array_annotated->b = attr_count;
+
+  p_array_nested_annotated
+    = (struct nested_annotated *) malloc (sizeof (struct nested_annotated));
+  p_array_nested_annotated->c = (int *) malloc (sizeof (int) * attr_count);
+  p_array_nested_annotated->b = attr_count;
+
+  return;
+}
+
+void __attribute__((__noinline__)) test ()
+{
+    EXPECT(__builtin_dynamic_object_size(p_array->c, 1), -1);
+    EXPECT(__builtin_dynamic_object_size(p_array_annotated->c, 1),
+          p_array_annotated->b * sizeof (int));
+    EXPECT(__builtin_dynamic_object_size(p_array_nested_annotated->c, 1),
+          p_array_nested_annotated->b * sizeof (int));
+}

Suggest extending this test for at least char so that you verify scaling. e.g. you could have this in pointer-counted-by-4.c:

```
#ifndef PTR_TYPE
#define PTR_TYPE int
#endif

struct pointer_array {
  int b;
  PTR_TYPE *c;
} *p_array;

struct annotated {
  PTR_TYPE *c __attribute__ ((counted_by (b)));
  int b;
} *p_array_annotated;

struct nested_annotated {
  PTR_TYPE *c __attribute__ ((counted_by (b)));
  struct {
    union {
      int b;
      float f;  
    };
    int n;
  };
} *p_array_nested_annotated;
```

and then have another test pointer-counted-by-4-char.c that simply includes this test:

```
#define PTR_TYPE char
#include "pointer-counted-by-4.c"
```

+
+int main(int argc, char *argv[])
+{
+  setup (10,10);
+  test ();
+  DONE ();
+}
diff --git a/gcc/testsuite/gcc.dg/pointer-counted-by-5.c 
b/gcc/testsuite/gcc.dg/pointer-counted-by-5.c
new file mode 100644
index 00000000000..45918c3e654
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pointer-counted-by-5.c
@@ -0,0 +1,48 @@
+/* Test the attribute counted_by for pointer fields and its usage in
+ * __builtin_dynamic_object_size: when the counted_by field is negative.  */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+#include "builtin-object-size-common.h"
+
+struct annotated {
+  int b;
+  int *c __attribute__ ((counted_by (b)));
+} *array_annotated;
+
+struct nested_annotated {
+  int *c __attribute__ ((counted_by (b)));
+  struct {
+    union {
+      int b;
+      float f; 
+    };
+    int n;
+  };
+} *array_nested_annotated;
+
+void __attribute__((__noinline__)) setup (int attr_count)
+{
+  array_annotated
+    = (struct annotated *)malloc (sizeof (struct annotated));
+  array_annotated->b = attr_count;
+
+  array_nested_annotated
+    = (struct nested_annotated *)malloc (sizeof (struct nested_annotated));
+  array_nested_annotated->b = attr_count - 1;
+
+  return;
+}
+
+void __attribute__((__noinline__)) test ()
+{
+    EXPECT(__builtin_dynamic_object_size(array_annotated->c, 1), 0);
+    EXPECT(__builtin_dynamic_object_size(array_nested_annotated->c, 1), 0);
+}
+
+int main(int argc, char *argv[])
+{
+  setup (-10);
+  test ();
+  DONE ();
+}

OK.

diff --git a/gcc/testsuite/gcc.dg/pointer-counted-by-6.c 
b/gcc/testsuite/gcc.dg/pointer-counted-by-6.c
new file mode 100644
index 00000000000..4a1baa41d9a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pointer-counted-by-6.c
@@ -0,0 +1,47 @@
+/* Test the attribute counted_by for pointer fields and its usage in
+ * __builtin_dynamic_object_size: when the type of the pointer
+ * is casting to another type.  */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+#include "builtin-object-size-common.h"
+
+typedef unsigned short u16;
+
+struct info {
+       u16 data_len;
+       char *data __attribute__((counted_by(data_len)));
+};
+
+struct foo {
+       int a;
+       int b;
+};
+
+static __attribute__((__noinline__))
+struct info *setup ()
+{
+ struct info *p;
+ size_t bytes = 3 * sizeof(struct foo);
+
+ p = (struct info *) malloc (sizeof (struct info));
+ p->data = (char *) malloc (bytes);
+ p->data_len = bytes;
+
+ return p;
+}
+
+static void
+__attribute__((__noinline__)) report (struct info *p)
+{
+ struct foo *bar = (struct foo *)p->data;
+ EXPECT(__builtin_dynamic_object_size((char *)(bar + 1), 1), 16);
+ EXPECT(__builtin_dynamic_object_size((char *)(bar + 2), 1), 8);

How about this instead; it eliminates the magic constants and also preserves the intent of the test in a more general (i.e. integer size agnostic) sense:

  EXPECT(__builtin_dynamic_object_size((char *)(bar + 1), 1),
         sizeof (struct foo) * 2);
  EXPECT(__builtin_dynamic_object_size((char *)(bar + 2), 1),
         sizeof (struct foo));

+}
+
+int main(int argc, char *argv[])
+{
+ struct info *p = setup();
+ report(p);
+ return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/pointer-counted-by-7.c 
b/gcc/testsuite/gcc.dg/pointer-counted-by-7.c
new file mode 100644
index 00000000000..01addbb857d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pointer-counted-by-7.c
@@ -0,0 +1,30 @@
+/* Additional test of the attribute counted_by for pointer field and its usage
+   in __builtin_dynamic_object_size.  */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+#include "builtin-object-size-common.h"
+
+struct annotated {
+  int b;
+  int *c __attribute__ ((counted_by (b)));
+};
+
+struct annotated __attribute__((__noinline__)) setup (int attr_count)
+{
+  struct annotated p_array_annotated;
+  p_array_annotated.c = (int *) malloc (sizeof (int) * attr_count);
+  p_array_annotated.b = attr_count;
+
+  return p_array_annotated;
+}
+
+int main(int argc, char *argv[])
+{
+  struct annotated x = setup (10);
+  int *p = x.c;
+  x = setup (20);
+  EXPECT(__builtin_dynamic_object_size (p, 1), 10 * sizeof (int));
+  EXPECT(__builtin_dynamic_object_size (x.c, 1), 20 * sizeof (int));
+  DONE ();
+}

OK. One note: all of the tests leak memory, but it's not an unbounded leak, so it's probably OK to ignore.

diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
index f348673ae75..170f1197c8d 100644
--- a/gcc/tree-object-size.cc
+++ b/gcc/tree-object-size.cc
@@ -773,10 +773,12 @@ addr_object_size (struct object_size_info *osi, 
const_tree ptr,
     4th argument TYPE_OF_SIZE: A constant 0 with its TYPE being the same as 
the TYPE
      of the object referenced by REF_TO_SIZE
     6th argument: A constant 0 with the pointer TYPE to the original flexible
-     array type.
+     array type or pointer field type.
The size of the element can be retrived from the TYPE of the 6th argument
-   of the call, which is the pointer to the array type.  */
+   of the call, which is the pointer to the original flexible array type or
+   the type of the original pointer field.  */
+
  static tree
  access_with_size_object_size (const gcall *call, int object_size_type)
  {
@@ -786,7 +788,7 @@ access_with_size_object_size (const gcall *call, int 
object_size_type)
gcc_assert (gimple_call_internal_p (call, IFN_ACCESS_WITH_SIZE));
    /* The type of the 6th argument type is the pointer TYPE to the original
-     flexible array type.  */
+     flexible array type or to the original pointer type.  */
    tree pointer_to_array_type = TREE_TYPE (gimple_call_arg (call, 5));
    gcc_assert (POINTER_TYPE_P (pointer_to_array_type));
    tree element_type = TREE_TYPE (TREE_TYPE (pointer_to_array_type));

Only a comment update to indicate the scope change for .ACCESS_WITH_SIZE. OK.

@@ -1854,6 +1856,10 @@ collect_object_sizes_for (struct object_size_info *osi, 
tree var)
              if (TREE_CODE (rhs) == SSA_NAME
                  && POINTER_TYPE_P (TREE_TYPE (rhs)))
              reexamine = merge_object_sizes (osi, var, rhs);
+           else if (TREE_CODE (rhs) == MEM_REF
+                    && POINTER_TYPE_P (TREE_TYPE (rhs))
+                    && TREE_CODE (TREE_OPERAND (rhs, 0)) == SSA_NAME)
+             reexamine = merge_object_sizes (osi, var, TREE_OPERAND (rhs, 0));
              else
                expr_object_size (osi, var, rhs);
            }

Interesting, looks like this would improve coverage for more than just this specific case of pointers within structs. Nice! Is this what pointer-counted-by-7.c covers?

Thanks,
Sid

Reply via email to