public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: gcc-patches@gcc.gnu.org
Cc: Marek Polacek <polacek@redhat.com>, Dmitry Vyukov <dvyukov@google.com>
Subject: Re: [Patch] pr65779 - [5/6 Regression] undefined local symbol on powerpc
Date: Mon, 20 Apr 2015 07:47:00 -0000	[thread overview]
Message-ID: <20150420073507.GA1725@tucnak.redhat.com> (raw)
In-Reply-To: <20150420031049.GB12627@bubble.grove.modra.org>

On Mon, Apr 20, 2015 at 12:40:49PM +0930, Alan Modra wrote:
> with the log for the ubsan fails
> /src/gcc-5/gcc/testsuite/c-c++-common/ubsan/object-size-10.c:19:11: runtime error: index 128 out of bounds for type 'char [128]'
> /src/gcc-5/gcc/testsuite/c-c++-common/ubsan/object-size-10.c:19:11: runtime error: load of address 0x0804a000 with insufficient space for an object of type 'char'
> 0x0804a000: note: pointer points here
> <memory cannot be printed>

The issue here is that libsanitizer wants to print some context around the
variable, and doesn't try too hard, so if the variable is too close to the
end of the RW PT_LOAD, you get different message from what is expected.
In your case, most likely the end of the array happens to be exactly at the
end of the PT_LOAD segment.

So, the fix is either to try harder in ubsan renderMemorySnippet function
(it first computes the region it wishes to print, then has
  if (!IsAccessibleMemoryRange(Min, Max - Min)) {
    Printf("<memory cannot be printed>\n");
    return;
  }
).  Supposedly it could, if there are any page boundary crosses in the
Min .. Max region lower a little bit (to the page boundary) the end and/or
increase to the page boundary the start, and retry with that if it is
accessible.

Or we'd need to make the testcases that suffer from this accept also
the <memory cannot be printed> in place of the memory content line, line
with ^ marker (don't remember if there is yet another one).

> gcc/
> 	PR debug/65779
> 	* shrink-wrap.c (insn_uses_reg): New function.
> 	(move_insn_for_shrink_wrap): Remove debug insns using regs set
> 	by the moved insn.
> gcc/testsuite/
> 	* gcc.dg/pr65779.c: New.
> 
> Index: gcc/shrink-wrap.c
> ===================================================================
> --- gcc/shrink-wrap.c	(revision 222160)
> +++ gcc/shrink-wrap.c	(working copy)
> @@ -182,6 +182,21 @@ live_edge_for_reg (basic_block bb, int regno, int
>    return live_edge;
>  }
>  
> +static bool
> +insn_uses_reg (rtx_insn *insn, unsigned int regno, unsigned int end_regno)
> +{
> +  df_ref use;
> +
> +  FOR_EACH_INSN_USE (use, insn)
> +    {
> +      rtx reg = DF_REF_REG (use);
> +
> +      if (REG_P (reg) && REGNO (reg) >= regno && REGNO (reg) < end_regno)
> +	return true;
> +    }
> +  return false;
> +}
> +
>  /* Try to move INSN from BB to a successor.  Return true on success.
>     USES and DEFS are the set of registers that are used and defined
>     after INSN in BB.  SPLIT_P indicates whether a live edge from BB
> @@ -340,10 +355,15 @@ move_insn_for_shrink_wrap (basic_block bb, rtx_ins
>        *split_p = true;
>      }
>  
> +  vec<basic_block> live_bbs;
> +  if (MAY_HAVE_DEBUG_INSNS)
> +    live_bbs.create (5);

Just wonder if using an
  auto_vec<basic_block, 5> live_bbs;

> +	  FOR_BB_INSNS_REVERSE (tmp_bb, dinsn)
> +	    {
> +	      if (dinsn == insn)
> +		break;
> +	      if (DEBUG_INSN_P (dinsn)
> +		  && insn_uses_reg (dinsn, dregno, end_dregno))
> +		{
> +		  if (*split_p)
> +		    /* If split, then we will be moving insn into a
> +		       newly created block immediately after the entry
> +		       block.  Move the debug info there too.  */
> +		    emit_debug_insn_after (PATTERN (dinsn), bb_note (bb));
> +		  delete_insn (dinsn);

Debug insns should never be deleted, nor moved.  You should either
reset them
(INSN_VAR_LOCATION_LOC (insn) = gen_rtx_UNKNOWN_VAR_LOC (); plus
df_insn_rescan_debug_internal (insn);), or try to adjust them based on the
instruction setting the register (say, if insn sets the register to
some other register + 10 and the other register is still live, you could
replace the uses of the register with (plus (the other register) (const_int 10)).

> +      live_bbs.release ();

If live_bbs is auto_vec, this would not be needed.

	Jakub

  reply	other threads:[~2015-04-20  7:47 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-20  3:11 Alan Modra
2015-04-20  7:47 ` Jakub Jelinek [this message]
2015-04-20  8:42   ` Alan Modra
2015-04-20  8:56     ` Jakub Jelinek
2015-04-20 13:00       ` Alan Modra
2015-04-20 13:17         ` Jakub Jelinek
2015-04-21 11:38           ` Alan Modra
2015-04-21 12:49             ` Jakub Jelinek
2015-05-02  3:27               ` Alan Modra
2015-04-20 18:17 ` Jeff Law
2015-04-20 23:56   ` Alan Modra

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=20150420073507.GA1725@tucnak.redhat.com \
    --to=jakub@redhat.com \
    --cc=dvyukov@google.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=polacek@redhat.com \
    /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).