This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC] Isolate & simplify paths with undefined behaviour


On 10/18/13 12:47, Marc Glisse wrote:

Maybe a new -fretroactive-undefined-behavior? (for later, obviously)
Something like that -- haven't thought much about the name.


* should cfg_altered be static (or a member of the pass class)?
At the minimum it should be static. Into the pass class would be better. I'll take care of that.


* tree-vrp has a function infer_nonnull_range, do you think we could
share it? We now store the VRP ranges for integers, but not for
pointers. If we did (or maybe just a non-null bit), the pass could just
test that bit on the variable found in the PHI.
I'm not sure what can really be shared here -- this patch searches for PHIs where one or more of the args is a NULL pointer. The NULL pointer will be explicit in the PHI arg. So a real world example (alias.i from gcc-4.6.0):

<bb 67>:
# iftmp.163_8 = PHI <iftmp.163_111(66), 0B(65), 0B(93)>
# _241 = PHI <_242(66), _109(65), _246(93)>
_112 = iftmp.163_8->u.hwint[0];
_113 = _241 + _112;
_114 = _113 * 8;
_115 = ref_28(D)->size;
if (_114 == _115)
  goto <bb 87>;
else
  goto <bb 68>;


So when BB67 is reached via BB65 or BB93 iftmp.163_8 will have the value 0 via the PHI node at the start of BB67 and it'll be derefereced in BB67. We'll isolate the 65->67 path and the 93->67 path.


What you're suggesting would be more useful for a warning pass which detected potential null pointer dereferences. If we continue with the same example and assume we isolated the two paths noted above we have this remaining as BB67:

<bb 67>:
# iftmp.163_8 = PHI <iftmp.163_111(66)>
# _241 = PHI <_242(66)>
_112 = iftmp.163_8->u.hwint[0];
_113 = _241 + _112;
_114 = _113 * 8;
_115 = ref_28(D)->size;
if (_114 == _115)
  goto <bb 87>;
else
  goto <bb 68>;


If we've determined via VRP that iftmp.163_111 has a non-null property, then we would not warn about a potential null pointer dereference. That's an example of where preserving the range information for pointers out of VRP would be useful.

I actually have another patch which does precisely these kinds of warnings. It needs more TLC before we could even consider integrating it.

BTW, if we continue with that example, immediately after isolation we have (I'm including our block's predecessor because it's going to merge). BB63 is the pred, BB64 is the block where we isolated out two paths:




;;   basic block 63, loop depth 0, count 0, freq 529, maybe hot
;;    prev block 62, next block 64, flags: (NEW, REACHABLE)
;;    pred:       62 [27.0%]  (TRUE_VALUE,EXECUTABLE)
  iftmp.161_102 = _99->size;
  _186 = iftmp.161_102->u.hwint[0];
  _69 = mem_25(D)->u.fld[1].rt_mem;
  iftmp.163_111 = _69->offset;

;;    succ:       64 [100.0%]  (FALLTHRU,EXECUTABLE)

;;   basic block 64, loop depth 0, count 0, freq 623, maybe hot
;;    prev block 63, next block 65, flags: (NEW, REACHABLE)
;;    pred:       63 [100.0%]  (FALLTHRU,EXECUTABLE)
  # iftmp.163_8 = PHI <iftmp.163_111(63)>
  # _241 = PHI <_186(63)>
  _112 = iftmp.163_8->u.hwint[0];
  _113 = _241 + _112;
  _114 = _113 * 8;
  _115 = ref_28(D)->size;
  if (_114 == _115)
    goto <bb 83>;
  else
    goto <bb 65>;

We soon realize that we can cprop away the two PHI nodes at the start of BB64 and merge BB64 and BB63 together resulting in:


;;   basic block 63, loop depth 0, count 0, freq 529, maybe hot
;;    prev block 62, next block 64, flags: (NEW, REACHABLE)
;;    pred:       62 [27.0%]  (TRUE_VALUE,EXECUTABLE)
  iftmp.161_102 = _99->size;
  _186 = iftmp.161_102->u.hwint[0];
  _69 = mem_25(D)->u.fld[1].rt_mem;
  iftmp.163_111 = _69->offset;
  _112 = iftmp.163_111->u.hwint[0];
  _113 = _112 + _186;
  _114 = _113 * 8;
  _115 = ref_28(D)->size;
  if (_114 == _115)
    goto <bb 81>;
  else
    goto <bb 64>;



Later we discover some CSE opportunities and remove dead code resulting in:

;;   basic block 66, loop depth 0, count 0, freq 529, maybe hot
;;    prev block 65, next block 67, flags: (NEW, REACHABLE)
;;    pred:       65 [27.0%]  (TRUE_VALUE,EXECUTABLE)
  _186 = _91->u.hwint[0];
  _113 = _166 + _186;
  _114 = _113 * 8;
  _115 = ref_28(D)->size;
  if (_114 == _115)
    goto <bb 67>;
  else
    goto <bb 68>;



Needless to say, this is goodness.








Jeff




Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]