Bug 102731 - inconsistent handling of dereferncing a null pointer
Summary: inconsistent handling of dereferncing a null pointer
Status: UNCONFIRMED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 12.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
Depends on:
Blocks: Warray-bounds Wstringop-overflow
  Show dependency treegraph
 
Reported: 2021-10-13 16:13 UTC by Martin Sebor
Modified: 2021-10-13 16:13 UTC (History)
0 users

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Sebor 2021-10-13 16:13:01 UTC
The test case below shows that GCC doesn't handle null pointer dereferences consistently or as helpfully as it could or should.  Of the three equivalent functions, GCC only issues a warning pointing out the invalid access only for the first.  For the other two it doesn't warn even though it clearly detects the invalid access and injects a trap after it.

$ cat x.c && gcc -O2 -S -Wall -fdump-tree-optimized=/dev/stdout x.c
struct A { char n, a[4]; };

void f (struct A *p)
{
  if (p)
    return;
  __builtin_memset (p->a, 0, 4);   // warning, no trap
}

void g (struct A *p)
{
  if (p)
    return;

  p->a[0] = 0;                     // trap, no warning
  p->a[1] = 0;
  p->a[2] = 0;
  p->a[3] = 0;
}

void h (struct A *p)
{
  if (p)
    return;

  for (int i = 0; i != 4; ++i)
    p->a[i] = 0;                   // trap, no warning
}

x.c: In function ‘f’:
x.c:7:3: warning: ‘__builtin_memset’ offset [0, 3] is out of the bounds [0, 0] [-Warray-bounds]
    7 |   __builtin_memset (p->a, 0, 4);   // warning, no trap
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~

;; Function f (f, funcdef_no=0, decl_uid=1981, cgraph_uid=1, symbol_order=0)

Removing basic block 5
void f (struct A * p)
{
  <bb 2> [local count: 1073741824]:
  if (p_3(D) != 0B)
    goto <bb 4>; [70.93%]
  else
    goto <bb 3>; [29.07%]

  <bb 3> [local count: 312136752]:
  __builtin_memset (1B, 0, 4); [tail call]

  <bb 4> [local count: 1073741824]:
  return;

}



;; Function g (g, funcdef_no=1, decl_uid=1984, cgraph_uid=2, symbol_order=1)

void g (struct A * p)
{
  <bb 2> [local count: 1073741824]:
  if (p_2(D) != 0B)
    goto <bb 4>; [54.59%]
  else
    goto <bb 3>; [45.41%]

  <bb 3> [local count: 487586160]:
  MEM[(struct A *)0B].a[0] ={v} 0;
  __builtin_trap ();

  <bb 4> [local count: 1073741824]:
  return;

}



;; Function h (h, funcdef_no=2, decl_uid=1987, cgraph_uid=3, symbol_order=2)

void h (struct A * p)
{
  <bb 2> [local count: 472909864]:
  if (p_4(D) != 0B)
    goto <bb 4>; [54.59%]
  else
    goto <bb 3>; [45.41%]

  <bb 3> [local count: 214748368]:
  MEM[(struct A *)0B].a[0] ={v} 0;
  __builtin_trap ();

  <bb 4> [local count: 472909864]:
  return;

}
Comment 1 Martin Sebor 2021-10-13 16:13:29 UTC
Ideally, all three instances of the invalid access would be handled the same: by issuing an appropriate warning (preferably more descriptive of the problem than the one below) and injecting a trap (perhaps under the control of some option).

The -Warray-bounds (or, with it disabled, the equivalent -Wstringop-overflow) instance is the result of the logic in compute_objsize() for constant addresses:

  if (code == INTEGER_CST)
    {
      /* Pointer constants other than null are most likely the result
	 of erroneous null pointer addition/subtraction.  Set size to
	 zero.  For null pointers, set size to the maximum for now
	 since those may be the result of jump threading.  */
      if (integer_zerop (ptr))
	pref->set_max_size_range ();
      else
	pref->sizrng[0] = pref->sizrng[1] = 0;
      pref->ref = ptr;

      return true;
    }

Warnings issued due to this logic are discussed in pr99578 and its duplicates.  It's inconvenient for projects (like the kernel) that deliberately accesses objects at constant addresses.  The purpose of this bug is to show that the logic isn't sufficiently effective and the warnings issued due to it not sufficiently clear for users to understand.  A better solution is needed, preferably one that diagnoses the null pointer arithmetic before it's folded into non-null constant dereference.