mbox series

[V4,0/2] Handle component_ref to a structure/union field including FAM for builtin_object_size

Message ID 20230224183505.4112295-1-qing.zhao@oracle.com
Headers
Series Handle component_ref to a structure/union field including FAM for builtin_object_size |

Message

Qing Zhao Feb. 24, 2023, 6:35 p.m. UTC 
  Hi, Joseph and Richard,

Could you please review this patch and let me know whether it’s ready
 for committing into GCC13?

The fix to Bug PR101832 is an important patch for kernel security
purpose. it's better to be put into GCC13.

=============================

These are the 4th version of the patches for PR101832, to fix
builtin_object_size to correctly handle component_ref to a
structure/union field that includes a flexible array member.

also includes a documentation update for the GCC extension on embedding
a structure/union with flexible array member into another structure.
which includes a fix to PR77650.

compared to the 3rd version of the patch, the major changes are:

1. update the documentation part per Joseph's comments.

compared to the 2nd version of the patch, the major changes are:

1. only include C99 flexible array member to this extension, trailing [0], [1]
  and [4] are all excluded.
2. for the new bit type_include_flexarray in tree_type_common, print it
  and also stream in/out it. 
3. update testing cases.
4. more clarification on the documentation. warnings for deprecating the 
  case when the structure with C99 FAM is embedded in the middle of
  another structure. 
5. add a new warning option -Wgnu-variable-sized-type-not-at-end for
  identifing all such cases.

bootstrapped and regression tested on aarch64 and x86.

Okay for commit?

thanks.

Qing

Qing Zhao (2):
  Handle component_ref to a structre/union field including C99 FAM
    [PR101832]
  Update documentation to clarify a GCC extension

 gcc/c-family/c.opt                            |   5 +
 gcc/c/c-decl.cc                               |  19 +++
 gcc/cp/module.cc                              |   2 +
 gcc/doc/extend.texi                           |  48 ++++++-
 gcc/print-tree.cc                             |   5 +
 .../gcc.dg/builtin-object-size-pr101832.c     | 134 ++++++++++++++++++
 .../gcc.dg/variable-sized-type-flex-array.c   |  31 ++++
 gcc/tree-core.h                               |   4 +-
 gcc/tree-object-size.cc                       |  79 +++++++----
 gcc/tree-streamer-in.cc                       |   1 +
 gcc/tree-streamer-out.cc                      |   1 +
 gcc/tree.h                                    |   6 +
 12 files changed, 305 insertions(+), 30 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
 create mode 100644 gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c

Comments

Qing Zhao March 3, 2023, 12:02 a.m. UTC | #1 
Ping

Qing

> On Feb 24, 2023, at 1:35 PM, Qing Zhao <qing.zhao@oracle.com> wrote:
> 
> Hi, Joseph and Richard,
> 
> Could you please review this patch and let me know whether it’s ready
> for committing into GCC13?
> 
> The fix to Bug PR101832 is an important patch for kernel security
> purpose. it's better to be put into GCC13.
> 
> =============================
> 
> These are the 4th version of the patches for PR101832, to fix
> builtin_object_size to correctly handle component_ref to a
> structure/union field that includes a flexible array member.
> 
> also includes a documentation update for the GCC extension on embedding
> a structure/union with flexible array member into another structure.
> which includes a fix to PR77650.
> 
> compared to the 3rd version of the patch, the major changes are:
> 
> 1. update the documentation part per Joseph's comments.
> 
> compared to the 2nd version of the patch, the major changes are:
> 
> 1. only include C99 flexible array member to this extension, trailing [0], [1]
>  and [4] are all excluded.
> 2. for the new bit type_include_flexarray in tree_type_common, print it
>  and also stream in/out it. 
> 3. update testing cases.
> 4. more clarification on the documentation. warnings for deprecating the 
>  case when the structure with C99 FAM is embedded in the middle of
>  another structure. 
> 5. add a new warning option -Wgnu-variable-sized-type-not-at-end for
>  identifing all such cases.
> 
> bootstrapped and regression tested on aarch64 and x86.
> 
> Okay for commit?
> 
> thanks.
> 
> Qing
> 
> Qing Zhao (2):
>  Handle component_ref to a structre/union field including C99 FAM
>    [PR101832]
>  Update documentation to clarify a GCC extension
> 
> gcc/c-family/c.opt                            |   5 +
> gcc/c/c-decl.cc                               |  19 +++
> gcc/cp/module.cc                              |   2 +
> gcc/doc/extend.texi                           |  48 ++++++-
> gcc/print-tree.cc                             |   5 +
> .../gcc.dg/builtin-object-size-pr101832.c     | 134 ++++++++++++++++++
> .../gcc.dg/variable-sized-type-flex-array.c   |  31 ++++
> gcc/tree-core.h                               |   4 +-
> gcc/tree-object-size.cc                       |  79 +++++++----
> gcc/tree-streamer-in.cc                       |   1 +
> gcc/tree-streamer-out.cc                      |   1 +
> gcc/tree.h                                    |   6 +
> 12 files changed, 305 insertions(+), 30 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
> create mode 100644 gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c
> 
> -- 
> 2.31.1
>
Kees Cook March 8, 2023, 6:45 p.m. UTC | #2 
On Fri, Feb 24, 2023 at 06:35:03PM +0000, Qing Zhao wrote:
> Hi, Joseph and Richard,
> 
> Could you please review this patch and let me know whether it’s ready
>  for committing into GCC13?
> 
> The fix to Bug PR101832 is an important patch for kernel security
> purpose. it's better to be put into GCC13.

Hi!

This series tests well for me. Getting this landed would be very nice
for the Linux kernel. :)

Are there any additional review notes for it, or is it ready to land?

Thanks!

-Kees