public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
From: "marcel.jacobse at ewetel dot net" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug fortran/101135] Use of absent assumed-shape array argument as an actual argument for an optional dummy argument mistakenly flagged as error by UndefinedBehaviourSanitizer
Date: Sat, 26 Jun 2021 13:20:09 +0000	[thread overview]
Message-ID: <bug-101135-4-dQrJF7kYHZ@http.gcc.gnu.org/bugzilla/> (raw)
In-Reply-To: <bug-101135-4@http.gcc.gnu.org/bugzilla/>

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101135

--- Comment #2 from Marcel Jacobse <marcel.jacobse at ewetel dot net> ---
I think I found the issue within gfortran by looking at the output of
-fdump-tree-all. For the example, the file a-example.f90.005t.original lists
this intermediate representation for test_wrapper:

__attribute__((fn spec (". w ")))
void test_wrapper (real(kind=4)[1] * restrict y)
{
  {
    struct array01_real(kind=4) parm.5;
    struct array01_real(kind=4) * D.3980;

    parm.5.span = 4;
    parm.5.dtype = {.elem_len=4, .rank=1, .type=3};
    parm.5.dim[0].lbound = 1;
    parm.5.dim[0].ubound = 1;
    parm.5.dim[0].stride = 1;
    parm.5.data = (void *) &(*y)[0];
    parm.5.offset = -1;
    D.3980 = y != 0B ? &parm.5 : 0B;
    test (D.3980);
  }
}

While there is a check for parameter y being present in the assignment for
D.3980, it kinda seems too late. For setting the data member of the parm.5
array descriptor, parameter y was already dereferenced unconditionally before.
So if y is absent, there is a null pointer dereference that UBSan seems to
complain about, even if its result is never used.

As a rookie attempt to fix this, I added a conditional to the data member
assignment:

diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c
index a6bcd2b5ab7..503ceba82ee 100644
--- a/gcc/fortran/trans-array.c
+++ b/gcc/fortran/trans-array.c
@@ -7068,6 +7068,13 @@ gfc_get_dataptr_offset (stmtblock_t *block, tree parm,
tree desc, tree offset,

   /* Set the target data pointer.  */
   offset = gfc_build_addr_expr (gfc_array_dataptr_type (desc), tmp);
+  if (expr->expr_type == EXPR_VARIABLE && expr->symtree->n.sym->attr.optional)
+    {
+      tree present = gfc_conv_expr_present (expr->symtree->n.sym);
+      offset = build3_loc (input_location, COND_EXPR, TREE_TYPE(offset),
+                           present, offset,
build_zero_cst(TREE_TYPE(offset)));
+      offset = gfc_evaluate_now (offset, block);
+    }
   gfc_conv_descriptor_data_set (block, parm, offset);
 }

With that, test_wrapper now looks like this:

__attribute__((fn spec (". w ")))
void test_wrapper (real(kind=4)[1] * restrict y)
{
  {
    struct array01_real(kind=4) parm.5;
    real(kind=4)[0:] * restrict D.3980;
    struct array01_real(kind=4) * D.3981;

    parm.5.span = 4;
    parm.5.dtype = {.elem_len=4, .rank=1, .type=3};
    parm.5.dim[0].lbound = 1;
    parm.5.dim[0].ubound = 1;
    parm.5.dim[0].stride = 1;
    D.3980 = y != 0B ? (real(kind=4)[0:] * restrict) &(*y)[0] : 0B;
    parm.5.data = (void *) D.3980;
    parm.5.offset = -1;
    D.3981 = y != 0B ? &parm.5 : 0B;
    test (D.3981);
  }
}

That means y is only dereferenced if it is present, and UBSan does not throw an
error anymore.

This fix seems quite hacky to me with how late it applies, but at least it
highlights the issue well to my mind. I suppose a better, proper fix would
maybe wrap the whole initialization of the array descriptor in a conditional
branch? So that the array descriptor is only assigned if the parameter is not
absent. Perhaps that conditional could surround gfc_conv_expr_descriptor or
even one level higher gfc_conv_array_parameter.

  parent reply	other threads:[~2021-06-26 13:20 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-19 12:49 [Bug fortran/101135] New: " marcel.jacobse at ewetel dot net
2021-06-26  1:07 ` [Bug fortran/101135] " marcel.jacobse at ewetel dot net
2021-06-26 13:20 ` marcel.jacobse at ewetel dot net [this message]
2022-01-27 20:38 ` [Bug fortran/101135] Load of null pointer when passing absent assumed-shape array argument for an optional dummy argument anlauf at gcc dot gnu.org
2022-01-28 22:22 ` anlauf at gcc dot gnu.org
2022-01-29 21:43 ` anlauf at gcc dot gnu.org
2022-01-31 20:49 ` anlauf at gcc dot gnu.org
2024-03-17 21:42 ` cvs-commit at gcc dot gnu.org
2024-03-23 20:13 ` cvs-commit at gcc dot gnu.org
2024-03-23 20:14 ` anlauf at gcc dot gnu.org
2024-03-23 21:01 ` marcel.jacobse at ewetel dot net

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bug-101135-4-dQrJF7kYHZ@http.gcc.gnu.org/bugzilla/ \
    --to=gcc-bugzilla@gcc.gnu.org \
    --cc=gcc-bugs@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).