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

Using array of size 1 as surrogate for a flexible array size in QContiguousCache

    XMLWordPrintable

    Details

    • Type: Task
    • Status: Closed
    • Priority: P2: Important
    • Resolution: Invalid
    • Affects Version/s: 6.2.0
    • Fix Version/s: None
    • Labels:
      None
    • Platform/s:
      All

      Description

      QContiguousCache uses

      template <typename T>
      struct QContiguousCacheTypedData : public QContiguousCacheData
      {
          T array[1];
      };
      

      as its Data type, to which its d member points; and dereferences d->array[…] for various indices that are not guaranteed to be 0. The allocateData() used to allocate d calls a suitable aligned malloc with space for more than 1 T and casts the resulting pointer type back to Data *, so the allocated memory is indeed large enough for such dereferencing. However, this isn't C, it's C++, in which (IIUC) this is UB; the compiler is allowed to optimise under the assumption that the overt size of the array in its declaration is its real size.
      See QTBUG-19199 for UB in a similar context (cured in Qt 6; I noticed this issue while searching for any other instances of the same pattern).
      This can potentially cause random crashes in code using QContiguousCache.
      Of course, fixing this would be SiC – and we missed the 6.0 deadline; but perhaps we can include it as an exceptional SC-break before 6.2 ?

      Fortunately QContiguousCache does not appear to be heavily used.
      (There is one other place we use an array[1], in qtbase/tests/benchmarks/corelib/tools/qvector/qrawvector.h, but it's test code and the type appears to be used only to determine alignment).

        Attachments

          Issue Links

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

            Activity

              People

              Assignee:
              ievgenii.meshcheriakov Ievgenii Meshcheriakov
              Reporter:
              Eddy Edward Welbourne
              PM Owner:
              Vladimir Minenko Vladimir Minenko
              RnD Owner:
              Alex Blasche Alex Blasche
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Gerrit Reviews

                  There are no open Gerrit changes