CLD2DynamicDataLoader calls delete instead of delete[] on array types

[GoogleCodeExporter]

Upon running some browser tests in Chrome, the following error was encountered 
when attempting to call CLD2::loadDataFromRawAddress():

memory allocation/deallocation mismatch at 0x155bb621cb20: allocated with new 
[] being deallocated with delete
Received signal 11 SEGV_MAPERR 000000000039
...
#6 0x000002b7b791 MallocBlock::CheckLocked()
#7 0x000002b7b422 MallocBlock::CheckAndClear()
#8 0x000002b7bb4a MallocBlock::Deallocate()
#9 0x000002b79109 DebugDeallocate()
#10 0x000009e02885 operator delete()
#11 0x000006ecd635 CLD2DynamicDataLoader::loadDataInternal()
#12 0x000006ecd325 CLD2DynamicDataLoader::loadDataRaw()
#13 0x000006eba963 CLD2::loadDataFromRawAddress()

I'm not sure why this wasn't caught earlier in testing. It may be a consequence 
of toolchain changes in Chromium, but the error seems valid and should be 
fixed. This was previously working without issue on both Linux and Android 
platform builds for x64 and ARM respectively.

I will review the other uses of delete to see if there are more occurrences. 
This should be a trivial fix, but blocks adoption of CLD2 dynamic mode in 
Chromium.

Original issue reported on code.google.com by andrewha...@google.com on 15 May 2014 at 4:32

[GoogleCodeExporter]

$ find . -name "*dynamic*.cc" | xargs grep delete                      
./cld2_dynamic_data_tool.cc:    delete header->tableHeaders;
./cld2_dynamic_data_tool.cc:    delete header;
./cld2_dynamic_data_loader.cc:    delete header;
./cld2_dynamic_data_loader.cc:    delete header;
./cld2_dynamic_data_loader.cc:    delete tableHeaders;
./cld2_dynamic_data_loader.cc:    delete header;
./cld2_dynamic_data_loader.cc:    delete tableHeaders;
./cld2_dynamic_data_loader.cc:  delete((*scoringTables)->unigram_compat_obj); 
// tableSummaries[0] from loadDataFile
./cld2_dynamic_data_loader.cc:  delete(*scoringTables);
./cld2_dynamic_data_loader.cc:  delete header->tableHeaders;
./cld2_dynamic_data_loader.cc:  delete header;

I think all we need to fix here is the tableHeaders deletion work.

Original comment by andrewha...@google.com on 15 May 2014 at 4:37

[GoogleCodeExporter]

This is fixed in r161:
https://code.google.com/p/cld2/source/detail?r=161

I've run all the unit tests again, and confirmed that this fixes the issue in 
the Chromium build system as well.

Original comment by andrewha...@google.com on 16 May 2014 at 10:36

• Changed state: Verified

[GoogleCodeExporter]

For posterity:

* This doesn't affect the binary file format at all. Previously generated dumps 
will still work properly.
* Because the objects pointed to by the pointer were structs instead of class 
instances, the use of delete and delete[] should be exactly equivalent (i.e., 
there is no destructor to call on each element of the array, so there is 
nothing gained by adding the [])

This is purely a compiler-happiness change.

Original comment by andrewha...@chromium.org on 16 May 2014 at 11:45

[GoogleCodeExporter]

Also for posterity, I think what happened is that Chromium's build system 
finally absorbed this change:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=29185

Original comment by andrewha...@google.com on 16 May 2014 at 11:46