On Thu, Mar 4, 2021 at 1:19 AM Andrew Pinski <pins...@gmail.com> wrote:
> On Wed, Mar 3, 2021 at 4:02 PM Christoph Müllner <cmuell...@gcc.gnu.org> > wrote: > > > > [CCing Andrew Pinski, who had the same question] > > > > On 2/15/21 1:12 PM, Richard Earnshaw wrote: > > > On 09/12/2020 17:21, Christoph Müllner wrote: > > >> From: Christoph Muellner <christoph.muell...@theobroma-systems.com> > > >> > > >> It is possible to call aarch64_declare_function_name() and > > >> have cfun not set. Let's sanitize the access to this variable. > > >> > > >> gcc/ > > >> > > >> * config/aarch64/aarch64.c (aarch64_declare_function_name): > > >> Santize access to cfun. > > >> --- > > >> gcc/config/aarch64/aarch64.c | 3 ++- > > >> 1 file changed, 2 insertions(+), 1 deletion(-) > > >> > > >> diff --git a/gcc/config/aarch64/aarch64.c > b/gcc/config/aarch64/aarch64.c > > >> index 67ffba02d3e..264ccb8beb2 100644 > > >> --- a/gcc/config/aarch64/aarch64.c > > >> +++ b/gcc/config/aarch64/aarch64.c > > >> @@ -19317,7 +19317,8 @@ aarch64_declare_function_name (FILE *stream, > const char* name, > > >> ASM_OUTPUT_TYPE_DIRECTIVE (stream, name, "function"); > > >> ASM_OUTPUT_LABEL (stream, name); > > >> > > >> - cfun->machine->label_is_assembled = true; > > >> + if (cfun) > > >> + cfun->machine->label_is_assembled = true; > > >> } > > >> > > >> /* Implement PRINT_PATCHABLE_FUNCTION_ENTRY. Check if the patch > area is after > > >> > > > > > > Do you have a simple testcase that demonstrates this? If so, we likely > > > have a regression here that will not only need fixing, but back-porting > > > as well. > > > > I was testing this on master back then in December and it this patch > > had an influence on the tests (make check), but I can't recall which > ones. > > I rebased the patches today and cannot reproduce the issues anymore with > "make check". > > I.e. this patch does not influence any tests as of today's master branch > (anymore). > > > > However, if I also apply patch 3/3 of this series, then this patch is > needed. > > More or less all new tests of patch 3/3 trigger this test otherwise. > > That still does not describe why cfun is null in the case of patch 3/3? > From looking into that patch I noticed you call set_cfun wtih null in > output_indirect_thunk_function shouldn't you push and pop the cfun > instead? > I got very much inspired by the function with the same name in gcc/config/i386/i386.c. Therefore I assumed, set_cfun(NULL) is fine. I guess, that the code does so, because it is called via the macro TARGET_ASM_CODE_END, and setting cfun to NULL is considered as prevention of mistakes. Thanks, Christoph > > Thanks, > Andrew Pinski > > > > > Thanks, > > Christoph >
