[PATCH] Improve sequence logic in cxx_init_decl_processing

[PATCH] Improve sequence logic in cxx_init_decl_processing

[Olivier Hainque]

[-- Attachment #1: Type: text/plain, Size: 1348 bytes --]

Hello,

commit aa2c978400f3b3ca6e9f2d18598a379589e77ba0, introduced per

  https://gcc.gnu.org/pipermail/gcc-patches/2020-May/545552.html

makes references to __cxa_pure_virtual weak and this is causing
issues on some VxWorks configurations, where weak symbols are only
supported for one of the two major operating modes, and not on all
versions.

While trying to circumvent that, I noticed that the current
code in cxx_init_decl_processing does something like:

  if (flag_weak)
    /* If no definition is available, resolve references to NULL.  */
    declare_weak (abort_fndecl);
  ...
  if (! supports_one_only ())
    flag_weak = 0;


The code possibly resetting flag_weak should presumlably execute
before the test checking the flag, or we'd need a comment explaining
why this surprising order is on purpose.

The attached patch just moves the reset above the test.

It bootstraps/regtests fine on x86_64-linux and allows better control
on vxWorks. I'm not yet clear on some of the ramifications there (tigthening
the definitions of SUPPORTS_ONE_ONLY and TARGET_SUPPORTS_WEAK yields lots of
dg test failures) but that's another story.

Ok to commit?

Thanks in advance!

2021-12-30  Olivier Hainque  <hainque@adacore.com>

gcc/
	* cp/decl.c (cxx_init_decl_processing): Move code possibly
	altering flag_weak before code testing it.

Olivier


[-- Attachment #2: 0001-Improve-sequence-logic-in-cxx_init_decl_processing.patch --]
[-- Type: application/octet-stream, Size: 1412 bytes --]

From bac3f8fad76fa4f406e0c96906cfdd4b6609744c Mon Sep 17 00:00:00 2001
From: Olivier Hainque <hainque@adacore.com>
Date: Thu, 30 Dec 2021 20:47:31 +0000
Subject: [PATCH] Improve sequence logic in cxx_init_decl_processing

Checking for one_only/weak support is better done
before deciding to turn references to __cxa_pure_virtual weak.

This helps at least on VxWorks where one_only / weak support
varies between kernel and rtp modes as well as across VxWorks
versions.

2021-12-30  Olivier Hainque  <hainque@adacore.com>

gcc/
	* cp/decl.c (cxx_init_decl_processing): Move code possibly
	altering flag_weak before code testing it.
---
 gcc/cp/decl.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 0b71c00f5ab..993c7105784 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -4718,6 +4718,9 @@ cxx_init_decl_processing (void)
     nullptr_node = build_int_cst (nullptr_type_node, 0);
   }
 
+  if (! supports_one_only ())
+    flag_weak = 0;
+
   abort_fndecl
     = build_library_fn_ptr ("__cxa_pure_virtual", void_ftype,
 			    ECF_NORETURN | ECF_NOTHROW | ECF_COLD);
@@ -4733,9 +4736,6 @@ cxx_init_decl_processing (void)
   if (flag_exceptions)
     init_exception_processing ();
 
-  if (! supports_one_only ())
-    flag_weak = 0;
-
   if (modules_p ())
     init_modules (parse_in);
 
-- 
2.25.1


[-- Attachment #3: Type: text/plain, Size: 3 bytes --]

Re: [PATCH] Improve sequence logic in cxx_init_decl_processing

[Jason Merrill]

On 1/6/22 03:26, Olivier Hainque wrote:
> Hello,
> 
> commit aa2c978400f3b3ca6e9f2d18598a379589e77ba0, introduced per
> 
>    https://gcc.gnu.org/pipermail/gcc-patches/2020-May/545552.html
> 
> makes references to __cxa_pure_virtual weak and this is causing
> issues on some VxWorks configurations, where weak symbols are only
> supported for one of the two major operating modes, and not on all
> versions.
> 
> While trying to circumvent that, I noticed that the current
> code in cxx_init_decl_processing does something like:
> 
>    if (flag_weak)
>      /* If no definition is available, resolve references to NULL.  */
>      declare_weak (abort_fndecl);
>    ...
>    if (! supports_one_only ())
>      flag_weak = 0;
> 
> 
> The code possibly resetting flag_weak should presumlably execute
> before the test checking the flag, or we'd need a comment explaining
> why this surprising order is on purpose.
> 
> The attached patch just moves the reset above the test.
> 
> It bootstraps/regtests fine on x86_64-linux and allows better control
> on vxWorks. I'm not yet clear on some of the ramifications there (tigthening
> the definitions of SUPPORTS_ONE_ONLY and TARGET_SUPPORTS_WEAK yields lots of
> dg test failures) but that's another story.
> 
> Ok to commit?

OK.

> Thanks in advance!
> 
> 2021-12-30  Olivier Hainque  <hainque@adacore.com>
> 
> gcc/
> 	* cp/decl.c (cxx_init_decl_processing): Move code possibly
> 	altering flag_weak before code testing it.
> 
> Olivier
> 
> 
> 
> 
> 

Re: [PATCH] Improve sequence logic in cxx_init_decl_processing

[Olivier Hainque]

> On 10 Jan 2022, at 20:02, Jason Merrill <jason@redhat.com> wrote:
> 
>> The attached patch just moves the reset above the test.
> 
> OK.

Great, thanks Jason!

>> 2021-12-30  Olivier Hainque  <hainque@adacore.com>
>> gcc/
>> 	* cp/decl.c (cxx_init_decl_processing): Move code possibly
>> 	altering flag_weak before code testing it.
>> Olivier