Menu

#258 sdcc - bad C++ STL code in SDCCralloc.hpp

None
closed-fixed
None
5
2016-01-03
2016-01-03
No

Hello SDCC team,

After investigating a crash (in win32 debug mode) i found this:

Some of the code asumes its ok to call begin() and end() for an empty var object.

This is not completely true i think.

http://www.cplusplus.com/reference/set/set/begin/

This describes begin() with empty list as: If the container is empty, the returned iterator value shall not be dereferenced.

https://msdn.microsoft.com/en-us/library/ey2tx4e0.aspx

This describes begin() with empty list as: A bidirectional iterator addressing the first element in the set or the location succeeding an empty set.

Sounds like the spec describes it as invalid, and Microsoft describes it as return the next location which in this case would be something pseudo-random (depending on other factors).

This patch fixes it up by checking for empty lists explicitly. This fixes the win32 crashes.

Thanks,

/pedro

1 Attachments

Discussion

  • Philipp Klaus Krause

    But by §23.2.1 of the C++ standard, "begin() returns an iterator referring to the first element in the container. end() returns an iterator which is the past-the-end value for the container. If the container is empty, then begin() == end();"

    And we have the test

    if (i == i_end)
      return(true);
    if (ai == ai_end)
      return(false);
    

    So if a set is empty, we never dereference the iterator in the current code.

    Philipp

     

    Last edit: Philipp Klaus Krause 2016-01-03
  • Peter Dons Tychsen

    Hi SDCC/Phillip,

    Ah, yes. Going back i see that the MS version does the same. The real culprit is not what i thought.
    Docs say that "If the set is empty, then set::end() == set::begin()."

    I was fooled by a messed up debug session.

    The real culprit is simply when both lists are empty the function should return "false", as they are equal. MSVC runtime checks that "A < B" and "B < A" are mutually exclusive in debug mode.

    I have made a new patch that fixes the problem.

    Thanks for spotting my incorrect conclusion.

    Thanks,

    /pedro

     
  • Philipp Klaus Krause

    • assigned_to: Philipp Klaus Krause
    • Group: -->
     
  • Philipp Klaus Krause

    • status: open --> closed-fixed
     
  • Philipp Klaus Krause

    Fixed in revision #9440. Thanks for looking into this issue, which for me as a non-Windows user was hard to Debug.
    The change results in different register assignments. On my system, for the regression tests, code size slightly increases for z80, slightly decreases for hc08, s08, stm8.

    Philipp

     

Log in to post a comment.