diff mbox series

Fortran: Create fresh ts.u.cl for result in gfc_get_symbol_for_expr [PR118441]

Message ID ceed12e0-28b1-4e9e-ba21-e7fd18feb9bd@baylibre.com
State New
Headers
Series Fortran: Create fresh ts.u.cl for result in gfc_get_symbol_for_expr [PR118441] |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm fail Patch failed to apply
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 fail Patch failed to apply

Commit Message

Tobias Burnus Jan. 15, 2025, 10:49 p.m. UTC 
  As shown in the PR, updating the function declaration
for _gfortran_pack_char could leak into the character
typespec back-end declaration to the the caller by
setting the backend declaration of the caller.

The problem is that that function returns a character string
and in trans-intrinsic.cc, we obtain the function declaration
from the call (which may not always work, hmm).

In any case, the routine already takes care to of copying
the ts->u.cl character length data,
   i.e. sym->formal[->next etc.]->ts.u.cl
but it didn't do so for the result variable, itself,
i.e. sym->ts.u.cl.

Well, this patch does so – I am just not completely sure
what it should do for the character length. In Fortran,
the result can either be deferred - or has an explicit
length expression (such as 'len(n)'). I now went for
'NULL' instead of copying the length from the caller.

For this case, it does not matter as '_s' also has
ts->u.cl->length == NULL;

Build & regtested on x86-64-gnu-linux.
OK? Other thoughts?

Tobias

PS: The issue is long standing but since a recent GCC 15
commit (cf. PR), the issue is now triggered and causes an ICE
for the testcase from the PR.

Comments

Andre Vehreschild Jan. 16, 2025, 7:46 a.m. UTC | #1 
Hi Tobias,

the patch looks reasonable to me. I wouldn't know where to get the "correct"
charlen either. In fact I have the same problem at the moment with an array
spec. Anyway, one small question: Your testcase does not have a dg-do line. Is
this intentional? 

Thanks for the patch,
	Andre

On Wed, 15 Jan 2025 23:49:01 +0100
Tobias Burnus <tburnus@baylibre.com> wrote:

> As shown in the PR, updating the function declaration
> for _gfortran_pack_char could leak into the character
> typespec back-end declaration to the the caller by
> setting the backend declaration of the caller.
> 
> The problem is that that function returns a character string
> and in trans-intrinsic.cc, we obtain the function declaration
> from the call (which may not always work, hmm).
> 
> In any case, the routine already takes care to of copying
> the ts->u.cl character length data,
>    i.e. sym->formal[->next etc.]->ts.u.cl
> but it didn't do so for the result variable, itself,
> i.e. sym->ts.u.cl.
> 
> Well, this patch does so – I am just not completely sure
> what it should do for the character length. In Fortran,
> the result can either be deferred - or has an explicit
> length expression (such as 'len(n)'). I now went for
> 'NULL' instead of copying the length from the caller.
> 
> For this case, it does not matter as '_s' also has
> ts->u.cl->length == NULL;
> 
> Build & regtested on x86-64-gnu-linux.
> OK? Other thoughts?
> 
> Tobias
> 
> PS: The issue is long standing but since a recent GCC 15
> commit (cf. PR), the issue is now triggered and causes an ICE
> for the testcase from the PR.
diff mbox series

Patch

Fortran: Create fresh ts.u.cl for result in gfc_get_symbol_for_expr [PR118441]

For intrinsic routines, called in libraries, the prototype is created from
the call via gfc_get_symbol_for_expr. For the actual arguments, it calls
gfc_copy_formal_args_intr which already ensures that the ts.u.cl is freshly
allocated.

This commit now ensures the same for character-returning functions.

	PR fortran/118441

gcc/fortran/ChangeLog:

	* trans-intrinsic.cc (gfc_get_symbol_for_expr): Use
	gfc_new_charlen for character-returning functions.

gcc/testsuite/ChangeLog:

	* gfortran.dg/gomp/intrinsic_pack_7.f90: New test.

 gcc/fortran/trans-intrinsic.cc                      | 2 ++
 gcc/testsuite/gfortran.dg/gomp/intrinsic_pack_7.f90 | 9 +++++++++
 2 files changed, 11 insertions(+)

diff --git a/gcc/fortran/trans-intrinsic.cc b/gcc/fortran/trans-intrinsic.cc
index cc3a2e5fc10..afbec5b2752 100644
--- a/gcc/fortran/trans-intrinsic.cc
+++ b/gcc/fortran/trans-intrinsic.cc
@@ -4242,6 +4242,8 @@  gfc_get_symbol_for_expr (gfc_expr * expr, bool ignore_optional)
   sym = gfc_new_symbol (expr->value.function.name, NULL);
 
   sym->ts = expr->ts;
+  if (sym->ts.type == BT_CHARACTER)
+    sym->ts.u.cl = gfc_new_charlen (gfc_current_ns, NULL);
   sym->attr.external = 1;
   sym->attr.function = 1;
   sym->attr.always_explicit = 1;
diff --git a/gcc/testsuite/gfortran.dg/gomp/intrinsic_pack_7.f90 b/gcc/testsuite/gfortran.dg/gomp/intrinsic_pack_7.f90
new file mode 100644
index 00000000000..576a8994092
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/gomp/intrinsic_pack_7.f90
@@ -0,0 +1,9 @@ 
+! PR fortran/118441
+
+subroutine sub(s)
+  character(len=*), intent(inout) :: s(:)
+  integer :: n
+  s( : ) =       s(:)                     ! OK
+  n      = count(s(:) /= '')
+  s(1:n) = pack (s(:), mask=(s(:) /= '')) ! ICE
+end subroutine sub