Bug 82813

Summary: warning: '.builtin_memcpy' writing between 2 and 6 bytes into a region of size 0 overflows the destination [-Wstringop-overflow=]
Product: gcc Reporter: Martin Liška <marxin>
Component: adaAssignee: Not yet assigned to anyone <unassigned>
Status: RESOLVED FIXED    
Severity: normal CC: charlet, ebotcazou, msebor
Priority: P3 Keywords: diagnostic
Version: unknown   
Target Milestone: 7.4   
Host: Target:
Build: Known to work:
Known to fail: Last reconfirmed: 2017-11-02 00:00:00
Bug Depends on: 80545    
Bug Blocks: 88443    
Attachments: optimized tree dump

Description Martin Liška 2017-11-02 14:32:31 UTC
Created attachment 42536 [details]
optimized tree dump

Hello.

Not being sure whether it's real issue or false positive:

   214                    elsif Including_RTS then
   215                       for J in Start .. Sw'Last loop
   216                          if Sw (J) = Directory_Separator then
   217                             Switch :=
   218                               new String'
   219                                 (Sw (1 .. Start - 1)
   220                                  & Parent
   221                                  & Directory_Separator
   222                                  & Sw (Start .. Sw'Last));
   223                             return;
   224                          end if;
   225                       end loop;

Optimized dump is in attachment.
Comment 1 Eric Botcazou 2017-11-02 16:01:38 UTC
I can reproduce.
Comment 2 Eric Botcazou 2017-11-02 16:29:59 UTC
It's unreachable code:

  <bb 56> [0.23%] [count: INV]:
  # DEBUG S117b.276 => &D.9593
  .builtin_memcpy (&MEM[(void *)&D.9593], pretmp_305, _250);

The only predecessor is:

  <bb 55> [0.32%] [count: INV]:
  L113b_224 = start_130 + _322;
  # DEBUG L114b => 1
  # DEBUG make_util__ensure_absolute_path__B_4__L_5__TTS117bSP1___L => 1
  # DEBUG make_util__ensure_absolute_path__B_4__L_5__TTS117bSP1___U => L113b_224
  # DEBUG D.7306 => 1
  # DEBUG iftmp.271 => 1 + 18446744073709551615
  if (L113b_224 != 0)
    goto <bb 54>; [64.00%]
  else
    goto <bb 56>; [36.00%]

whose only predecessor is:

  <bb 23> [0.63%] [count: INV]:
  _37 = parent$P_BOUNDS_1->UB0;
  _38 = parent$P_BOUNDS_1->LB0;
  _330 = iftmp.245_24 - start_130;
  _322 = _330 + 1;
  _250 = (sizetype) j_231;
  pretmp_305 = &MEM[(void *)sw.246_6][1 ...]{lb: 1 sz: 1};
  if (_37 >= _38)
    goto <bb 24>; [50.00%]
  else
    goto <bb 55>; [50.00%]

So <bb 56> is reachable only if parent$P_BOUNDS_1->UB0 < parent$P_BOUNDS_1->LB0 is true.

But this condition is caught by the block just above:

                  if Parent'Length = 0 then
                     Do_Fail
                       ("relative search path switches ("""
                        & Sw
                        & """) are not allowed");

  <bb 17> [7.26%] [count: INV]:
  _19 = parent$P_BOUNDS_1->LB0;
  _20 = parent$P_BOUNDS_1->UB0;
  if (_19 > _20)
    goto <bb 18>; [33.00%]
  else
    goto <bb 19>; [67.00%]

<bb 19> is a dominator of <bb 23> so <bb 23> is reachable only if parent$P_BOUNDS_1->LB0 > parent$P_BOUNDS_1->UB0 is false, which is the opposite of the above condition, so <bb 56> is not reachable.
Comment 3 Martin Sebor 2017-11-02 16:58:21 UTC
The warning will go away once pr80545 is fixed but I wonder if a better (independent) solution in this case is to detect that the code is, in fact, unreachable, and avoid emitting it to begin with.  Alternatively, if it is too hard to determine that it's unreachable, it should be possible to detect that the memcpy call is invalid earlier and replace it with either __builtin_unreachable or __builtin_trap.
Comment 4 Eric Botcazou 2017-11-02 17:20:43 UTC
> The warning will go away once pr80545 is fixed but I wonder if a better
> (independent) solution in this case is to detect that the code is, in fact,
> unreachable, and avoid emitting it to begin with.

Yes, it's a missed optimization at the GIMPLE level.

> Alternatively, if it is too hard to determine that it's unreachable, it should
> be possible to detect that the memcpy call is invalid earlier and replace it 
> with either __builtin_unreachable or __builtin_trap.

Possibly, 121t.phicprop1 has:

  S117b.276_245 = .builtin_alloca_with_align (0, 8);
  # DEBUG S117b.276 => S117b.276_245
  _46 = (sizetype) j_231;
  _48 = &*sw.246_6[1 ...]{lb: 1 sz: 1};
  _49 = &*S117b.276_245[1 ...]{lb: 1 sz: 1};
  .builtin_memcpy (_49, _48, _46);
Comment 5 Eric Botcazou 2018-03-12 22:40:37 UTC
Author: ebotcazou
Date: Mon Mar 12 22:40:05 2018
New Revision: 258466

URL: https://gcc.gnu.org/viewcvs?rev=258466&root=gcc&view=rev
Log:
	PR ada/82813
	* gcc-interface/misc.c (gnat_post_options): Disable string overflow
	warnings.

Modified:
    trunk/gcc/ada/ChangeLog
    trunk/gcc/ada/gcc-interface/misc.c
Comment 6 Eric Botcazou 2018-03-12 22:40:51 UTC
Author: ebotcazou
Date: Mon Mar 12 22:40:19 2018
New Revision: 258467

URL: https://gcc.gnu.org/viewcvs?rev=258467&root=gcc&view=rev
Log:
	PR ada/82813
	* gcc-interface/misc.c (gnat_post_options): Disable string overflow
	warnings.

Modified:
    branches/gcc-7-branch/gcc/ada/ChangeLog
    branches/gcc-7-branch/gcc/ada/gcc-interface/misc.c
Comment 7 Eric Botcazou 2018-03-12 22:41:58 UTC
Sort of.