[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)

Balazs Benics via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 27 05:48:54 PDT 2024


https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/86411

>From b6ca6f0ef83d03e299d6ee9a8ed9b8044477914e Mon Sep 17 00:00:00 2001
From: komalverma04 <114138604+komalverma04 at users.noreply.github.com>
Date: Sat, 23 Mar 2024 11:14:44 -0700
Subject: [PATCH 1/8] Update checkers.rst

Modification of documentation to demonstrate utilization of AllowedPad within PaddingChecker, along with its use and effects on code analysis.
---
 clang/docs/analyzer/checkers.rst | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index fe211514914272..64b09bc6ecd1d8 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -804,10 +804,28 @@ Check for performance anti-patterns when using Grand Central Dispatch.
 
 .. _optin-performance-Padding:
 
-optin.performance.Padding
-"""""""""""""""""""""""""
+optin.performance.Padding (PaddingChecker)
+"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
 Check for excessively padded structs.
 
+.. code-block:: objc
+
+ struct TestStruct {
+      int data1;   // 4 bytes
+      char data2;  // 1 byte
+      char padding[27];  // 27 bytes of padding
+    };  // Total size: 32 bytes 
+  
+  void TestFunction() {
+      struct TestStruct struct1;  // Warning is generated due to excessive padding.
+    }
+
+   // Reports are only generated if the excessive padding exceeds 'AllowedPad' in bytes.
+
+  // AllowedPad: Default Value: 24 bytes
+
+   Usage: `AllowedPad=<value>`
+
 .. _optin-portability-UnixAPI:
 
 optin.portability.UnixAPI

>From 403115cd960653a3afe0491d2855d35d377d9312 Mon Sep 17 00:00:00 2001
From: komalverma04 <114138604+komalverma04 at users.noreply.github.com>
Date: Sat, 23 Mar 2024 11:20:46 -0700
Subject: [PATCH 2/8] Update Checkers.td

Changed NotDocumented to HasDocumentation for Padding Checker under performance checker.
---
 clang/include/clang/StaticAnalyzer/Checkers/Checkers.td | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 686e5e99f4a62c..c0e4e9a70c2ef3 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -908,7 +908,7 @@ def PaddingChecker : Checker<"Padding">,
                   "24",
                   Released>
   ]>,
-  Documentation<NotDocumented>;
+  Documentation<HasDocumentation>;
 
 } // end: "padding"
 

>From cdef12fa6a6b6c4833bc2017dae9557ee7115c92 Mon Sep 17 00:00:00 2001
From: komalverma04 <komal148btit21 at igdtuw.ac.in>
Date: Sun, 24 Mar 2024 20:39:56 +0530
Subject: [PATCH 3/8] Update checkers.rst

Made Indentation consistent and added description on limitation of checker.
---
 clang/docs/analyzer/checkers.rst | 44 ++++++++++++++++++++------------
 1 file changed, 27 insertions(+), 17 deletions(-)

diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 64b09bc6ecd1d8..85f3f33d68bc5d 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -804,27 +804,37 @@ Check for performance anti-patterns when using Grand Central Dispatch.
 
 .. _optin-performance-Padding:
 
