allocate vector explicitly

* when appending a position, the old version would call
  appendToAllocatedVector() even in cases where no vector had
  already been allocated.  That routine then accessed a union
  to get old size, accessing some random data that would
  mostly be 0 by accident.

2 files changed, 43 insertions, 19 deletions

diff --git a/searchlib/src/vespa/searchlib/fef/termfieldmatchdata.cpp b/searchlib/src/vespa/searchlib/fef/termfieldmatchdata.cpp
index 7ff5dcf8b6c..34502a68ddc 100644
--- a/searchlib/src/vespa/searchlib/fef/termfieldmatchdata.cpp
+++ b/searchlib/src/vespa/searchlib/fef/termfieldmatchdata.cpp
@@ -40,7 +40,7 @@ TermFieldMatchData & TermFieldMatchData::operator = (const TermFieldMatchData &
 TermFieldMatchData::~TermFieldMatchData()
 {
     if (isRawScore()) {
-    } else if (isMultiPos()) {
+    } else if (allocated()) {
         delete [] _data._positions._positions;
     } else {
         getFixed()->~TermFieldMatchDataPosition();
@@ -79,28 +79,38 @@ constexpr size_t MAX_ELEMS =  std::numeric_limits<uint16_t>::max();
 void
 TermFieldMatchData::resizePositionVector(size_t sz)
 {
-    size_t newSize(std::min(MAX_ELEMS, std::max(1ul, sz)));
+    assert(allocated());
+    assert(_sz >= 2);
+    size_t newSize(std::min(MAX_ELEMS, sz));
     TermFieldMatchDataPosition * n = new TermFieldMatchDataPosition[newSize];
-    if (_sz > 0) {
-        if (isMultiPos()) {
-            for (size_t i(0); i < _data._positions._allocated; i++) {
-                n[i] = _data._positions._positions[i];
-            }
-            delete [] _data._positions._positions;
-        } else {
-            assert(_sz == 1);
-            n[0] = *getFixed();
-            _data._positions._maxElementLength = getFixed()->getElementLen();
-        }
+    for (size_t i(0); i < _data._positions._allocated; i++) {
+        n[i] = _data._positions._positions[i];
     }
-    _fieldId = _fieldId | 0x4000;
+    delete [] _data._positions._positions;
     _data._positions._allocated = newSize;
     _data._positions._positions = n;
 }
 
 void
+TermFieldMatchData::allocateVectorAndAppend(const TermFieldMatchDataPosition &pos)
+{
+    assert(_sz == 1);
+    assert(!allocated());
+    size_t newSize = 2;
+    TermFieldMatchDataPosition * n = new TermFieldMatchDataPosition[newSize];
+    n[0] = *getFixed();
+    n[1] = pos;
+    _data._positions._maxElementLength = std::max(getFixed()->getElementLen(), pos.getElementLen());
+    _data._positions._allocated = newSize;
+    _data._positions._positions = n;
+    _fieldId = _fieldId | 0x4000; // set allocated() flag
+    _sz++;
+}
+
+void
 TermFieldMatchData::appendPositionToAllocatedVector(const TermFieldMatchDataPosition &pos)
 {
+    assert(allocated());
     if (__builtin_expect(_sz >= _data._positions._allocated, false)) {
         resizePositionVector(_sz*2);
     }
@@ -112,4 +122,4 @@ TermFieldMatchData::appendPositionToAllocatedVector(const TermFieldMatchDataPosi
     }
 }
 
-}
+} // namespace
diff --git a/searchlib/src/vespa/searchlib/fef/termfieldmatchdata.h b/searchlib/src/vespa/searchlib/fef/termfieldmatchdata.h
index 018af889557..11d1c22b61c 100644
--- a/searchlib/src/vespa/searchlib/fef/termfieldmatchdata.h
+++ b/searchlib/src/vespa/searchlib/fef/termfieldmatchdata.h
@@ -47,6 +47,7 @@ private:
     int32_t  getElementWeight() const { return empty() ? 1 : allocated() ? getMultiple()->getElementWeight() : getFixed()->getElementWeight(); }
     uint32_t getMaxElementLength() const { return empty() ? 0 : allocated() ? _data._positions._maxElementLength : getFixed()->getElementLen(); }
     void appendPositionToAllocatedVector(const TermFieldMatchDataPosition &pos);
+    void allocateVectorAndAppend(const TermFieldMatchDataPosition &pos);
     void resizePositionVector(size_t sz) __attribute__((noinline));
 
     enum { FIELDID_MASK = 0x1fff};
@@ -66,7 +67,18 @@ public:
     size_t capacity() const { return allocated() ? _data._positions._allocated : 1; }
     void reservePositions(size_t sz) {
         if (sz > capacity()) {
-            resizePositionVector(sz);
+            if (allocated()) {
+                resizePositionVector(sz);
+            } else {
+                TermFieldMatchDataPosition * n = new TermFieldMatchDataPosition[sz];
+                if (_sz == 1) {
+                    n[0] = *getFixed();
+                    _data._positions._maxElementLength = n[0].getElementLen();
+                }
+                _data._positions._allocated = sz;
+                _data._positions._positions = n;
+                _fieldId = _fieldId | 0x4000; // set allocated() flag
+            }
         }
     }
 
@@ -224,11 +236,13 @@ public:
      * @param pos low-level occurrence information
      **/
     TermFieldMatchData &appendPosition(const TermFieldMatchDataPosition &pos) {
-        if (isMultiPos() || (_sz > 0)) {
-            appendPositionToAllocatedVector(pos);
-        } else {
+        if (_sz == 0 && !allocated()) {
             _sz = 1;
             new (_data._position) TermFieldMatchDataPosition(pos);
+        } else if (allocated()) {
+            appendPositionToAllocatedVector(pos);
+        } else {
+            allocateVectorAndAppend(pos);
         }
         return *this;
     }