public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
From: "msebor at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug middle-end/99578] gcc-11 -Warray-bounds or -Wstringop-overread warning when accessing a pointer from integer literal
Date: Mon, 15 Mar 2021 19:57:33 +0000	[thread overview]
Message-ID: <bug-99578-4-v71JHa3I4T@http.gcc.gnu.org/bugzilla/> (raw)
In-Reply-To: <bug-99578-4@http.gcc.gnu.org/bugzilla/>

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

--- Comment #8 from Martin Sebor <msebor at gcc dot gnu.org> ---
(In reply to Arnd Bergmann from comment #6)
> I figured out the qnx4 warning in the end: https://godbolt.org/z/hvqjr3

The false positive is a known problem caused by redundancy elimination (the
FRE/PRE passes) substituting one member for another when they both refer to the
same address.  It often happens with union members that share the same starting
offset but can also happen with struct members in code that refers to both the
end of one and the beginning of the next member with no intervening padding.

There's no particular reason why it picks one or the other member (maybe it
picks whichever it sees first in some order, I don't know), but the warning
only triggers when it substitutes the smaller of the two members in a larger
access.  In the strnlen test case w/o sanitization it happens to pick the
bigger member.  This can be seen in the output of -fdump-tree-pre-details:

Inserted pretmp_12 = &de_9(D)->D.2393.D.2392.dl_fname;
 in predecessor 3 (0004)
Replaced &de_9(D)->D.2393.D.2392.dl_fname with pretmp_12 in all uses of _5 =
&de_9(D)->D.2393.D.2392.dl_fname;
Replaced &de_9(D)->D.2393.D.2389.di_fname with pretmp_12 in all uses of _3 =
&de_9(D)->D.2393.D.2389.di_fname;
Removing dead stmt _3 = &de_9(D)->D.2393.D.2389.di_fname;
Removing dead stmt _5 = &de_9(D)->D.2393.D.2392.dl_fname;

The instrumentation introduced by the sanitizers changes the IL in ways that
affect the choices made by subsequent transformations.  In this case, the
sanitizer first inserts a call to __builtin___tsan_read1() to record the access
to di_fname[0].  The sanitizers run after PRE but before some FRE iterations. 
The resulting IL from the thread sanitizer looks like this:

  <bb 2> [local count: 1073741824]:
  _13 = __builtin_return_address (0);
  __builtin___tsan_func_entry (_13);
  _5 = &de_9(D)->D.2390.D.2386.di_fname[0];
  __builtin___tsan_read1 (_5);                    <<< record access to
di_fname[0]
  _1 = de_9(D)->D.2390.D.2386.di_fname[0];
  if (_1 == 0)
    goto <bb 7>; [34.00%]
  else
    goto <bb 3>; [66.00%]

  <bb 7> [local count: 365072224]:
  goto <bb 6>; [100.00%]

  <bb 3> [local count: 708669601]:
  _3 = &de_9(D)->di_status;
  __builtin___tsan_read1 (_3);
  _2 = de_9(D)->di_status;
  pretmp_12 = &de_9(D)->D.2390.D.2389.dl_fname;   <<< temporary added by PRE
  if (_2 != 0)
    goto <bb 4>; [50.00%]
  else
    goto <bb 5>; [50.00%]

  <bb 4> [local count: 354334800]:
  _4 = __builtin_strnlen (pretmp_12, 16);
  _11 = (int) _4;
  goto <bb 6>; [100.00%]

  <bb 5> [local count: 354334800]:
  _6 = __builtin_strnlen (pretmp_12, 48);
  _10 = (int) _6;

  <bb 6> [local count: 1073741824]:
  # _7 = PHI <0(7), _11(4), _10(5)>
  __builtin___tsan_func_exit ();
  return _7;

}

The IL above is then changed by FRE which replaces dl_fname with di_fname:

Value numbering stmt = pretmp_12 = &de_9(D)->D.2390.D.2389.dl_fname;
Setting value number of pretmp_12 to _5 (changed)
_5 is available for _5
Replaced &de_9(D)->D.2390.D.2389.dl_fname with _5 in all uses of pretmp_12 =
&de_9(D)->D.2390.D.2389.dl_fname;