-optin.performance.Padding (PaddingChecker)
-"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
+optin.performance.Padding (C, C++, objC)
+""""""""""""""""""""""""""""""""""""""""
 Check for excessively padded structs.
 
-.. code-block:: objc
-
- struct TestStruct {
-      int data1;   // 4 bytes
-      char data2;  // 1 byte
-      char padding[27];  // 27 bytes of padding
-    };  // Total size: 32 bytes 
-  
-  void TestFunction() {
-      struct TestStruct struct1;  // Warning is generated due to excessive padding.
-    }
-
-   // Reports are only generated if the excessive padding exceeds 'AllowedPad' in bytes.
+This checker detects structs with excessive padding, which can lead to wasted memory and decreased performance. Padding bytes are added by compilers to align data within the struct for performance optimization or memory alignment purposes. However, excessive padding can significantly increase the size of the struct without adding useful data, leading to inefficient memory usage, cache misses, and decreased performance.
+
+.. code-block:: C
+
+   #include <stdio.h>
+   // #pragma pack(1) // Uncomment it to disable structure padding
+   struct TestStruct {
+       char data1;  // 1 byte
+       char data2;  // 1 byte
+       int data3;   // 4 bytes
+   };             // Total size: 6 bytes
+   
+   int main () {
+       struct TestStruct struct1;
+       print("Structure size: %d",sizeof(struct1)); // Structure size: 8
+       return 0;
+   }
+   
+Total memory used is 8 bytes due to structure padding. In this case, padding is of 2 bytes. Padding is done to decrease the number of CPU cycles needed to access data members of the structure; it increases the performance of the processor but at the penalty of memory.
+Padding can be disabled by using the pragma directive.
+Padding can also be decreased by putting data members of the structure in descending order of their size.
 
-  // AllowedPad: Default Value: 24 bytes
+Reports are only generated if the excessive padding exceeds 'AllowedPad' in bytes. AllowedPad is the threshold value of padding.
 
-   Usage: `AllowedPad=<value>`
+AllowedPad Option:
+- Default Value: 24 bytes
+- Usage: ``AllowedPad=<value>``
 
 .. _optin-portability-UnixAPI:
 

>From 54bc64616cc2ca5bb1d0bac32767dad0290743c0 Mon Sep 17 00:00:00 2001
From: komalverma04 <komal148btit21 at igdtuw.ac.in>
Date: Sun, 24 Mar 2024 21:12:31 +0530
Subject: [PATCH 4/8] Update checkers.rst

Fixed some typos
---
 clang/docs/analyzer/checkers.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 85f3f33d68bc5d..ef8a1ed6fb889e 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -810,7 +810,7 @@ Check for excessively padded structs.
 
 This checker detects structs with excessive padding, which can lead to wasted memory and decreased performance. Padding bytes are added by compilers to align data within the struct for performance optimization or memory alignment purposes. However, excessive padding can significantly increase the size of the struct without adding useful data, leading to inefficient memory usage, cache misses, and decreased performance.
 
-.. code-block:: C
+.. code-block:: c
 
    #include <stdio.h>
    // #pragma pack(1) // Uncomment it to disable structure padding
@@ -822,7 +822,7 @@ This checker detects structs with excessive padding, which can lead to wasted me
    
    int main () {
        struct TestStruct struct1;
-       print("Structure size: %d",sizeof(struct1)); // Structure size: 8
+       printf("Structure size: %d",sizeof(struct1)); // Structure size: 8
        return 0;
    }
    

>From fb65412a90c2fb36330cb79e9605e3220ad02d76 Mon Sep 17 00:00:00 2001
From: komalverma04 <komal148btit21 at igdtuw.ac.in>
Date: Sun, 24 Mar 2024 22:22:01 +0530
Subject: [PATCH 5/8] Update checkers.rst

Fixed a typo in objC and changed it to ObjC
---
 clang/docs/analyzer/checkers.rst | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index ef8a1ed6fb889e..c2215f8eb6581a 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -804,7 +804,7 @@ Check for performance anti-patterns when using Grand Central Dispatch.
 
 .. _optin-performance-Padding:
 
-optin.performance.Padding (C, C++, objC)
+optin.performance.Padding (C, C++, ObjC)
 """"""""""""""""""""""""""""""""""""""""
 Check for excessively padded structs.
 
@@ -826,11 +826,11 @@ This checker detects structs with excessive padding, which can lead to wasted me
        return 0;
    }
    
-Total memory used is 8 bytes due to structure padding. In this case, padding is of 2 bytes. Padding is done to decrease the number of CPU cycles needed to access data members of the structure; it increases the performance of the processor but at the penalty of memory.
+Total memory used is 8 bytes due to structure padding. In this case, padding is of 2 bytes. Padding is done to decrease the number of CPU cycles needed to access data members of the structure, it increases the performance of the processor but at the penalty of memory.
 Padding can be disabled by using the pragma directive.
 Padding can also be decreased by putting data members of the structure in descending order of their size.
 
-Reports are only generated if the excessive padding exceeds 'AllowedPad' in bytes. AllowedPad is the threshold value of padding.
+Reports are only generated if the excessive padding exceeds ``AllowedPad`` in bytes. AllowedPad is the threshold value of padding.
 
 AllowedPad Option:
 - Default Value: 24 bytes

>From 4e0dcbb59534e759d469be4837726e6bf147977d Mon Sep 17 00:00:00 2001
From: komalverma04 <komal148btit21 at igdtuw.ac.in>
Date: Wed, 27 Mar 2024 00:41:20 +0530
Subject: [PATCH 6/8] Add description for ``AllowedPad``, fix space
 inconsistency and modify the example code

Fixing of trailing spaces. Described the option to make more clarity and modify the example code to correctly demonstrate the issue and added improved code also for more clarity.
---
 clang/docs/analyzer/checkers.rst | 50 ++++++++++++++++++--------------
 1 file changed, 29 insertions(+), 21 deletions(-)

diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index c2215f8eb6581a..d51a110159295d 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -809,32 +809,40 @@ optin.performance.Padding (C, C++, ObjC)
 Check for excessively padded structs.
 
 This checker detects structs with excessive padding, which can lead to wasted memory and decreased performance. Padding bytes are added by compilers to align data within the struct for performance optimization or memory alignment purposes. However, excessive padding can significantly increase the size of the struct without adding useful data, leading to inefficient memory usage, cache misses, and decreased performance.
