| Message ID | CAMe9rOqqbgpoaO3ehy+4YAjfu14-KkqxS6GVpJPrsqo3t0c+sQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add patch_area_size and patch_area_entry to crtl | expand |
On Wed, Feb 5, 2020 at 12:20 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Wed, Feb 5, 2020 at 9:00 AM Richard Sandiford > <richard.sandiford@arm.com> wrote: > > > > "H.J. Lu" <hjl.tools@gmail.com> writes: > > > Currently patchable area is at the wrong place. > > > > Agreed :-) > > > > > It is placed immediately > > > after function label and before .cfi_startproc. A backend should be able > > > to add a pseudo patchable area instruction durectly into RTL. This patch > > > adds patch_area_size and patch_area_entry to cfun so that the patchable > > > area info is available in RTL passes. > > > > It might be better to add it to crtl, since it should only be needed > > during rtl generation. > > > > > It also limits patch_area_size and patch_area_entry to 65535, which is > > > a reasonable maximum size for patchable area. > > > > > > gcc/ > > > > > > PR target/93492 > > > * function.c (expand_function_start): Set cfun->patch_area_size > > > and cfun->patch_area_entry. > > > * function.h (function): Add patch_area_size and patch_area_entry. > > > * opts.c (common_handle_option): Limit > > > function_entry_patch_area_size and function_entry_patch_area_start > > > to USHRT_MAX. Fix a typo in error message. > > > * varasm.c (assemble_start_function): Use cfun->patch_area_size > > > and cfun->patch_area_entry. > > > * doc/invoke.texi: Document the maximum value for > > > -fpatchable-function-entry. > > > > > > gcc/testsuite/ > > > > > > PR target/93492 > > > * c-c++-common/patchable_function_entry-error-1.c: New test. > > > * c-c++-common/patchable_function_entry-error-2.c: Likewise. > > > * c-c++-common/patchable_function_entry-error-3.c: Likewise. > > > --- > > > gcc/doc/invoke.texi | 1 + > > > gcc/function.c | 35 +++++++++++++++++++ > > > gcc/function.h | 6 ++++ > > > gcc/opts.c | 4 ++- > > > .../patchable_function_entry-error-1.c | 9 +++++ > > > .../patchable_function_entry-error-2.c | 9 +++++ > > > .../patchable_function_entry-error-3.c | 20 +++++++++++ > > > gcc/varasm.c | 30 ++-------------- > > > 8 files changed, 85 insertions(+), 29 deletions(-) > > > create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c > > > create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c > > > create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c > > > > > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > > > index 35b341e759f..dd4835199b0 100644 > > > --- a/gcc/doc/invoke.texi > > > +++ b/gcc/doc/invoke.texi > > > @@ -13966,6 +13966,7 @@ If @code{N=0}, no pad location is recorded. > > > The NOP instructions are inserted at---and maybe before, depending on > > > @var{M}---the function entry address, even before the prologue. > > > > > > +The maximum value of @var{N} and @var{M} is 65535. > > > @end table > > > > > > > > > diff --git a/gcc/function.c b/gcc/function.c > > > index d8008f60422..badbf538eec 100644 > > > --- a/gcc/function.c > > > +++ b/gcc/function.c > > > @@ -5202,6 +5202,41 @@ expand_function_start (tree subr) > > > /* If we are doing generic stack checking, the probe should go here. */ > > > if (flag_stack_check == GENERIC_STACK_CHECK) > > > stack_check_probe_note = emit_note (NOTE_INSN_DELETED); > > > + > > > + unsigned HOST_WIDE_INT patch_area_size = function_entry_patch_area_size; > > > + unsigned HOST_WIDE_INT patch_area_entry = function_entry_patch_area_start; > > > + > > > + tree patchable_function_entry_attr > > > + = lookup_attribute ("patchable_function_entry", > > > + DECL_ATTRIBUTES (cfun->decl)); > > > + if (patchable_function_entry_attr) > > > + { > > > + tree pp_val = TREE_VALUE (patchable_function_entry_attr); > > > + tree patchable_function_entry_value1 = TREE_VALUE (pp_val); > > > + > > > + patch_area_size = tree_to_uhwi (patchable_function_entry_value1); > > > + patch_area_entry = 0; > > > + if (TREE_CHAIN (pp_val) != NULL_TREE) > > > + { > > > + tree patchable_function_entry_value2 > > > + = TREE_VALUE (TREE_CHAIN (pp_val)); > > > + patch_area_entry = tree_to_uhwi (patchable_function_entry_value2); > > > + } > > > + if (patch_area_size > USHRT_MAX || patch_area_size > USHRT_MAX) > > > + error ("invalid values for %<patchable_function_entry%> attribute"); > > > > This should probably go in handle_patchable_function_entry_attribute > > instead. It doesn't look like we have a clear policy about whether > > errors or warnings are right for unrecognised arguments to known > > attribute names, but handle_patchable_function_entry_attribute reports > > an OPT_Wattributes warning for arguments that aren't integers. Doing the > > same for out-of-range integers would be more consistent and means that > > we wouldn't break existing code if we relaxed/changed the rules in future. > > Like this? OK for master if there is no regression? > There is a small problem. Warnings from C and C++ frond-ends are different: [hjl@gnu-skx-1 gcc]$ cat x.c void __attribute__((patchable_function_entry(65536))) foo1 (void) { /* { dg-warning "'patchable_function_entry' attribute argument '65536' is out of range" } */ } [hjl@gnu-skx-1 gcc]$ ./xgcc -B./ -S x.c x.c:4:1: warning: ‘patchable_function_entry’ attribute argument ‘65536’ is out of range (> 65535) [-Wattributes] 4 | { /* { dg-warning "'patchable_function_entry' attribute argument '65536' is out of range" } */ | ^ [hjl@gnu-skx-1 gcc]$ ./xg++ -B./ -S x.c x.c:3:11: warning: ‘patchable_function_entry’ attribute argument ‘65536’ is out of range (> 65535) [-Wattributes] 3 | foo1 (void) | ^ [hjl@gnu-skx-1 gcc]$ C warns at line 4 and C++ warns at line 3. Do I need separate tests for C and C++?
On Wed, Feb 05, 2020 at 02:24:48PM -0800, H.J. Lu wrote: > On Wed, Feb 5, 2020 at 12:20 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > On Wed, Feb 5, 2020 at 9:00 AM Richard Sandiford > > <richard.sandiford@arm.com> wrote: > > > > > > "H.J. Lu" <hjl.tools@gmail.com> writes: > > > > Currently patchable area is at the wrong place. > > > > > > Agreed :-) > > > > > > > It is placed immediately > > > > after function label and before .cfi_startproc. A backend should be able > > > > to add a pseudo patchable area instruction durectly into RTL. This patch > > > > adds patch_area_size and patch_area_entry to cfun so that the patchable > > > > area info is available in RTL passes. > > > > > > It might be better to add it to crtl, since it should only be needed > > > during rtl generation. > > > > > > > It also limits patch_area_size and patch_area_entry to 65535, which is > > > > a reasonable maximum size for patchable area. > > > > > > > > gcc/ > > > > > > > > PR target/93492 > > > > * function.c (expand_function_start): Set cfun->patch_area_size > > > > and cfun->patch_area_entry. > > > > * function.h (function): Add patch_area_size and patch_area_entry. > > > > * opts.c (common_handle_option): Limit > > > > function_entry_patch_area_size and function_entry_patch_area_start > > > > to USHRT_MAX. Fix a typo in error message. > > > > * varasm.c (assemble_start_function): Use cfun->patch_area_size > > > > and cfun->patch_area_entry. > > > > * doc/invoke.texi: Document the maximum value for > > > > -fpatchable-function-entry. > > > > > > > > gcc/testsuite/ > > > > > > > > PR target/93492 > > > > * c-c++-common/patchable_function_entry-error-1.c: New test. > > > > * c-c++-common/patchable_function_entry-error-2.c: Likewise. > > > > * c-c++-common/patchable_function_entry-error-3.c: Likewise. > > > > --- > > > > gcc/doc/invoke.texi | 1 + > > > > gcc/function.c | 35 +++++++++++++++++++ > > > > gcc/function.h | 6 ++++ > > > > gcc/opts.c | 4 ++- > > > > .../patchable_function_entry-error-1.c | 9 +++++ > > > > .../patchable_function_entry-error-2.c | 9 +++++ > > > > .../patchable_function_entry-error-3.c | 20 +++++++++++ > > > > gcc/varasm.c | 30 ++-------------- > > > > 8 files changed, 85 insertions(+), 29 deletions(-) > > > > create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c > > > > create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c > > > > create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c > > > > > > > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > > > > index 35b341e759f..dd4835199b0 100644 > > > > --- a/gcc/doc/invoke.texi > > > > +++ b/gcc/doc/invoke.texi > > > > @@ -13966,6 +13966,7 @@ If @code{N=0}, no pad location is recorded. > > > > The NOP instructions are inserted at---and maybe before, depending on > > > > @var{M}---the function entry address, even before the prologue. > > > > > > > > +The maximum value of @var{N} and @var{M} is 65535. > > > > @end table > > > > > > > > > > > > diff --git a/gcc/function.c b/gcc/function.c > > > > index d8008f60422..badbf538eec 100644 > > > > --- a/gcc/function.c > > > > +++ b/gcc/function.c > > > > @@ -5202,6 +5202,41 @@ expand_function_start (tree subr) > > > > /* If we are doing generic stack checking, the probe should go here. */ > > > > if (flag_stack_check == GENERIC_STACK_CHECK) > > > > stack_check_probe_note = emit_note (NOTE_INSN_DELETED); > > > > + > > > > + unsigned HOST_WIDE_INT patch_area_size = function_entry_patch_area_size; > > > > + unsigned HOST_WIDE_INT patch_area_entry = function_entry_patch_area_start; > > > > + > > > > + tree patchable_function_entry_attr > > > > + = lookup_attribute ("patchable_function_entry", > > > > + DECL_ATTRIBUTES (cfun->decl)); > > > > + if (patchable_function_entry_attr) > > > > + { > > > > + tree pp_val = TREE_VALUE (patchable_function_entry_attr); > > > > + tree patchable_function_entry_value1 = TREE_VALUE (pp_val); > > > > + > > > > + patch_area_size = tree_to_uhwi (patchable_function_entry_value1); > > > > + patch_area_entry = 0; > > > > + if (TREE_CHAIN (pp_val) != NULL_TREE) > > > > + { > > > > + tree patchable_function_entry_value2 > > > > + = TREE_VALUE (TREE_CHAIN (pp_val)); > > > > + patch_area_entry = tree_to_uhwi (patchable_function_entry_value2); > > > > + } > > > > + if (patch_area_size > USHRT_MAX || patch_area_size > USHRT_MAX) > > > > + error ("invalid values for %<patchable_function_entry%> attribute"); > > > > > > This should probably go in handle_patchable_function_entry_attribute > > > instead. It doesn't look like we have a clear policy about whether > > > errors or warnings are right for unrecognised arguments to known > > > attribute names, but handle_patchable_function_entry_attribute reports > > > an OPT_Wattributes warning for arguments that aren't integers. Doing the > > > same for out-of-range integers would be more consistent and means that > > > we wouldn't break existing code if we relaxed/changed the rules in future. > > > > Like this? OK for master if there is no regression? > > > > There is a small problem. Warnings from C and C++ frond-ends are different: > > [hjl@gnu-skx-1 gcc]$ cat x.c > void > __attribute__((patchable_function_entry(65536))) > foo1 (void) > { /* { dg-warning "'patchable_function_entry' attribute argument > '65536' is out of range" } */ > } > [hjl@gnu-skx-1 gcc]$ ./xgcc -B./ -S x.c > x.c:4:1: warning: ‘patchable_function_entry’ attribute argument > ‘65536’ is out of range (> 65535) [-Wattributes] > 4 | { /* { dg-warning "'patchable_function_entry' attribute > argument '65536' is out of range" } */ > | ^ > [hjl@gnu-skx-1 gcc]$ ./xg++ -B./ -S x.c > x.c:3:11: warning: ‘patchable_function_entry’ attribute argument > ‘65536’ is out of range (> 65535) [-Wattributes] > 3 | foo1 (void) > | ^ > [hjl@gnu-skx-1 gcc]$ > > C warns at line 4 and C++ warns at line 3. Do I need separate tests > for C and C++? I think better would be /* { dg-error "foo" "" { target c } } */ /* { dg-error "bar" "" { target c++ } .-1 } */ Marek
On Wed, Feb 5, 2020 at 2:37 PM Marek Polacek <polacek@redhat.com> wrote: > > On Wed, Feb 05, 2020 at 02:24:48PM -0800, H.J. Lu wrote: > > On Wed, Feb 5, 2020 at 12:20 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > On Wed, Feb 5, 2020 at 9:00 AM Richard Sandiford > > > <richard.sandiford@arm.com> wrote: > > > > > > > > "H.J. Lu" <hjl.tools@gmail.com> writes: > > > > > Currently patchable area is at the wrong place. > > > > > > > > Agreed :-) > > > > > > > > > It is placed immediately > > > > > after function label and before .cfi_startproc. A backend should be able > > > > > to add a pseudo patchable area instruction durectly into RTL. This patch > > > > > adds patch_area_size and patch_area_entry to cfun so that the patchable > > > > > area info is available in RTL passes. > > > > > > > > It might be better to add it to crtl, since it should only be needed > > > > during rtl generation. > > > > > > > > > It also limits patch_area_size and patch_area_entry to 65535, which is > > > > > a reasonable maximum size for patchable area. > > > > > > > > > > gcc/ > > > > > > > > > > PR target/93492 > > > > > * function.c (expand_function_start): Set cfun->patch_area_size > > > > > and cfun->patch_area_entry. > > > > > * function.h (function): Add patch_area_size and patch_area_entry. > > > > > * opts.c (common_handle_option): Limit > > > > > function_entry_patch_area_size and function_entry_patch_area_start > > > > > to USHRT_MAX. Fix a typo in error message. > > > > > * varasm.c (assemble_start_function): Use cfun->patch_area_size > > > > > and cfun->patch_area_entry. > > > > > * doc/invoke.texi: Document the maximum value for > > > > > -fpatchable-function-entry. > > > > > > > > > > gcc/testsuite/ > > > > > > > > > > PR target/93492 > > > > > * c-c++-common/patchable_function_entry-error-1.c: New test. > > > > > * c-c++-common/patchable_function_entry-error-2.c: Likewise. > > > > > * c-c++-common/patchable_function_entry-error-3.c: Likewise. > > > > > --- > > > > > gcc/doc/invoke.texi | 1 + > > > > > gcc/function.c | 35 +++++++++++++++++++ > > > > > gcc/function.h | 6 ++++ > > > > > gcc/opts.c | 4 ++- > > > > > .../patchable_function_entry-error-1.c | 9 +++++ > > > > > .../patchable_function_entry-error-2.c | 9 +++++ > > > > > .../patchable_function_entry-error-3.c | 20 +++++++++++ > > > > > gcc/varasm.c | 30 ++-------------- > > > > > 8 files changed, 85 insertions(+), 29 deletions(-) > > > > > create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c > > > > > create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c > > > > > create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c > > > > > > > > > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > > > > > index 35b341e759f..dd4835199b0 100644 > > > > > --- a/gcc/doc/invoke.texi > > > > > +++ b/gcc/doc/invoke.texi > > > > > @@ -13966,6 +13966,7 @@ If @code{N=0}, no pad location is recorded. > > > > > The NOP instructions are inserted at---and maybe before, depending on > > > > > @var{M}---the function entry address, even before the prologue. > > > > > > > > > > +The maximum value of @var{N} and @var{M} is 65535. > > > > > @end table > > > > > > > > > > > > > > > diff --git a/gcc/function.c b/gcc/function.c > > > > > index d8008f60422..badbf538eec 100644 > > > > > --- a/gcc/function.c > > > > > +++ b/gcc/function.c > > > > > @@ -5202,6 +5202,41 @@ expand_function_start (tree subr) > > > > > /* If we are doing generic stack checking, the probe should go here. */ > > > > > if (flag_stack_check == GENERIC_STACK_CHECK) > > > > > stack_check_probe_note = emit_note (NOTE_INSN_DELETED); > > > > > + > > > > > + unsigned HOST_WIDE_INT patch_area_size = function_entry_patch_area_size; > > > > > + unsigned HOST_WIDE_INT patch_area_entry = function_entry_patch_area_start; > > > > > + > > > > > + tree patchable_function_entry_attr > > > > > + = lookup_attribute ("patchable_function_entry", > > > > > + DECL_ATTRIBUTES (cfun->decl)); > > > > > + if (patchable_function_entry_attr) > > > > > + { > > > > > + tree pp_val = TREE_VALUE (patchable_function_entry_attr); > > > > > + tree patchable_function_entry_value1 = TREE_VALUE (pp_val); > > > > > + > > > > > + patch_area_size = tree_to_uhwi (patchable_function_entry_value1); > > > > > + patch_area_entry = 0; > > > > > + if (TREE_CHAIN (pp_val) != NULL_TREE) > > > > > + { > > > > > + tree patchable_function_entry_value2 > > > > > + = TREE_VALUE (TREE_CHAIN (pp_val)); > > > > > + patch_area_entry = tree_to_uhwi (patchable_function_entry_value2); > > > > > + } > > > > > + if (patch_area_size > USHRT_MAX || patch_area_size > USHRT_MAX) > > > > > + error ("invalid values for %<patchable_function_entry%> attribute"); > > > > > > > > This should probably go in handle_patchable_function_entry_attribute > > > > instead. It doesn't look like we have a clear policy about whether > > > > errors or warnings are right for unrecognised arguments to known > > > > attribute names, but handle_patchable_function_entry_attribute reports > > > > an OPT_Wattributes warning for arguments that aren't integers. Doing the > > > > same for out-of-range integers would be more consistent and means that > > > > we wouldn't break existing code if we relaxed/changed the rules in future. > > > > > > Like this? OK for master if there is no regression? > > > > > > > There is a small problem. Warnings from C and C++ frond-ends are different: > > > > [hjl@gnu-skx-1 gcc]$ cat x.c > > void > > __attribute__((patchable_function_entry(65536))) > > foo1 (void) > > { /* { dg-warning "'patchable_function_entry' attribute argument > > '65536' is out of range" } */ > > } > > [hjl@gnu-skx-1 gcc]$ ./xgcc -B./ -S x.c > > x.c:4:1: warning: ‘patchable_function_entry’ attribute argument > > ‘65536’ is out of range (> 65535) [-Wattributes] > > 4 | { /* { dg-warning "'patchable_function_entry' attribute > > argument '65536' is out of range" } */ > > | ^ > > [hjl@gnu-skx-1 gcc]$ ./xg++ -B./ -S x.c > > x.c:3:11: warning: ‘patchable_function_entry’ attribute argument > > ‘65536’ is out of range (> 65535) [-Wattributes] > > 3 | foo1 (void) > > | ^ > > [hjl@gnu-skx-1 gcc]$ > > > > C warns at line 4 and C++ warns at line 3. Do I need separate tests > > for C and C++? > > I think better would be > > /* { dg-error "foo" "" { target c } } */ > /* { dg-error "bar" "" { target c++ } .-1 } */ > > Marek > It worked. Thanks.
On Wed, Feb 5, 2020 at 2:51 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Wed, Feb 5, 2020 at 2:37 PM Marek Polacek <polacek@redhat.com> wrote: > > > > On Wed, Feb 05, 2020 at 02:24:48PM -0800, H.J. Lu wrote: > > > On Wed, Feb 5, 2020 at 12:20 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > > > On Wed, Feb 5, 2020 at 9:00 AM Richard Sandiford > > > > <richard.sandiford@arm.com> wrote: > > > > > > > > > > "H.J. Lu" <hjl.tools@gmail.com> writes: > > > > > > Currently patchable area is at the wrong place. > > > > > > > > > > Agreed :-) > > > > > > > > > > > It is placed immediately > > > > > > after function label and before .cfi_startproc. A backend should be able > > > > > > to add a pseudo patchable area instruction durectly into RTL. This patch > > > > > > adds patch_area_size and patch_area_entry to cfun so that the patchable > > > > > > area info is available in RTL passes. > > > > > > > > > > It might be better to add it to crtl, since it should only be needed > > > > > during rtl generation. > > > > > > > > > > > It also limits patch_area_size and patch_area_entry to 65535, which is > > > > > > a reasonable maximum size for patchable area. > > > > > > > > > > > > gcc/ > > > > > > > > > > > > PR target/93492 > > > > > > * function.c (expand_function_start): Set cfun->patch_area_size > > > > > > and cfun->patch_area_entry. > > > > > > * function.h (function): Add patch_area_size and patch_area_entry. > > > > > > * opts.c (common_handle_option): Limit > > > > > > function_entry_patch_area_size and function_entry_patch_area_start > > > > > > to USHRT_MAX. Fix a typo in error message. > > > > > > * varasm.c (assemble_start_function): Use cfun->patch_area_size > > > > > > and cfun->patch_area_entry. > > > > > > * doc/invoke.texi: Document the maximum value for > > > > > > -fpatchable-function-entry. > > > > > > > > > > > > gcc/testsuite/ > > > > > > > > > > > > PR target/93492 > > > > > > * c-c++-common/patchable_function_entry-error-1.c: New test. > > > > > > * c-c++-common/patchable_function_entry-error-2.c: Likewise. > > > > > > * c-c++-common/patchable_function_entry-error-3.c: Likewise. > > > > > > --- > > > > > > gcc/doc/invoke.texi | 1 + > > > > > > gcc/function.c | 35 +++++++++++++++++++ > > > > > > gcc/function.h | 6 ++++ > > > > > > gcc/opts.c | 4 ++- > > > > > > .../patchable_function_entry-error-1.c | 9 +++++ > > > > > > .../patchable_function_entry-error-2.c | 9 +++++ > > > > > > .../patchable_function_entry-error-3.c | 20 +++++++++++ > > > > > > gcc/varasm.c | 30 ++-------------- > > > > > > 8 files changed, 85 insertions(+), 29 deletions(-) > > > > > > create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c > > > > > > create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c > > > > > > create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c > > > > > > > > > > > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > > > > > > index 35b341e759f..dd4835199b0 100644 > > > > > > --- a/gcc/doc/invoke.texi > > > > > > +++ b/gcc/doc/invoke.texi > > > > > > @@ -13966,6 +13966,7 @@ If @code{N=0}, no pad location is recorded. > > > > > > The NOP instructions are inserted at---and maybe before, depending on > > > > > > @var{M}---the function entry address, even before the prologue. > > > > > > > > > > > > +The maximum value of @var{N} and @var{M} is 65535. > > > > > > @end table > > > > > > > > > > > > > > > > > > diff --git a/gcc/function.c b/gcc/function.c > > > > > > index d8008f60422..badbf538eec 100644 > > > > > > --- a/gcc/function.c > > > > > > +++ b/gcc/function.c > > > > > > @@ -5202,6 +5202,41 @@ expand_function_start (tree subr) > > > > > > /* If we are doing generic stack checking, the probe should go here. */ > > > > > > if (flag_stack_check == GENERIC_STACK_CHECK) > > > > > > stack_check_probe_note = emit_note (NOTE_INSN_DELETED); > > > > > > + > > > > > > + unsigned HOST_WIDE_INT patch_area_size = function_entry_patch_area_size; > > > > > > + unsigned HOST_WIDE_INT patch_area_entry = function_entry_patch_area_start; > > > > > > + > > > > > > + tree patchable_function_entry_attr > > > > > > + = lookup_attribute ("patchable_function_entry", > > > > > > + DECL_ATTRIBUTES (cfun->decl)); > > > > > > + if (patchable_function_entry_attr) > > > > > > + { > > > > > > + tree pp_val = TREE_VALUE (patchable_function_entry_attr); > > > > > > + tree patchable_function_entry_value1 = TREE_VALUE (pp_val); > > > > > > + > > > > > > + patch_area_size = tree_to_uhwi (patchable_function_entry_value1); > > > > > > + patch_area_entry = 0; > > > > > > + if (TREE_CHAIN (pp_val) != NULL_TREE) > > > > > > + { > > > > > > + tree patchable_function_entry_value2 > > > > > > + = TREE_VALUE (TREE_CHAIN (pp_val)); > > > > > > + patch_area_entry = tree_to_uhwi (patchable_function_entry_value2); > > > > > > + } > > > > > > + if (patch_area_size > USHRT_MAX || patch_area_size > USHRT_MAX) > > > > > > + error ("invalid values for %<patchable_function_entry%> attribute"); > > > > > > > > > > This should probably go in handle_patchable_function_entry_attribute > > > > > instead. It doesn't look like we have a clear policy about whether > > > > > errors or warnings are right for unrecognised arguments to known > > > > > attribute names, but handle_patchable_function_entry_attribute reports > > > > > an OPT_Wattributes warning for arguments that aren't integers. Doing the > > > > > same for out-of-range integers would be more consistent and means that > > > > > we wouldn't break existing code if we relaxed/changed the rules in future. > > > > > > > > Like this? OK for master if there is no regression? > > > > > > > > > > There is a small problem. Warnings from C and C++ frond-ends are different: > > > > > > [hjl@gnu-skx-1 gcc]$ cat x.c > > > void > > > __attribute__((patchable_function_entry(65536))) > > > foo1 (void) > > > { /* { dg-warning "'patchable_function_entry' attribute argument > > > '65536' is out of range" } */ > > > } > > > [hjl@gnu-skx-1 gcc]$ ./xgcc -B./ -S x.c > > > x.c:4:1: warning: ‘patchable_function_entry’ attribute argument > > > ‘65536’ is out of range (> 65535) [-Wattributes] > > > 4 | { /* { dg-warning "'patchable_function_entry' attribute > > > argument '65536' is out of range" } */ > > > | ^ > > > [hjl@gnu-skx-1 gcc]$ ./xg++ -B./ -S x.c > > > x.c:3:11: warning: ‘patchable_function_entry’ attribute argument > > > ‘65536’ is out of range (> 65535) [-Wattributes] > > > 3 | foo1 (void) > > > | ^ > > > [hjl@gnu-skx-1 gcc]$ > > > > > > C warns at line 4 and C++ warns at line 3. Do I need separate tests > > > for C and C++? > > > > I think better would be > > > > /* { dg-error "foo" "" { target c } } */ > > /* { dg-error "bar" "" { target c++ } .-1 } */ > > > > Marek > > > > It worked. Here is the patch with updated tests. There are no regressions on Linux/x86-64. OK for master? Thanks.
"H.J. Lu" <hjl.tools@gmail.com> writes: > On Wed, Feb 5, 2020 at 2:51 PM H.J. Lu <hjl.tools@gmail.com> wrote: >> >> On Wed, Feb 5, 2020 at 2:37 PM Marek Polacek <polacek@redhat.com> wrote: >> > >> > On Wed, Feb 05, 2020 at 02:24:48PM -0800, H.J. Lu wrote: >> > > On Wed, Feb 5, 2020 at 12:20 PM H.J. Lu <hjl.tools@gmail.com> wrote: >> > > > >> > > > On Wed, Feb 5, 2020 at 9:00 AM Richard Sandiford >> > > > <richard.sandiford@arm.com> wrote: >> > > > > >> > > > > "H.J. Lu" <hjl.tools@gmail.com> writes: >> > > > > > Currently patchable area is at the wrong place. >> > > > > >> > > > > Agreed :-) >> > > > > >> > > > > > It is placed immediately >> > > > > > after function label and before .cfi_startproc. A backend should be able >> > > > > > to add a pseudo patchable area instruction durectly into RTL. This patch >> > > > > > adds patch_area_size and patch_area_entry to cfun so that the patchable >> > > > > > area info is available in RTL passes. >> > > > > >> > > > > It might be better to add it to crtl, since it should only be needed >> > > > > during rtl generation. >> > > > > >> > > > > > It also limits patch_area_size and patch_area_entry to 65535, which is >> > > > > > a reasonable maximum size for patchable area. >> > > > > > >> > > > > > gcc/ >> > > > > > >> > > > > > PR target/93492 >> > > > > > * function.c (expand_function_start): Set cfun->patch_area_size >> > > > > > and cfun->patch_area_entry. >> > > > > > * function.h (function): Add patch_area_size and patch_area_entry. >> > > > > > * opts.c (common_handle_option): Limit >> > > > > > function_entry_patch_area_size and function_entry_patch_area_start >> > > > > > to USHRT_MAX. Fix a typo in error message. >> > > > > > * varasm.c (assemble_start_function): Use cfun->patch_area_size >> > > > > > and cfun->patch_area_entry. >> > > > > > * doc/invoke.texi: Document the maximum value for >> > > > > > -fpatchable-function-entry. >> > > > > > >> > > > > > gcc/testsuite/ >> > > > > > >> > > > > > PR target/93492 >> > > > > > * c-c++-common/patchable_function_entry-error-1.c: New test. >> > > > > > * c-c++-common/patchable_function_entry-error-2.c: Likewise. >> > > > > > * c-c++-common/patchable_function_entry-error-3.c: Likewise. >> > > > > > --- >> > > > > > gcc/doc/invoke.texi | 1 + >> > > > > > gcc/function.c | 35 +++++++++++++++++++ >> > > > > > gcc/function.h | 6 ++++ >> > > > > > gcc/opts.c | 4 ++- >> > > > > > .../patchable_function_entry-error-1.c | 9 +++++ >> > > > > > .../patchable_function_entry-error-2.c | 9 +++++ >> > > > > > .../patchable_function_entry-error-3.c | 20 +++++++++++ >> > > > > > gcc/varasm.c | 30 ++-------------- >> > > > > > 8 files changed, 85 insertions(+), 29 deletions(-) >> > > > > > create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c >> > > > > > create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c >> > > > > > create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c >> > > > > > >> > > > > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi >> > > > > > index 35b341e759f..dd4835199b0 100644 >> > > > > > --- a/gcc/doc/invoke.texi >> > > > > > +++ b/gcc/doc/invoke.texi >> > > > > > @@ -13966,6 +13966,7 @@ If @code{N=0}, no pad location is recorded. >> > > > > > The NOP instructions are inserted at---and maybe before, depending on >> > > > > > @var{M}---the function entry address, even before the prologue. >> > > > > > >> > > > > > +The maximum value of @var{N} and @var{M} is 65535. >> > > > > > @end table >> > > > > > >> > > > > > >> > > > > > diff --git a/gcc/function.c b/gcc/function.c >> > > > > > index d8008f60422..badbf538eec 100644 >> > > > > > --- a/gcc/function.c >> > > > > > +++ b/gcc/function.c >> > > > > > @@ -5202,6 +5202,41 @@ expand_function_start (tree subr) >> > > > > > /* If we are doing generic stack checking, the probe should go here. */ >> > > > > > if (flag_stack_check == GENERIC_STACK_CHECK) >> > > > > > stack_check_probe_note = emit_note (NOTE_INSN_DELETED); >> > > > > > + >> > > > > > + unsigned HOST_WIDE_INT patch_area_size = function_entry_patch_area_size; >> > > > > > + unsigned HOST_WIDE_INT patch_area_entry = function_entry_patch_area_start; >> > > > > > + >> > > > > > + tree patchable_function_entry_attr >> > > > > > + = lookup_attribute ("patchable_function_entry", >> > > > > > + DECL_ATTRIBUTES (cfun->decl)); >> > > > > > + if (patchable_function_entry_attr) >> > > > > > + { >> > > > > > + tree pp_val = TREE_VALUE (patchable_function_entry_attr); >> > > > > > + tree patchable_function_entry_value1 = TREE_VALUE (pp_val); >> > > > > > + >> > > > > > + patch_area_size = tree_to_uhwi (patchable_function_entry_value1); >> > > > > > + patch_area_entry = 0; >> > > > > > + if (TREE_CHAIN (pp_val) != NULL_TREE) >> > > > > > + { >> > > > > > + tree patchable_function_entry_value2 >> > > > > > + = TREE_VALUE (TREE_CHAIN (pp_val)); >> > > > > > + patch_area_entry = tree_to_uhwi (patchable_function_entry_value2); >> > > > > > + } >> > > > > > + if (patch_area_size > USHRT_MAX || patch_area_size > USHRT_MAX) >> > > > > > + error ("invalid values for %<patchable_function_entry%> attribute"); >> > > > > >> > > > > This should probably go in handle_patchable_function_entry_attribute >> > > > > instead. It doesn't look like we have a clear policy about whether >> > > > > errors or warnings are right for unrecognised arguments to known >> > > > > attribute names, but handle_patchable_function_entry_attribute reports >> > > > > an OPT_Wattributes warning for arguments that aren't integers. Doing the >> > > > > same for out-of-range integers would be more consistent and means that >> > > > > we wouldn't break existing code if we relaxed/changed the rules in future. >> > > > >> > > > Like this? OK for master if there is no regression? >> > > > >> > > >> > > There is a small problem. Warnings from C and C++ frond-ends are different: >> > > >> > > [hjl@gnu-skx-1 gcc]$ cat x.c >> > > void >> > > __attribute__((patchable_function_entry(65536))) >> > > foo1 (void) >> > > { /* { dg-warning "'patchable_function_entry' attribute argument >> > > '65536' is out of range" } */ >> > > } >> > > [hjl@gnu-skx-1 gcc]$ ./xgcc -B./ -S x.c >> > > x.c:4:1: warning: ‘patchable_function_entry’ attribute argument >> > > ‘65536’ is out of range (> 65535) [-Wattributes] >> > > 4 | { /* { dg-warning "'patchable_function_entry' attribute >> > > argument '65536' is out of range" } */ >> > > | ^ >> > > [hjl@gnu-skx-1 gcc]$ ./xg++ -B./ -S x.c >> > > x.c:3:11: warning: ‘patchable_function_entry’ attribute argument >> > > ‘65536’ is out of range (> 65535) [-Wattributes] >> > > 3 | foo1 (void) >> > > | ^ >> > > [hjl@gnu-skx-1 gcc]$ >> > > >> > > C warns at line 4 and C++ warns at line 3. Do I need separate tests >> > > for C and C++? >> > >> > I think better would be >> > >> > /* { dg-error "foo" "" { target c } } */ >> > /* { dg-error "bar" "" { target c++ } .-1 } */ >> > >> > Marek >> > >> >> It worked. > > Here is the patch with updated tests. There are no regressions on > Linux/x86-64. > OK for master? > > Thanks. > > > -- > H.J. > > From 8320bbb83e53f9e135fe1eca50840baacf157881 Mon Sep 17 00:00:00 2001 > From: "H.J. Lu" <hjl.tools@gmail.com> > Date: Wed, 5 Feb 2020 04:01:59 -0800 > Subject: [PATCH] Add patch_area_size and patch_area_entry to crtl > > Currently patchable area is at the wrong place. It is placed immediately > after function label and before .cfi_startproc. A backend should be able > to add a pseudo patchable area instruction durectly into RTL. This patch > adds patch_area_size and patch_area_entry to crtl so that the patchable > area info is available in RTL passes. > > It also limits patch_area_size and patch_area_entry to 65535, which is > a reasonable maximum size for patchable area. > > gcc/c-family/ > > PR target/93492 > * c-attribs.c (handle_patchable_function_entry_attribute): Limit > value to USHRT_MAX (65535). > > gcc/ > > PR target/93492 > * cfgexpand.c (pass_expand::execute): Set crtl->patch_area_size > and crtl->patch_area_entry. > * emit-rtl.h (rtl_data): Add patch_area_size and patch_area_entry. > * opts.c (common_handle_option): Limit > function_entry_patch_area_size and function_entry_patch_area_start > to USHRT_MAX. Fix a typo in error message. > * varasm.c (assemble_start_function): Use crtl->patch_area_size > and crtl->patch_area_entry. > * doc/invoke.texi: Document the maximum value for > -fpatchable-function-entry. > > gcc/testsuite/ > > PR target/93492 > * c-c++-common/patchable_function_entry-error-1.c: New test. > * c-c++-common/patchable_function_entry-error-2.c: Likewise. > * c-c++-common/patchable_function_entry-error-3.c: Likewise. LGTM, but an RM should have the final say on whether this is suitable for stage 4. Thanks, Richard
On Thu, Feb 6, 2020 at 12:01 AM Richard Sandiford <richard.sandiford@arm.com> wrote: > > "H.J. Lu" <hjl.tools@gmail.com> writes: > > On Wed, Feb 5, 2020 at 2:51 PM H.J. Lu <hjl.tools@gmail.com> wrote: > >> > >> On Wed, Feb 5, 2020 at 2:37 PM Marek Polacek <polacek@redhat.com> wrote: > >> > > >> > On Wed, Feb 05, 2020 at 02:24:48PM -0800, H.J. Lu wrote: > >> > > On Wed, Feb 5, 2020 at 12:20 PM H.J. Lu <hjl.tools@gmail.com> wrote: > >> > > > > >> > > > On Wed, Feb 5, 2020 at 9:00 AM Richard Sandiford > >> > > > <richard.sandiford@arm.com> wrote: > >> > > > > > >> > > > > "H.J. Lu" <hjl.tools@gmail.com> writes: > >> > > > > > Currently patchable area is at the wrong place. > >> > > > > > >> > > > > Agreed :-) > >> > > > > > >> > > > > > It is placed immediately > >> > > > > > after function label and before .cfi_startproc. A backend should be able > >> > > > > > to add a pseudo patchable area instruction durectly into RTL. This patch > >> > > > > > adds patch_area_size and patch_area_entry to cfun so that the patchable > >> > > > > > area info is available in RTL passes. > >> > > > > > >> > > > > It might be better to add it to crtl, since it should only be needed > >> > > > > during rtl generation. > >> > > > > > >> > > > > > It also limits patch_area_size and patch_area_entry to 65535, which is > >> > > > > > a reasonable maximum size for patchable area. > >> > > > > > > >> > > > > > gcc/ > >> > > > > > > >> > > > > > PR target/93492 > >> > > > > > * function.c (expand_function_start): Set cfun->patch_area_size > >> > > > > > and cfun->patch_area_entry. > >> > > > > > * function.h (function): Add patch_area_size and patch_area_entry. > >> > > > > > * opts.c (common_handle_option): Limit > >> > > > > > function_entry_patch_area_size and function_entry_patch_area_start > >> > > > > > to USHRT_MAX. Fix a typo in error message. > >> > > > > > * varasm.c (assemble_start_function): Use cfun->patch_area_size > >> > > > > > and cfun->patch_area_entry. > >> > > > > > * doc/invoke.texi: Document the maximum value for > >> > > > > > -fpatchable-function-entry. > >> > > > > > > >> > > > > > gcc/testsuite/ > >> > > > > > > >> > > > > > PR target/93492 > >> > > > > > * c-c++-common/patchable_function_entry-error-1.c: New test. > >> > > > > > * c-c++-common/patchable_function_entry-error-2.c: Likewise. > >> > > > > > * c-c++-common/patchable_function_entry-error-3.c: Likewise. > >> > > > > > --- > >> > > > > > gcc/doc/invoke.texi | 1 + > >> > > > > > gcc/function.c | 35 +++++++++++++++++++ > >> > > > > > gcc/function.h | 6 ++++ > >> > > > > > gcc/opts.c | 4 ++- > >> > > > > > .../patchable_function_entry-error-1.c | 9 +++++ > >> > > > > > .../patchable_function_entry-error-2.c | 9 +++++ > >> > > > > > .../patchable_function_entry-error-3.c | 20 +++++++++++ > >> > > > > > gcc/varasm.c | 30 ++-------------- > >> > > > > > 8 files changed, 85 insertions(+), 29 deletions(-) > >> > > > > > create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c > >> > > > > > create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c > >> > > > > > create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c > >> > > > > > > >> > > > > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > >> > > > > > index 35b341e759f..dd4835199b0 100644 > >> > > > > > --- a/gcc/doc/invoke.texi > >> > > > > > +++ b/gcc/doc/invoke.texi > >> > > > > > @@ -13966,6 +13966,7 @@ If @code{N=0}, no pad location is recorded. > >> > > > > > The NOP instructions are inserted at---and maybe before, depending on > >> > > > > > @var{M}---the function entry address, even before the prologue. > >> > > > > > > >> > > > > > +The maximum value of @var{N} and @var{M} is 65535. > >> > > > > > @end table > >> > > > > > > >> > > > > > > >> > > > > > diff --git a/gcc/function.c b/gcc/function.c > >> > > > > > index d8008f60422..badbf538eec 100644 > >> > > > > > --- a/gcc/function.c > >> > > > > > +++ b/gcc/function.c > >> > > > > > @@ -5202,6 +5202,41 @@ expand_function_start (tree subr) > >> > > > > > /* If we are doing generic stack checking, the probe should go here. */ > >> > > > > > if (flag_stack_check == GENERIC_STACK_CHECK) > >> > > > > > stack_check_probe_note = emit_note (NOTE_INSN_DELETED); > >> > > > > > + > >> > > > > > + unsigned HOST_WIDE_INT patch_area_size = function_entry_patch_area_size; > >> > > > > > + unsigned HOST_WIDE_INT patch_area_entry = function_entry_patch_area_start; > >> > > > > > + > >> > > > > > + tree patchable_function_entry_attr > >> > > > > > + = lookup_attribute ("patchable_function_entry", > >> > > > > > + DECL_ATTRIBUTES (cfun->decl)); > >> > > > > > + if (patchable_function_entry_attr) > >> > > > > > + { > >> > > > > > + tree pp_val = TREE_VALUE (patchable_function_entry_attr); > >> > > > > > + tree patchable_function_entry_value1 = TREE_VALUE (pp_val); > >> > > > > > + > >> > > > > > + patch_area_size = tree_to_uhwi (patchable_function_entry_value1); > >> > > > > > + patch_area_entry = 0; > >> > > > > > + if (TREE_CHAIN (pp_val) != NULL_TREE) > >> > > > > > + { > >> > > > > > + tree patchable_function_entry_value2 > >> > > > > > + = TREE_VALUE (TREE_CHAIN (pp_val)); > >> > > > > > + patch_area_entry = tree_to_uhwi (patchable_function_entry_value2); > >> > > > > > + } > >> > > > > > + if (patch_area_size > USHRT_MAX || patch_area_size > USHRT_MAX) > >> > > > > > + error ("invalid values for %<patchable_function_entry%> attribute"); > >> > > > > > >> > > > > This should probably go in handle_patchable_function_entry_attribute > >> > > > > instead. It doesn't look like we have a clear policy about whether > >> > > > > errors or warnings are right for unrecognised arguments to known > >> > > > > attribute names, but handle_patchable_function_entry_attribute reports > >> > > > > an OPT_Wattributes warning for arguments that aren't integers. Doing the > >> > > > > same for out-of-range integers would be more consistent and means that > >> > > > > we wouldn't break existing code if we relaxed/changed the rules in future. > >> > > > > >> > > > Like this? OK for master if there is no regression? > >> > > > > >> > > > >> > > There is a small problem. Warnings from C and C++ frond-ends are different: > >> > > > >> > > [hjl@gnu-skx-1 gcc]$ cat x.c > >> > > void > >> > > __attribute__((patchable_function_entry(65536))) > >> > > foo1 (void) > >> > > { /* { dg-warning "'patchable_function_entry' attribute argument > >> > > '65536' is out of range" } */ > >> > > } > >> > > [hjl@gnu-skx-1 gcc]$ ./xgcc -B./ -S x.c > >> > > x.c:4:1: warning: ‘patchable_function_entry’ attribute argument > >> > > ‘65536’ is out of range (> 65535) [-Wattributes] > >> > > 4 | { /* { dg-warning "'patchable_function_entry' attribute > >> > > argument '65536' is out of range" } */ > >> > > | ^ > >> > > [hjl@gnu-skx-1 gcc]$ ./xg++ -B./ -S x.c > >> > > x.c:3:11: warning: ‘patchable_function_entry’ attribute argument > >> > > ‘65536’ is out of range (> 65535) [-Wattributes] > >> > > 3 | foo1 (void) > >> > > | ^ > >> > > [hjl@gnu-skx-1 gcc]$ > >> > > > >> > > C warns at line 4 and C++ warns at line 3. Do I need separate tests > >> > > for C and C++? > >> > > >> > I think better would be > >> > > >> > /* { dg-error "foo" "" { target c } } */ > >> > /* { dg-error "bar" "" { target c++ } .-1 } */ > >> > > >> > Marek > >> > > >> > >> It worked. > > > > Here is the patch with updated tests. There are no regressions on > > Linux/x86-64. > > OK for master? > > > > Thanks. > > > > > > -- > > H.J. > > > > From 8320bbb83e53f9e135fe1eca50840baacf157881 Mon Sep 17 00:00:00 2001 > > From: "H.J. Lu" <hjl.tools@gmail.com> > > Date: Wed, 5 Feb 2020 04:01:59 -0800 > > Subject: [PATCH] Add patch_area_size and patch_area_entry to crtl > > > > Currently patchable area is at the wrong place. It is placed immediately > > after function label and before .cfi_startproc. A backend should be able > > to add a pseudo patchable area instruction durectly into RTL. This patch > > adds patch_area_size and patch_area_entry to crtl so that the patchable > > area info is available in RTL passes. > > > > It also limits patch_area_size and patch_area_entry to 65535, which is > > a reasonable maximum size for patchable area. > > > > gcc/c-family/ > > > > PR target/93492 > > * c-attribs.c (handle_patchable_function_entry_attribute): Limit > > value to USHRT_MAX (65535). > > > > gcc/ > > > > PR target/93492 > > * cfgexpand.c (pass_expand::execute): Set crtl->patch_area_size > > and crtl->patch_area_entry. > > * emit-rtl.h (rtl_data): Add patch_area_size and patch_area_entry. > > * opts.c (common_handle_option): Limit > > function_entry_patch_area_size and function_entry_patch_area_start > > to USHRT_MAX. Fix a typo in error message. > > * varasm.c (assemble_start_function): Use crtl->patch_area_size > > and crtl->patch_area_entry. > > * doc/invoke.texi: Document the maximum value for > > -fpatchable-function-entry. > > > > gcc/testsuite/ > > > > PR target/93492 > > * c-c++-common/patchable_function_entry-error-1.c: New test. > > * c-c++-common/patchable_function_entry-error-2.c: Likewise. > > * c-c++-common/patchable_function_entry-error-3.c: Likewise. > > LGTM, but an RM should have the final say on whether this is > suitable for stage 4. > This is the patch I am checking in. Thanks.
From 8a56c3424d4194dfc0290eaa666962c6e75f9ce8 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <hjl.tools@gmail.com> Date: Wed, 5 Feb 2020 04:01:59 -0800 Subject: [PATCH] Add patch_area_size and patch_area_entry to crtl Currently patchable area is at the wrong place. It is placed immediately after function label and before .cfi_startproc. A backend should be able to add a pseudo patchable area instruction durectly into RTL. This patch adds patch_area_size and patch_area_entry to crtl so that the patchable area info is available in RTL passes. It also limits patch_area_size and patch_area_entry to 65535, which is a reasonable maximum size for patchable area. gcc/c-family/ PR target/93492 * c-attribs.c (handle_patchable_function_entry_attribute): Limit value to USHRT_MAX (65535). gcc/ PR target/93492 * cfgexpand.c (pass_expand::execute): Set crtl->patch_area_size and crtl->patch_area_entry. * emit-rtl.h (rtl_data): Add patch_area_size and patch_area_entry. * opts.c (common_handle_option): Limit function_entry_patch_area_size and function_entry_patch_area_start to USHRT_MAX. Fix a typo in error message. * varasm.c (assemble_start_function): Use crtl->patch_area_size and crtl->patch_area_entry. * doc/invoke.texi: Document the maximum value for -fpatchable-function-entry. gcc/testsuite/ PR target/93492 * c-c++-common/patchable_function_entry-error-1.c: New test. * c-c++-common/patchable_function_entry-error-2.c: Likewise. * c-c++-common/patchable_function_entry-error-3.c: Likewise. --- gcc/c-family/c-attribs.c | 9 +++++ gcc/cfgexpand.c | 33 +++++++++++++++++++ gcc/doc/invoke.texi | 1 + gcc/emit-rtl.h | 6 ++++ gcc/opts.c | 4 ++- .../patchable_function_entry-error-1.c | 9 +++++ .../patchable_function_entry-error-2.c | 9 +++++ .../patchable_function_entry-error-3.c | 20 +++++++++++ gcc/varasm.c | 30 ++--------------- 9 files changed, 92 insertions(+), 29 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c index 7ec6fc848ac..15dbda1eff7 100644 --- a/gcc/c-family/c-attribs.c +++ b/gcc/c-family/c-attribs.c @@ -4539,6 +4539,15 @@ handle_patchable_function_entry_attribute (tree *, tree name, tree args, *no_add_attrs = true; return NULL_TREE; } + + if (tree_to_uhwi (val) > USHRT_MAX) + { + warning (OPT_Wattributes, + "%qE attribute argument %qE is out of range (> 65535)", + name, val); + *no_add_attrs = true; + return NULL_TREE; + } } return NULL_TREE; } diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c index 9864e4344d2..f063f50c263 100644 --- a/gcc/cfgexpand.c +++ b/gcc/cfgexpand.c @@ -6656,6 +6656,39 @@ pass_expand::execute (function *fun) if (crtl->tail_call_emit) fixup_tail_calls (); + unsigned HOST_WIDE_INT patch_area_size = function_entry_patch_area_size; + unsigned HOST_WIDE_INT patch_area_entry = function_entry_patch_area_start; + + tree patchable_function_entry_attr + = lookup_attribute ("patchable_function_entry", + DECL_ATTRIBUTES (cfun->decl)); + if (patchable_function_entry_attr) + { + tree pp_val = TREE_VALUE (patchable_function_entry_attr); + tree patchable_function_entry_value1 = TREE_VALUE (pp_val); + + patch_area_size = tree_to_uhwi (patchable_function_entry_value1); + patch_area_entry = 0; + if (TREE_CHAIN (pp_val) != NULL_TREE) + { + tree patchable_function_entry_value2 + = TREE_VALUE (TREE_CHAIN (pp_val)); + patch_area_entry = tree_to_uhwi (patchable_function_entry_value2); + } + } + + if (patch_area_entry > patch_area_size) + { + if (patch_area_size > 0) + warning (OPT_Wattributes, + "patchable function entry %wu exceeds size %wu", + patch_area_entry, patch_area_size); + patch_area_entry = 0; + } + + crtl->patch_area_size = patch_area_size; + crtl->patch_area_entry = patch_area_entry; + /* BB subdivision may have created basic blocks that are are only reachable from unlikely bbs but not marked as such in the profile. */ if (optimize) diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 35b341e759f..dd4835199b0 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -13966,6 +13966,7 @@ If @code{N=0}, no pad location is recorded. The NOP instructions are inserted at---and maybe before, depending on @var{M}---the function entry address, even before the prologue. +The maximum value of @var{N} and @var{M} is 65535. @end table diff --git a/gcc/emit-rtl.h b/gcc/emit-rtl.h index a878efe3cf7..3d6565c8a30 100644 --- a/gcc/emit-rtl.h +++ b/gcc/emit-rtl.h @@ -173,6 +173,12 @@ struct GTY(()) rtl_data { local stack. */ unsigned int stack_alignment_estimated; + /* How many NOP insns to place at each function entry by default. */ + unsigned short patch_area_size; + + /* How far the real asm entry point is into this area. */ + unsigned short patch_area_entry; + /* For reorg. */ /* Nonzero if function being compiled called builtin_return_addr or diff --git a/gcc/opts.c b/gcc/opts.c index 7affeb41a96..c6011f1f9b7 100644 --- a/gcc/opts.c +++ b/gcc/opts.c @@ -2598,10 +2598,12 @@ common_handle_option (struct gcc_options *opts, function_entry_patch_area_start = 0; } if (function_entry_patch_area_size < 0 + || function_entry_patch_area_size > USHRT_MAX || function_entry_patch_area_start < 0 + || function_entry_patch_area_start > USHRT_MAX || function_entry_patch_area_size < function_entry_patch_area_start) - error ("invalid arguments for %<-fpatchable_function_entry%>"); + error ("invalid arguments for %<-fpatchable-function-entry%>"); free (patch_area_arg); } break; diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c b/gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c new file mode 100644 index 00000000000..f60bf46cfe3 --- /dev/null +++ b/gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c @@ -0,0 +1,9 @@ +/* { dg-do compile { target { ! { nvptx*-*-* visium-*-* } } } } */ +/* { dg-options "-O2 -fpatchable-function-entry=65536,1" } */ +/* { dg-additional-options "-fno-pie" { target sparc*-*-* } } */ +/* { dg-error "invalid arguments for '-fpatchable-function-entry'" "" { target *-*-* } 0 } */ + +void +foo (void) +{ +} diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c b/gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c new file mode 100644 index 00000000000..90f88c78be7 --- /dev/null +++ b/gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c @@ -0,0 +1,9 @@ +/* { dg-do compile { target { ! { nvptx*-*-* visium-*-* } } } } */ +/* { dg-options "-O2 -fpatchable-function-entry=1,65536" } */ +/* { dg-additional-options "-fno-pie" { target sparc*-*-* } } */ +/* { dg-error "invalid arguments for '-fpatchable-function-entry'" "" { target *-*-* } 0 } */ + +void +foo (void) +{ +} diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c b/gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c new file mode 100644 index 00000000000..45e93988886 --- /dev/null +++ b/gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c @@ -0,0 +1,20 @@ +/* { dg-do compile { target { ! { nvptx*-*-* visium-*-* } } } } */ +/* { dg-additional-options "-fno-pie" { target sparc*-*-* } } */ + +void + __attribute__((patchable_function_entry(65536))) +foo1 (void) +{ /* { dg-warning "'patchable_function_entry' attribute argument '65536' is out of range" } */ +} + +void + __attribute__((patchable_function_entry(65536,1))) +foo2 (void) +{ /* { dg-warning "'patchable_function_entry' attribute argument '65536' is out of range" } */ +} + +void + __attribute__((patchable_function_entry(65536,65536))) +foo3 (void) +{ /* { dg-warning "'patchable_function_entry' attribute argument '65536' is out of range" } */ +} diff --git a/gcc/varasm.c b/gcc/varasm.c index dc6da6c0b5b..9179fecdf85 100644 --- a/gcc/varasm.c +++ b/gcc/varasm.c @@ -1857,34 +1857,8 @@ assemble_start_function (tree decl, const char *fnname) if (DECL_PRESERVE_P (decl)) targetm.asm_out.mark_decl_preserved (fnname); - unsigned HOST_WIDE_INT patch_area_size = function_entry_patch_area_size; - unsigned HOST_WIDE_INT patch_area_entry = function_entry_patch_area_start; - - tree patchable_function_entry_attr - = lookup_attribute ("patchable_function_entry", DECL_ATTRIBUTES (decl)); - if (patchable_function_entry_attr) - { - tree pp_val = TREE_VALUE (patchable_function_entry_attr); - tree patchable_function_entry_value1 = TREE_VALUE (pp_val); - - patch_area_size = tree_to_uhwi (patchable_function_entry_value1); - patch_area_entry = 0; - if (TREE_CHAIN (pp_val) != NULL_TREE) - { - tree patchable_function_entry_value2 - = TREE_VALUE (TREE_CHAIN (pp_val)); - patch_area_entry = tree_to_uhwi (patchable_function_entry_value2); - } - } - - if (patch_area_entry > patch_area_size) - { - if (patch_area_size > 0) - warning (OPT_Wattributes, - "patchable function entry %wu exceeds size %wu", - patch_area_entry, patch_area_size); - patch_area_entry = 0; - } + unsigned short patch_area_size = crtl->patch_area_size; + unsigned short patch_area_entry = crtl->patch_area_entry; /* Emit the patching area before the entry label, if any. */ if (patch_area_entry > 0) -- 2.24.1