The warning is then issued by the strlen pass that runs after FRE5 and that
sees the IL below:

  <bb 2> [local count: 1073741824]:
  _13 = __builtin_return_address (0);
  __builtin___tsan_func_entry (_13);
  _5 = &de_9(D)->D.2393.D.2389.di_fname[0];   <<< inserted by sanitizer
  __builtin___tsan_read1 (_5);                <<< record access to di_fname[0] 
  _1 = de_9(D)->D.2393.D.2389.di_fname[0];    <<< inserted by FRE5
  if (_1 == 0)
    goto <bb 6>; [34.00%]
  else
    goto <bb 3>; [66.00%]

  <bb 3> [local count: 708669601]:
  _3 = &de_9(D)->di_status;
  __builtin___tsan_read1 (_3);
  _2 = de_9(D)->di_status;
  if (_2 != 0)
    goto <bb 4>; [50.00%]
  else
    goto <bb 5>; [50.00%]

  <bb 4> [local count: 354334800]:
  _4 = strnlen (_5, 16);
  _11 = (int) _4;
  goto <bb 6>; [100.00%]

  <bb 5> [local count: 354334800]:
  _6 = strnlen (_5, 48);                      <<< -Wstringop-overread
  _10 = (int) _6;

  <bb 6> [local count: 1073741824]:
  # _7 = PHI <0(2), _11(4), _10(5)>
  __builtin___tsan_func_exit ();
  return _7;

}

I'm hoping to change PRE/FRE in GCC 12 to replace the member substitution with
one involving &object + offsetof (type, memer).  That will avoid the false
positives in these cases with no adverse impact on optimization.

  parent reply	other threads:[~2021-03-15 19:57 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-13 14:23 [Bug c/99578] New: " arnd at linaro dot org
2021-03-13 20:40 ` [Bug c/99578] " msebor at gcc dot gnu.org
2021-03-13 21:40 ` arnd at linaro dot org
2021-03-13 22:38 ` arnd at linaro dot org
2021-03-14  0:57 ` [Bug middle-end/99578] " msebor at gcc dot gnu.org
2021-03-14 11:54 ` arnd at linaro dot org
2021-03-14 21:25 ` arnd at linaro dot org
2021-03-15  8:38 ` rguenth at gcc dot gnu.org
2021-03-15 19:57 ` msebor at gcc dot gnu.org [this message]
2021-03-15 20:24 ` msebor at gcc dot gnu.org
2021-04-21 19:34 ` pinskia at gcc dot gnu.org
2021-04-28 16:11 ` msebor at gcc dot gnu.org
2021-05-01 15:08 ` andi-gcc at firstfloor dot org
2021-05-19 15:07 ` msebor at gcc dot gnu.org
2021-05-19 18:01 ` andrew.cooper3 at citrix dot com
2021-05-19 19:19 ` andrew.cooper3 at citrix dot com
2021-05-19 20:48 ` msebor at gcc dot gnu.org
2021-05-30 23:40 ` msebor at gcc dot gnu.org
2021-08-24 16:03 ` pinskia at gcc dot gnu.org
2021-09-14 18:46 ` [Bug middle-end/99578] [11/12 Regression] " pinskia at gcc dot gnu.org
2021-12-19 11:36 ` pinskia at gcc dot gnu.org
2021-12-21 13:53 ` pmenzel+gcc at molgen dot mpg.de
2022-01-14 15:57 ` pmenzel+gcc at molgen dot mpg.de
2022-01-21 13:18 ` rguenth at gcc dot gnu.org
2022-02-23 10:36 ` pinskia at gcc dot gnu.org
2022-02-23 12:53 ` jakub at gcc dot gnu.org
2022-02-23 16:50 ` msebor at gcc dot gnu.org
2022-02-23 16:57 ` jakub at gcc dot gnu.org
2022-02-23 17:55 ` msebor at gcc dot gnu.org
2022-03-07 19:30 ` pinskia at gcc dot gnu.org
2022-03-07 20:53 ` goswin-v-b at web dot de
2022-03-16 19:49 ` kees at outflux dot net
2022-03-16 21:15 ` msebor at gcc dot gnu.org
2022-03-16 23:10 ` jwerner at chromium dot org
2022-03-17 10:49 ` redi at gcc dot gnu.org
2022-03-17 10:53 ` redi at gcc dot gnu.org
2022-03-17 12:52 ` jakub at gcc dot gnu.org
2022-03-18 18:02 ` cvs-commit at gcc dot gnu.org
2022-03-18 18:05 ` [Bug middle-end/99578] [11 " jakub at gcc dot gnu.org
2022-03-19 11:02 ` goswin-v-b at web dot de
2022-03-29  5:54 ` cvs-commit at gcc dot gnu.org
2022-03-30  8:04 ` marxin at gcc dot gnu.org
2022-03-30  8:16 ` jakub 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-99578-4-v71JHa3I4T@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).