Re: [Patch, fortran] PR fortran/92142 - CFI_setpointer corrupts descriptor

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [Patch, fortran] PR fortran/92142 - CFI_setpointer corrupts descriptor

Tobias Burnus
Hi,

+      fprintf (stderr, "CFI_setpointer: Result is NULL.\n");

> + return CFI_INVALID_DESCRIPTOR;
> +! { dg-do run }
> +! { dg-additional-options "-fbounds-check" }
> +! { dg-additional-sources ISO_Fortran_binding_15.c }


If you generate to stdout/stderr like in this case, I think it makes
sense to also check for this output using "{dg-output …}".

Otherwise, it looks okay at a glance – but I defer the proper review to
either someone else or to later.

Another question would be: Is it always guaranteed that
result->attribute  is set? I am asking because it resembles to the
untrained eye the code at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92027

And there, the result attribute is unset – that might be a bug in the C
code of the test itself – or in libgomp. But it doesn't harm to quickly
think about whether that can be an issue here as well or not.

Cheers,

Tobias

Reply | Threaded
Open this post in threaded view
|

Re: [Patch, fortran] PR fortran/92142 - CFI_setpointer corrupts descriptor

Paul Richard Thomas
I will deal with this and various other issues associated with
ISO_Fortran_binding tomorrow.

Thanks for your help

Paul

On Thu, 17 Oct 2019 at 18:30, Tobias Burnus <[hidden email]> wrote:

>
> Hi,
>
> +      fprintf (stderr, "CFI_setpointer: Result is NULL.\n");
> …
> > + return CFI_INVALID_DESCRIPTOR;
> > +! { dg-do run }
> > +! { dg-additional-options "-fbounds-check" }
> > +! { dg-additional-sources ISO_Fortran_binding_15.c }
>
>
> If you generate to stdout/stderr like in this case, I think it makes
> sense to also check for this output using "{dg-output …}".
>
> Otherwise, it looks okay at a glance – but I defer the proper review to
> either someone else or to later.
>
> Another question would be: Is it always guaranteed that
> result->attribute  is set? I am asking because it resembles to the
> untrained eye the code at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92027
>
> And there, the result attribute is unset – that might be a bug in the C
> code of the test itself – or in libgomp. But it doesn't harm to quickly
> think about whether that can be an issue here as well or not.
>
> Cheers,
>
> Tobias
>


--
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein
Reply | Threaded
Open this post in threaded view
|

Re: [Patch, fortran] PR fortran/92142 - CFI_setpointer corrupts descriptor

José Rui Faustino de Sousa
In reply to this post by Tobias Burnus
Hi,

On 17/10/19 17:29, Tobias Burnus wrote:
> If you generate to stdout/stderr like in this case, I think it makes
> sense to also check for this output using "{dg-output …}".
>

Added the suggested check and a few touches to comments and the error
message.

> that might be a bug in the C
> code of the test itself
>

I took a look and although there are problems with the code of the test
I do not think they are relevant.

Index: gcc/testsuite/gfortran.dg/ISO_Fortran_binding_10.c
===================================================================
--- gcc/testsuite/gfortran.dg/ISO_Fortran_binding_10.c (revision 277537)
+++ gcc/testsuite/gfortran.dg/ISO_Fortran_binding_10.c (working copy)
@@ -15,7 +15,7 @@
    bool err;
    CFI_CDESC_T(1) that;
    CFI_index_t lb[] = { 0, 0 };
-  CFI_index_t ub[] = { 4, 1 };
+  CFI_index_t ub[] = { 4, 0 };
    CFI_index_t st[] = { 2, 0 };
    int chksum[] = { 9, 36, 38 };

@@ -39,7 +39,7 @@

    if (*status != CFI_SUCCESS)
      {
-      printf("FAIL C: status is %i\n",status);
+      printf("FAIL C: status is %i\n", *status);
        return;
      }

Best regards,
José Rui


PR92142.patch (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Patch, fortran] PR fortran/92142 - CFI_setpointer corrupts descriptor

Tobias Burnus-3
Hi José,

On 10/29/19 11:35 AM, José Rui Faustino de Sousa wrote:
> Added the suggested check and a few touches to comments and the error
> message.
Thanks.
>> that might be a bug in the C code of the test itself
> I took a look and although there are problems with the code of the
> test I do not think they are relevant.
>
> Index: gcc/testsuite/gfortran.dg/ISO_Fortran_binding_10.c

I have included that patch.

Other remark: ChangeLog missing in the updated version of the patch. (I
just saw that you did include it when posting initially.)

I have now committed the patch as Rev. 278048 to the trunk.
Thanks for the patch!

Cheers,

Tobias


committed.diff (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Patch, fortran] PR fortran/92142 - CFI_setpointer corrupts descriptor

Tobias Burnus-3
On 11/11/19 2:58 PM, Mark Eggleston wrote:

> Unfortunately ISO_Fortran_binding_16.f90 contains a typo resulting in:
> the cause is that it refers to ISO_Fortran_binding_15.c instead of
> ISO_Fortran_binding_16.c in the dg-additional-sources directive.

I was sure that I tested it and committed the right version after the
renaming was needed. (Paul's patch used the same file names).

In any case, it is now corrected in Rev. 278055.

Thanks,

Tobias


committed.diff (1K) Download Attachment