Menu

#3180 Potential endless loop in PIC16 register allocator

open
nobody
None
PIC16
5
2023-12-14
2021-02-13
No

What exactly is the intended behavior of regFindFreeNext in pic16/ralloc.c?
If the first of the two for loops reaches the end of the dRegs list before dReg equals creg, I'd say that the current implementation will keep going forever, because dReg becomes NULL at that point.
Would an additional null pointer check as in ; dReg && dReg != creg; be sufficient?

static reg_info *
regFindFreeNext(set *dRegs, reg_info *creg)
{
  reg_info *dReg;

    if(creg) {
      /* position at current register */
      for(dReg = setFirstItem(dRegs); dReg != creg; dReg = setNextItem(dRegs));
    }

    for(dReg = setNextItem(dRegs); dReg; dReg = setNextItem(dRegs)) {
      if(dReg->isFree) {
        return dReg;
      }
    }

  return NULL;
}

(This is [r12063].)

Discussion

  • Philipp Klaus Krause

    The pic Backends are currently unmaintained. On one hand, patches are welcome, on the other hand, current SDCC developers will have a hard time judging patches.
    I recommend to first get the pic16 backend to the point where "make --no-print-directory test-pic16" in sdcc/support/regression gives meaningful output (at least the first three entries of each line of output, i.e. test name, number of failed assertions, number of total assertions), to make it easier to check for regressions introduced by further changes in the pic16 backend.

     
  • Benedikt Freisen

    Now that PIC16 regression testing is working, again, I ran the regression tests with and without the modification mentioned above and I get 95 abnormal stops in both cases.
    It is therefore quite possible that there is no bug here, even though that line of code looks suspicious.

     

Log in to post a comment.