View Issue Details Jump to Notes ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0014997VTK(No Category)public2014-09-16 07:532014-10-05 18:43
ReporterAndrei Terechko 
Assigned ToBen Boeckel (Kitware) 
PrioritynormalSeverityminorReproducibilityhave not tried
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version6.0.0 
Target VersionFixed in Version6.2.0 
Summary0014997: Incorrect 'num' passed to Swap*{BE,LE}Range in CommonCoreCxx-otherByteSwap test
DescriptionIn the vtkCommonCoreCxxTests-otherByteSwap test various function calls manipulate memory that hasn't been completely initialized. For example, Common/Core/Testing/Cxx/otherByteSwap.cxx contains a call to vtkByteSwap::Swap2BERange():
 
  char cword[1024];
  // ... (some other vars and tests)
  memcpy (cword, "abcdefgh", 8);
  vtkByteSwap::Swap2BERange(cword,8);

In Common/Core/vtkByteSwap.h the declaration of Swap2BERange looks like this:

  // Swap a block of 2-, 4-, or 8-byte segments for storage as Big Endian.
  static void Swap2BERange(void* p, size_t num);
  static void Swap4BERange(void* p, size_t num);
  static void Swap8BERange(void* p, size_t num);

What goes wrong is that apparently num in these functions refers to the number of segments, while the test uses it at the number of bytes. Indeed, after a few function calls the following code is invoked with template argument T equal to short:

template <class T> inline void vtkByteSwapRange(T* first, size_t num)
{
  // Swap one value at a time.
  T* last = first + num;
  for(T* p=first; p != last; ++p)
    {
    vtkByteSwapper<sizeof(T)>::Swap(reinterpret_cast<char*>(p));
    }
}

Then, variable last is assigned to first+num*sizeof(short), which means in this case that the swap operation reads from 8 uninitialized bytes of the cword char array.

An upcoming dynamic verification tool from Vector Fabrics found this error in the VTK test-suite. Below is the tool's output, including the stack trace for one of the incorrect invocations:

[M0203] Read(s) from uninitialized stack object detected:
  the read in
    function vtkByteSwapper<2ul>::Swap at /data/peter/gits/VTK-master/Common/Core/vtkByteSwap.cxx:43
    called from function vtkByteSwapRange<short> at /data/peter/gits/VTK-master/Common/Core/vtkByteSwap.cxx:75
    called from function vtkByteSwapBERange<short> at /data/peter/gits/VTK-master/Common/Core/vtkByteSwap.cxx:193
    called from function vtkByteSwap::SwapBERange at /data/peter/gits/VTK-master/Common/Core/vtkByteSwap.cxx:240
    called from function vtkByteSwap::Swap2BERange at /data/peter/gits/VTK-master/Common/Core/vtkByteSwap.cxx:298
    called from function TestByteSwap at /data/peter/gits/VTK-master/Common/Core/Testing/Cxx/otherByteSwap.cxx:57
    called from function otherByteSwap at /data/peter/gits/VTK-master/Common/Core/Testing/Cxx/otherByteSwap.cxx:160
    called from function main at /data/peter/gits/VTK-master/build/Common/Core/Testing/Cxx/vtkCommonCoreCxxTests.cxx:372
    called from function _entry
  performed 1 access(es) of size 1 at an offset of 8 bytes from the start of
  the stack object of size 1024 allocated as `cword' in
    function TestByteSwap at /data/peter/gits/VTK-master/Common/Core/Testing/Cxx/otherByteSwap.cxx:32
    called from function otherByteSwap at /data/peter/gits/VTK-master/Common/Core/Testing/Cxx/otherByteSwap.cxx:160
    called from function main at /data/peter/gits/VTK-master/build/Common/Core/Testing/Cxx/vtkCommonCoreCxxTests.cxx:372
    called from function _entry

A possible patch fixing the incorrect invocations is attached.

Platform:
$ uname -a
Linux andrei 3.5.0-54-generic #81~precise1-Ubuntu SMP Tue Jul 15 04:02:22 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux

Affected versions:
release 6.1.0 and the current development branch with the latest commit a5f46aff1efca6087b12853f22551273001fc4a6 at the time of writing
Tagshackaton
ProjectTBD
Typeincorrect functionality
Attached Filespatch file icon VTK-byteswap-num-too-high-in-test-case.patch [^] (2,151 bytes) 2014-09-16 07:53 [Show Content]

 Relationships

  Notes
(0033508)
Ben Boeckel (Kitware) (developer)
2014-10-02 14:30

Patch pushed to Gerrit with minor changes to update the output strings as well. http://review.source.kitware.com/#/t/4777/ [^]
(0033588)
Cory Quammen (developer)
2014-10-05 18:43

Fixed in

commit dd542c6ebd2b52223b7518c53be21bb9372f8a79
Author: Ben Boeckel <ben.boeckel@kitware.com>
Date: Thu Oct 2 14:28:35 2014 -0400

    otherByteSwap: fix calls for byteswapping
    
    Patch submitted by Andrei Terechko. Slightly modified to match the
    updated calls with the output.
    
    Change-Id: Iecbf06e9c39b7d67310b0175d63719eba907c1f6

 Issue History
Date Modified Username Field Change
2014-09-16 07:53 Andrei Terechko New Issue
2014-09-16 07:53 Andrei Terechko File Added: VTK-byteswap-num-too-high-in-test-case.patch
2014-09-30 08:38 Bill Lorensen Tag Attached: hackaton
2014-09-30 08:39 Bill Lorensen Assigned To => Bill Lorensen
2014-09-30 08:39 Bill Lorensen Status backlog => tabled
2014-10-01 12:40 Berk Geveci Status tabled => backlog
2014-10-02 14:30 Ben Boeckel (Kitware) Assigned To Bill Lorensen => Ben Boeckel (Kitware)
2014-10-02 14:30 Ben Boeckel (Kitware) Note Added: 0033508
2014-10-02 14:30 Ben Boeckel (Kitware) Status backlog => gerrit review
2014-10-05 18:43 Cory Quammen Note Added: 0033588
2014-10-05 18:43 Cory Quammen Status gerrit review => closed
2014-10-05 18:43 Cory Quammen Resolution open => fixed
2014-10-05 18:43 Cory Quammen Fixed in Version => 6.2.0


Copyright © 2000 - 2018 MantisBT Team