public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
From: "rguenth at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug middle-end/109559] [12/13/14 Regression] Unexpected -Wmaybe-uninitialized warning when inlining with system header
Date: Mon, 05 Feb 2024 10:45:24 +0000	[thread overview]
Message-ID: <bug-109559-4-9RO9Vm55V1@http.gcc.gnu.org/bugzilla/> (raw)
In-Reply-To: <bug-109559-4@http.gcc.gnu.org/bugzilla/>

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |msebor at gcc dot gnu.org
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
Note the diagnostic is "valid" and for

FilonIntegral::integrate ()

<bb2:>
function_base::has_trivial_copy_and_destroy (&MEM[(struct function1
*)&f1].D.2804);

and we're using the stmts location to diagnose this which expands to

(gdb) p IS_ADHOC_LOC (location)
$8 = true
(gdb) p get_location_from_adhoc_loc (line_table, location)
$9 = 268224
(gdb) p expand_location ($9)
$10 = {file = 0x4d895d0 "", line = 5, column = 49, data = 0x0, sysp = true}

so the system header flag is correct.  There's

  if ((was_warning || diagnostic->kind == DK_WARNING)
      && ((!m_warn_system_headers
           && diagnostic->m_iinfo.m_allsyslocs)
          || m_inhibit_warnings))
    /* Bail if the warning is not to be reported because all locations in the
       inlining stack (if there is one) are in system headers.  */
    return false; 

I've added -Wno-system-headers, and

(gdb) p m_warn_system_headers
$14 = false
(gdb) p diagnostic->m_iinfo.m_allsyslocs
$15 = false
(gdb) p was_warning
$16 = true
(gdb) p m_inhibit_warnings
$17 = false

so the issue seems to be that the active m_set_locations_cb
tree-diagnostic.cc:set_inlining_locations computes that "wrongly".

The operator= associated inline block location isn't in a system header
(the abstract origin, the operator= FUNCTION_DECL does have a
DECL_SOURCE_LOCATION that's in a system header though).

_Note_ we're assigning that BLOCK the location of the _call_ (it's for
the parameter setup), _not_ the location of the callee!

  /* Build a block containing code to initialize the arguments, the
     actual inline expansion of the body, and a label for the return
     statements within the function to jump to.  The type of the
     statement expression is the return type of the function call.
     ???  If the call does not have an associated block then we will
     remap all callee blocks to NULL, effectively dropping most of
     its debug information.  This should only happen for calls to
     artificial decls inserted by the compiler itself.  We need to
     either link the inlined blocks into the caller block tree or
     not refer to them in any way to not break GC for locations.  */
  if (tree block = gimple_block (stmt))
    {
      /* We do want to assign a not UNKNOWN_LOCATION BLOCK_SOURCE_LOCATION
         to make inlined_function_outer_scope_p return true on this BLOCK.  */
      location_t loc = LOCATION_LOCUS (gimple_location (stmt));
      if (loc == UNKNOWN_LOCATION)
        loc = LOCATION_LOCUS (DECL_SOURCE_LOCATION (fn));
      if (loc == UNKNOWN_LOCATION)
        loc = BUILTINS_LOCATION;
      id->block = make_node (BLOCK);
      BLOCK_ABSTRACT_ORIGIN (id->block) = DECL_ORIGIN (fn);
      BLOCK_SOURCE_LOCATION (id->block) = loc;
      prepend_lexical_block (block, id->block);

since this particular hook implementation was added by Martin S. I don't
have high hopes of that being a concious decision.

  while (block && TREE_CODE (block) == BLOCK
         && BLOCK_ABSTRACT_ORIGIN (block))
    {
      tree ao = BLOCK_ABSTRACT_ORIGIN (block);
      if (TREE_CODE (ao) == FUNCTION_DECL)
        {
          if (!diagnostic->m_iinfo.m_ao)
            diagnostic->m_iinfo.m_ao = block;

          location_t bsloc = BLOCK_SOURCE_LOCATION (block);
          ilocs.safe_push (bsloc);
          if (in_system_header_at (bsloc))

I think this should either look at DECL_SOURCE_LOCATION (ao) or
at the location of the block nested in 'block'.

Note we then still warn because

  if (ilocs.length ())
    {
      /* When there is an inlining context use the macro expansion
         location for the original location and bump up NSYSLOCS if
         it's in a system header since it's not counted above.  */
      location_t sysloc = expansion_point_location_if_in_system_header (loc);
      if (sysloc != loc)

gets us the same location, failing to do

          loc = sysloc;
          ++nsyslocs;
        }

and then

  ilocs.safe_push (loc);

makes

  /* Set if all locations are in a system header.  */
  diagnostic->m_iinfo.m_allsyslocs = nsyslocs == ilocs.length ();

fail.  The logic is odd though, if it was not macro expanded it's off.

The following fixes it for me.

diff --git a/gcc/tree-diagnostic.cc b/gcc/tree-diagnostic.cc
index a660c7d0785..a49f8939ce7 100644
--- a/gcc/tree-diagnostic.cc
+++ b/gcc/tree-diagnostic.cc
@@ -328,7 +328,7 @@ set_inlining_locations (diagnostic_context *,
          if (!diagnostic->m_iinfo.m_ao)
            diagnostic->m_iinfo.m_ao = block;

-         location_t bsloc = BLOCK_SOURCE_LOCATION (block);
+         location_t bsloc = DECL_SOURCE_LOCATION (ao);
          ilocs.safe_push (bsloc);
          if (in_system_header_at (bsloc))
            ++nsyslocs;
@@ -339,17 +339,15 @@ set_inlining_locations (diagnostic_context *,
       block = BLOCK_SUPERCONTEXT (block);
     }

+  location_t sysloc;
+  /* When there is an inlining context use the macro expansion
+     location for the original location and bump up NSYSLOCS if
+     it's in a system header since it's not counted above.  */
   if (ilocs.length ())
     {
-      /* When there is an inlining context use the macro expansion
-        location for the original location and bump up NSYSLOCS if
-        it's in a system header since it's not counted above.  */
-      location_t sysloc = expansion_point_location_if_in_system_header (loc);
-      if (sysloc != loc)
-       {
-         loc = sysloc;
-         ++nsyslocs;
-       }
+      if (in_system_header_at (loc))
+       ++nsyslocs;
+      loc = expansion_point_location_if_in_system_header (loc);
     }
   else
     {

  parent reply	other threads:[~2024-02-05 10:45 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-19 20:23 [Bug middle-end/109559] New: " mpolacek at gcc dot gnu.org
2023-04-19 20:25 ` [Bug middle-end/109559] [12/13/14 Regression] " mpolacek at gcc dot gnu.org
2023-04-19 20:30 ` mpolacek at gcc dot gnu.org
2023-05-08 12:27 ` rguenth at gcc dot gnu.org
2024-01-12 10:29 ` rguenth at gcc dot gnu.org
2024-02-01 22:12 ` mpolacek at gcc dot gnu.org
2024-02-05 10:45 ` rguenth at gcc dot gnu.org [this message]
2024-02-05 12:36 ` rguenth at gcc dot gnu.org
2024-02-05 13:00 ` rguenth at gcc dot gnu.org
2024-02-06  9:53 ` rguenth at gcc dot gnu.org

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-109559-4-9RO9Vm55V1@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).