From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <gcc-bugzilla@gcc.gnu.org>
Received: by sourceware.org (Postfix, from userid 48)
	id C80933858D39; Tue, 28 Mar 2023 11:53:59 +0000 (GMT)
DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org C80933858D39
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org;
	s=default; t=1680004439;
	bh=GKoCXK//uxhipoI+NBmm4ftxAPpCu7EGQ2Jm1Qqdfks=;
	h=From:To:Subject:Date:In-Reply-To:References:From;
	b=X2S/35HTZB7mkjxhbhZiYLhrFG0BY7Zu6DlSNdslNFiKsyWEvIlhlg9WGMek6UNy4
	 5rlMn71H6XHmNbVvWtatlt4SEB9qPeW86gFuaZZJOY0VTHQZmSFOzP3A1QV22VEI6i
	 t/k06XjkiBdpy4p6ce7rPl7qW+lZ6G7b4mYrlB2o=
From: "rguenth at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: =?UTF-8?B?W0J1ZyB0cmVlLW9wdGltaXphdGlvbi8xMDkyOTBdIHdhcm5pbmc6?=
 =?UTF-8?B?IGFycmF5IHN1YnNjcmlwdCAtNTAgaXMgb3V0c2lkZSBhcnJheSBib3VuZHMg?=
 =?UTF-8?B?b2Yg4oCYc3RydWN0IGtvYmplY3RbMzYwMjg3OTcwMTg5NjM5Njdd4oCZ?=
Date: Tue, 28 Mar 2023 11:53:58 +0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: tree-optimization
X-Bugzilla-Version: 13.0
X-Bugzilla-Keywords: diagnostic
X-Bugzilla-Severity: normal
X-Bugzilla-Who: rguenth at gcc dot gnu.org
X-Bugzilla-Status: NEW
X-Bugzilla-Resolution: 
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags: 
X-Bugzilla-Changed-Fields: cf_reconfirmed_on everconfirmed bug_status
Message-ID: <bug-109290-4-UMXZldw4UN@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-109290-4@http.gcc.gnu.org/bugzilla/>
References: <bug-109290-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
List-Id: <gcc-bugs.sourceware.org>

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2023-03-28
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
dropping -fno-delete-null-pointer-checks avoids these diagnostics ... becau=
se
we actually diagnose


# VUSE <.MEM_4(D)>
val_8 =3D MEM[(u64 *)0B + -416B];

and that would have been isolated/removed by isolate-paths.

<bb 2> [local count: 1073741824]:
_1 =3D &MEM[(struct btrfs_space_info *)kobj_3(D) + -584B].lock;
if (_1 !=3D 0B)
  goto <bb 4>; [53.47%]
else
  goto <bb 3>; [46.53%]

<bb 3> [local count: 499612072]:
val_8 =3D MEM[(u64 *)0B + -416B];
goto <bb 5>; [100.00%]

<bb 4> [local count: 574129753]:
_raw_spin_lock (_1);
val_10 =3D MEM[(u64 *)kobj_3(D) + -416B];
_raw_spin_unlock (_1);

<bb 5> [local count: 1073741824]:
# val_11 =3D PHI <val_8(3), val_10(4)>
_12 =3D sysfs_emit (buf_5(D), "%llu\n", val_11);
_13 =3D (long int) _12;
return _13;

and that's because we call

  btrfs_show_u64(&sinfo->disk_used, &sinfo->lock, buf);

and btrfs_show_u64 does

static ssize_t btrfs_show_u64(u64 *value_ptr, spinlock_t *lock, char *buf)
{
 u64 val;
 if (lock)
  spin_lock(lock);
 val =3D *value_ptr;
 if (lock)
  spin_unlock(lock);
 return sysfs_emit(buf, "%llu\n", val);
}

again the array-bounds diagnostic isn't very helpful - a
-Wnull-dereference diagnostic would be more helpful here.

And yes, we thread the double if (lock) here.

Without -fno-delete-null-pointer-checks we optimize the function to

  <bb 2> [local count: 1073741824]:
  _9 =3D &MEM[(struct btrfs_space_info *)kobj_3(D) + -584B].lock;
  _raw_spin_lock (_9);
  val_10 =3D MEM[(u64 *)kobj_3(D) + -456B];
  _raw_spin_unlock (_9);=20
  _12 =3D sysfs_emit (buf_5(D), "%llu\n", val_10);
  _13 =3D (long int) _12;
  return _13;

If you enable -Wnull-dereference (and disable -fno-delete-null-pointer-chec=
ks)
you get all these cases diagnosed:

In function 'to_fs_info',
    inlined from 'btrfs_discard_kbps_limit_store' at fs/btrfs/sysfs.c:542:3=
4:
fs/btrfs/sysfs.c:1318:10: warning: potential null pointer dereference
[-Wnull-dereference]
fs/btrfs/sysfs.c:1318:10: warning: potential null pointer dereference
[-Wnull-dereference]
...

so it's really a sign of bad coding / abstraction.

Again confirmed because the diagnostic from -Warray-bounds isn't very helpf=
ul
in pointing out the possible problem.=