+``AllowedPad`` is the threshold value of padding by which if padding(Internal Padding + Trailing Padding) within the struct exceeds the ``AllowedPad`` value, clang static analyzer will give a warning. The default value of ``AllowedPad`` is 24 bytes. To set this option from the command-line, pass the ``-analyze -analyzerchecker=optin.performance -analyzer-config optin.performance.Padding:AllowedPad=2`` option.
 
 .. code-block:: c
 
-   #include <stdio.h>
-   // #pragma pack(1) // Uncomment it to disable structure padding
-   struct TestStruct {
-       char data1;  // 1 byte
-       char data2;  // 1 byte
-       int data3;   // 4 bytes
-   };             // Total size: 6 bytes
-   
-   int main () {
-       struct TestStruct struct1;
-       printf("Structure size: %d",sizeof(struct1)); // Structure size: 8
-       return 0;
-   }
-   
-Total memory used is 8 bytes due to structure padding. In this case, padding is of 2 bytes. Padding is done to decrease the number of CPU cycles needed to access data members of the structure, it increases the performance of the processor but at the penalty of memory.
-Padding can be disabled by using the pragma directive.
-Padding can also be decreased by putting data members of the structure in descending order of their size.
+ #include <stdio.h>
+ // #pragma pack(1) // Uncomment it to disable structure padding
+ struct TestStruct {
+   char data1; // 1 byte
+   int data2; // 4 bytes
+   char data3; // 1 byte
+ }; // We expect it's size to be 6 bytes
+ int main () {
+   struct TestStruct struct1;
+   printf("Structure size: %d", sizeof(struct1)); // Structure size: 12
+   return 0;
+ }
+Warning will be given as padding is greater than ``AllowedPad``(value is 2).
+Total memory used is 12 bytes due to structure padding. In this case, padding is of 6 bytes. Padding is done to decrease the number of CPU cycles needed to access data members of the structure, improving the performance of the processor but at the penalty of memory usage.
+Padding can be disabled by using the pragma directive or decreased by ordering structure fields by decreasing size to achieve the best layout. An optimal struct of the above example looks like this:
 
-Reports are only generated if the excessive padding exceeds ``AllowedPad`` in bytes. AllowedPad is the threshold value of padding.
+.. code-block:: c
 
-AllowedPad Option:
-- Default Value: 24 bytes
-- Usage: ``AllowedPad=<value>``
+ #include <stdio.h>
+ struct TestStruct {
+   int data1; // 4 bytes
+   char data2; // 1 byte
+   char data3; // 1 byte
+ }; // We expect it's size to be 6 bytes
+ int main () {
+   struct TestStruct struct1;
+   printf("Structure size: %d", sizeof(struct1)); // Structure size: 8
+   return 0;
+ }
+After reordering total memory used is 8 bytes now, with trailing padding of 2 bytes. No warning will be generated.
 
 .. _optin-portability-UnixAPI:
 

>From 8c909ae9c6c88ba3bbec303896151327df02fc21 Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs at gmail.com>
Date: Wed, 27 Mar 2024 11:10:51 +0100
Subject: [PATCH 7/8] Rework docs

---
 clang/docs/analyzer/checkers.rst | 104 ++++++++++++++++++++++---------
 1 file changed, 73 insertions(+), 31 deletions(-)

diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index d51a110159295d..79f74e69e389a8 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -808,41 +808,83 @@ optin.performance.Padding (C, C++, ObjC)
 """"""""""""""""""""""""""""""""""""""""
 Check for excessively padded structs.
 
-This checker detects structs with excessive padding, which can lead to wasted memory and decreased performance. Padding bytes are added by compilers to align data within the struct for performance optimization or memory alignment purposes. However, excessive padding can significantly increase the size of the struct without adding useful data, leading to inefficient memory usage, cache misses, and decreased performance.
-``AllowedPad`` is the threshold value of padding by which if padding(Internal Padding + Trailing Padding) within the struct exceeds the ``AllowedPad`` value, clang static analyzer will give a warning. The default value of ``AllowedPad`` is 24 bytes. To set this option from the command-line, pass the ``-analyze -analyzerchecker=optin.performance -analyzer-config optin.performance.Padding:AllowedPad=2`` option.
+This checker detects structs with excessive padding, which can lead to wasted
+memory thus decreased performance by reducing the effectiveness of the
+processor cache. Padding bytes are added by compilers to align data accesses
+as some processors require data to be aligned to certain boundaries. On others,
+unaligned data access are possible, but impose significantly larger latencies.
 
-.. code-block:: c
+To avoid padding bytes, the fields of a struct should be ordered by decreasing
+by alignment. Usually, its easier to think of the ``sizeof`` of the fields, and
+ordering the fields by ``sizeof`` would usually also lead to the same optimal
+layout.
 
- #include <stdio.h>
- // #pragma pack(1) // Uncomment it to disable structure padding
- struct TestStruct {
-   char data1; // 1 byte
-   int data2; // 4 bytes
-   char data3; // 1 byte
- }; // We expect it's size to be 6 bytes
- int main () {
-   struct TestStruct struct1;
-   printf("Structure size: %d", sizeof(struct1)); // Structure size: 12
-   return 0;
- }
-Warning will be given as padding is greater than ``AllowedPad``(value is 2).
-Total memory used is 12 bytes due to structure padding. In this case, padding is of 6 bytes. Padding is done to decrease the number of CPU cycles needed to access data members of the structure, improving the performance of the processor but at the penalty of memory usage.
-Padding can be disabled by using the pragma directive or decreased by ordering structure fields by decreasing size to achieve the best layout. An optimal struct of the above example looks like this:
+In rare cases, one can use the ``#pragma pack(1)`` directive to enforce a packed
+layout too, but it is discouraged and the reordering of fields should be
+preferred.
 
-.. code-block:: c
+.. code-block:: cpp
+
+ // warn: Excessive padding in 'struct NonOptimal' (35 padding bytes, where 3 is optimal)
+ struct NonOptimal {
+   char c1;
+   // 7 bytes of padding
+   std::int64_t big1; // 8 bytes
+   char c2;
+   // 7 bytes of padding
+   std::int64_t big2; // 8 bytes
+   char c3;
+   // 7 bytes of padding
+   std::int64_t big3; // 8 bytes
+   char c4;
+   // 7 bytes of padding
+   std::int64_t big4; // 8 bytes
+   char c5;
+   // 7 bytes of padding
+ };
+ static_assert(sizeof(NonOptimal) == 4*8+5+5*7);
+
+ // no-warning: The fields are nicely aligned to have the minimal amount of padding bytes.
+ struct Optimal {
+   std::int64_t big1; // 8 bytes
+   std::int64_t big2; // 8 bytes
+   std::int64_t big3; // 8 bytes
+   std::int64_t big4; // 8 bytes
+   char c1;
+   char c2;
+   char c3;
+   char c4;
+   char c5;
+   // 3 bytes of padding
+ };
+ static_assert(sizeof(Optimal) == 4*8+5+3);
+
+ // no-warning: Enforcing bitpacked representation.
+ // Access times will have signifficant overhead. Prefer reordering the fields instead.
+ #pragma pack(1)
+ struct BitPacked {
+   char c1;
+   std::int64_t big1; // 8 bytes
+   char c2;
+   std::int64_t big2; // 8 bytes
+   char c3;
+   std::int64_t big3; // 8 bytes
+   char c4;
+   std::int64_t big4; // 8 bytes
+   char c5;
+ };
+ static_assert(sizeof(BitPacked) == 4*8+5);
+
+The ``AllowedPad`` option can be used to specify a threshold for the number
+padding bytes raising the warning. If the number of padding bytes of the struct
+and the optimal number of padding bytes differ by more than the threshold value,
+a warning will be raised.
+
+By default, the ``AllowedPad`` threshold is 24 bytes.
+
+To override this threshold to e.g. 4 bytes, use the
+``-analyzer-config optin.performance.Padding:AllowedPad=4`` option.
 
- #include <stdio.h>
- struct TestStruct {
-   int data1; // 4 bytes
-   char data2; // 1 byte
-   char data3; // 1 byte
- }; // We expect it's size to be 6 bytes
- int main () {
-   struct TestStruct struct1;
-   printf("Structure size: %d", sizeof(struct1)); // Structure size: 8
-   return 0;
- }
-After reordering total memory used is 8 bytes now, with trailing padding of 2 bytes. No warning will be generated.
 
 .. _optin-portability-UnixAPI:
 

>From dde97b361ee6e566eb9b7c3e923bd072f45b4bdf Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs at gmail.com>
Date: Wed, 27 Mar 2024 13:48:45 +0100
Subject: [PATCH 8/8] Update clang/docs/analyzer/checkers.rst

Co-authored-by: NagyDonat <donat.nagy at ericsson.com>
---
 clang/docs/analyzer/checkers.rst | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 79f74e69e389a8..15e748b4cd1168 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -820,8 +820,9 @@ ordering the fields by ``sizeof`` would usually also lead to the same optimal
 layout.
 
 In rare cases, one can use the ``#pragma pack(1)`` directive to enforce a packed
-layout too, but it is discouraged and the reordering of fields should be
-preferred.
+layout too, but it can significantly increase the access times, so reordering the
+fields is usually a better solution.
+
 
 .. code-block:: cpp
 



More information about the cfe-commits mailing list