Uploaded image for project: 'Qt'
  1. Qt
  2. QTBUG-35979

V4 handles array lengths improperly

    XMLWordPrintable

    Details

    • Commits:
      7e115c5c5e954ab560afccbd04cb295232a0924c (qtdeclarative)

      Description

      When popping elements from the end of an array, subsequent attempts to lengthen the array by assigning to an element beyond the end do not work as expected. See attached file for a test case--the output from this script is:

      Initial Test Array value: 1,2,2
      Test Array after adding a value to index 2: 1,2,2,3,3,3
      Test Array after popping: 1,2,2
      Test Array after assigning new value to popped index (2): 1,2,2
      Test Array after adding new value to index after popped index (3): 1,2,2,4,4,4,4,5,5,5,5,5

      In the fourth line of output, the expected output would be "1,2,2,4,4,4,4", but instead it is just "1,2,2".

      I've traced the execution down into the V4 code, and I believe the problem comes from the fact that ArrayPrototype::method_pop() does not decrement Object::arrayDataLen. When a subsequent assignment is made to the array index of the previously-popped element, the function QV4::__qmljs_set_element() tries to call Object::propertyIndexFromArrayIndex() to determine whether the index is a valid one or not, and this function examines arrayDataLen. Since the pop call decrements the length property but not arrayDataLen, the propertyIndexFromArrayIndex() function will mistakenly return a valid index, and __qmljs_set_element() will assign directly to the array slot, instead of calling Object::putIndexed(), which would lengthen the array.

      If this is correct, then I think either the arrayDataLen member needs to be kept in better sync with the size of the array in all of the various places which can shorten it (splice() looks like it would cause the same problem), or else propertyIndexFromArrayIndex() should call arrayLength() instead of examining arrayDataLen to determine whether an index is valid or not. I'm not sure which of those two solutions is the correct one.

        Attachments

        No reviews matched the request. Check your Options in the drop-down menu of this sections header.

          Activity

            People

            Assignee:
            shausman Simon Hausmann
            Reporter:
            fischerm Matt Fischer
            Votes:
            1 Vote for this issue
            Watchers:
            2 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved:

                Gerrit Reviews

                There are no open Gerrit changes