Fix Undefined-shift in CPDF_SampledFunc::v_Init().

core/fpdfapi/fpdf_page/fpdf_page_func.cpp

 // Copyright 2014 PDFium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
 #include "core/fpdfapi/fpdf_page/pageint.h"
 
 #include <limits.h>
 
@@ skipping 458 matching lines @@
           m_Stack[m_StackCount - ii - 1] = last;
         }
       }
       break;
     }
     default:
       break;
   }
   return TRUE;
 }
-static FX_FLOAT PDF_Interpolate(FX_FLOAT x,
-                                FX_FLOAT xmin,
-                                FX_FLOAT xmax,
-                                FX_FLOAT ymin,
-                                FX_FLOAT ymax) {
-  return ((x - xmin) * (ymax - ymin) / (xmax - xmin)) + ymin;
+
+// See PDF Reference 1.7, page 170, table 3.36.
+bool IsValidBitsPerSample(uint32_t x) {
+  switch (x) {
+    case 1:
+    case 2:
+    case 4:
+    case 8:
+    case 12:
+    case 16:
+    case 24:
+    case 32:
+      return true;
+    default:
+      return false;
+  }
 }
-static uint32_t _GetBits32(const uint8_t* pData, int bitpos, int nbits) {
+
+// See PDF Reference 1.7, page 170.
+FX_FLOAT PDF_Interpolate(FX_FLOAT x,
+                         FX_FLOAT xmin,
+                         FX_FLOAT xmax,
+                         FX_FLOAT ymin,
+                         FX_FLOAT ymax) {
+  FX_FLOAT divisor = xmax - xmin;
+  return ymin + (divisor ? (x - xmin) * (ymax - ymin) / divisor : 0);
+}
+
+uint32_t GetBits32(const uint8_t* pData, int bitpos, int nbits) {

[Tom Sepez, 2016/05/19 16:53:40]

This has got to be mind-numbingly slow, consider making a better version.  I
think Cary has coded/tested the same thing more tightly in
https://codereview.chromium.org/1870463002 , please consolidate into fxcrt/
somewhere.

[Lei Zhang, 2016/05/19 21:16:19]

Ack. Will merge with the fx_skia_device.cpp copy in another CL.

   int result = 0;
-  for (int i = 0; i < nbits; i++)
-    if (pData[(bitpos + i) / 8] & (1 << (7 - (bitpos + i) % 8))) {
+  for (int i = 0; i < nbits; i++) {
+    if (pData[(bitpos + i) / 8] & (1 << (7 - (bitpos + i) % 8)))
       result |= 1 << (nbits - i - 1);
-    }
+  }
   return result;
 }
 
 class CPDF_PSFunc : public CPDF_Function {
  public:
-  CPDF_PSFunc() : CPDF_Function(Type::kType4PostScript) {}
+  CPDF_PSFunc() {}
+  ~CPDF_PSFunc() override {}
+
   // CPDF_Function
   FX_BOOL v_Init(CPDF_Object* pObj) override;
   FX_BOOL v_Call(FX_FLOAT* inputs, FX_FLOAT* results) const override;
 
  private:
   CPDF_PSEngine m_PS;
 };
 
 FX_BOOL CPDF_PSFunc::v_Init(CPDF_Object* pObj) {
   CPDF_StreamAcc acc;
@@ skipping 10 matching lines @@
   PS.Execute();
   if (PS.GetStackSize() < m_nOutputs)
     return FALSE;
   for (uint32_t i = 0; i < m_nOutputs; i++)
     results[m_nOutputs - i - 1] = PS.Pop();
   return TRUE;
 }
 
 }  // namespace
 
-CPDF_SampledFunc::CPDF_SampledFunc() : CPDF_Function(Type::kType0Sampled) {
-  m_pSampleStream = NULL;
-  m_pEncodeInfo = NULL;
-  m_pDecodeInfo = NULL;
-}
+CPDF_SampledFunc::CPDF_SampledFunc() {}
 
-CPDF_SampledFunc::~CPDF_SampledFunc() {
-  delete m_pSampleStream;
-  FX_Free(m_pEncodeInfo);
-  FX_Free(m_pDecodeInfo);
-}
+CPDF_SampledFunc::~CPDF_SampledFunc() {}
 
 FX_BOOL CPDF_SampledFunc::v_Init(CPDF_Object* pObj) {
   CPDF_Stream* pStream = pObj->AsStream();
   if (!pStream)
     return false;
 
   CPDF_Dictionary* pDict = pStream->GetDict();
   CPDF_Array* pSize = pDict->GetArrayBy("Size");
   CPDF_Array* pEncode = pDict->GetArrayBy("Encode");
   CPDF_Array* pDecode = pDict->GetArrayBy("Decode");
   m_nBitsPerSample = pDict->GetIntegerBy("BitsPerSample");
-  if (m_nBitsPerSample > 32) {
+  if (!IsValidBitsPerSample(m_nBitsPerSample))
     return FALSE;
-  }
-  m_SampleMax = 0xffffffff >> (32 - m_nBitsPerSample);
-  m_pSampleStream = new CPDF_StreamAcc;
+
+  m_SampleMax =
+      0xffffffff >> (m_nBitsPerSample == 32 ? 0 : 32 - m_nBitsPerSample);

[Tom Sepez, 2016/05/19 16:58:48]

Not groking why we need the ? operator, isn't 32 - 32 == 0  ?  Did you mean >=
32?

[Lei Zhang, 2016/05/19 21:16:19]

Uh, I'm being silly. It was the m_nBitsPerSample == 0 case that I was worried
about, but now that does not even reach here.

+  m_pSampleStream.reset(new CPDF_StreamAcc);
   m_pSampleStream->LoadAllData(pStream, FALSE);
-  m_pEncodeInfo = FX_Alloc(SampleEncodeInfo, m_nInputs);
   FX_SAFE_UINT32 nTotalSampleBits = 1;
+  m_pEncodeInfo.resize(m_nInputs);
   for (uint32_t i = 0; i < m_nInputs; i++) {
     m_pEncodeInfo[i].sizes = pSize ? pSize->GetIntegerAt(i) : 0;
     if (!pSize && i == 0)
       m_pEncodeInfo[i].sizes = pDict->GetIntegerBy("Size");
     nTotalSampleBits *= m_pEncodeInfo[i].sizes;
     if (pEncode) {
       m_pEncodeInfo[i].encode_min = pEncode->GetFloatAt(i * 2);
       m_pEncodeInfo[i].encode_max = pEncode->GetFloatAt(i * 2 + 1);
     } else {
       m_pEncodeInfo[i].encode_min = 0;
-      if (m_pEncodeInfo[i].sizes == 1)
-        m_pEncodeInfo[i].encode_max = 1;
-      else
-        m_pEncodeInfo[i].encode_max = (FX_FLOAT)m_pEncodeInfo[i].sizes - 1;
+      m_pEncodeInfo[i].encode_max = m_pEncodeInfo[i].sizes == 1
+                                        ? 1
+                                        : (FX_FLOAT)m_pEncodeInfo[i].sizes - 1;
     }
   }
   nTotalSampleBits *= m_nBitsPerSample;
   nTotalSampleBits *= m_nOutputs;
   FX_SAFE_UINT32 nTotalSampleBytes = nTotalSampleBits;
   nTotalSampleBytes += 7;
   nTotalSampleBytes /= 8;
   if (!nTotalSampleBytes.IsValid() || nTotalSampleBytes.ValueOrDie() == 0 ||
       nTotalSampleBytes.ValueOrDie() > m_pSampleStream->GetSize()) {
     return FALSE;
   }
-  m_pDecodeInfo = FX_Alloc(SampleDecodeInfo, m_nOutputs);
+  m_pDecodeInfo.resize(m_nOutputs);
   for (uint32_t i = 0; i < m_nOutputs; i++) {
     if (pDecode) {
       m_pDecodeInfo[i].decode_min = pDecode->GetFloatAt(2 * i);
       m_pDecodeInfo[i].decode_max = pDecode->GetFloatAt(2 * i + 1);
     } else {
       m_pDecodeInfo[i].decode_min = m_pRanges[i * 2];
       m_pDecodeInfo[i].decode_max = m_pRanges[i * 2 + 1];
     }
   }
   return TRUE;
@@ skipping 13 matching lines @@
       blocksize[i] = blocksize[i - 1] * m_pEncodeInfo[i - 1].sizes;
     encoded_input[i] = PDF_Interpolate(
         inputs[i], m_pDomains[i * 2], m_pDomains[i * 2 + 1],
         m_pEncodeInfo[i].encode_min, m_pEncodeInfo[i].encode_max);
     index[i] = std::min((uint32_t)std::max(0.f, encoded_input[i]),
                         m_pEncodeInfo[i].sizes - 1);
     pos += index[i] * blocksize[i];
   }
   FX_SAFE_INT32 bits_to_output = m_nOutputs;
   bits_to_output *= m_nBitsPerSample;
-  if (!bits_to_output.IsValid()) {
+  if (!bits_to_output.IsValid())
     return FALSE;
-  }
+
   FX_SAFE_INT32 bitpos = pos;
   bitpos *= bits_to_output.ValueOrDie();
-  if (!bitpos.IsValid()) {
+  if (!bitpos.IsValid())
     return FALSE;
-  }
+
   FX_SAFE_INT32 range_check = bitpos;
   range_check += bits_to_output.ValueOrDie();
-  if (!range_check.IsValid()) {
+  if (!range_check.IsValid())
     return FALSE;
-  }
+
   const uint8_t* pSampleData = m_pSampleStream->GetData();
-  if (!pSampleData) {
+  if (!pSampleData)
     return FALSE;
-  }
+
   for (uint32_t j = 0; j < m_nOutputs; j++) {
     uint32_t sample =
-        _GetBits32(pSampleData, bitpos.ValueOrDie() + j * m_nBitsPerSample,
-                   m_nBitsPerSample);
+        GetBits32(pSampleData, bitpos.ValueOrDie() + j * m_nBitsPerSample,
+                  m_nBitsPerSample);
     FX_FLOAT encoded = (FX_FLOAT)sample;
     for (uint32_t i = 0; i < m_nInputs; i++) {
       if (index[i] == m_pEncodeInfo[i].sizes - 1) {
         if (index[i] == 0)
           encoded = encoded_input[i] * (FX_FLOAT)sample;
       } else {
         FX_SAFE_INT32 bitpos2 = blocksize[i];
         bitpos2 += pos;
         bitpos2 *= m_nOutputs;
         bitpos2 += j;
         bitpos2 *= m_nBitsPerSample;
         if (!bitpos2.IsValid())
           return FALSE;
         uint32_t sample1 =
-            _GetBits32(pSampleData, bitpos2.ValueOrDie(), m_nBitsPerSample);
+            GetBits32(pSampleData, bitpos2.ValueOrDie(), m_nBitsPerSample);
         encoded += (encoded_input[i] - index[i]) *
                    ((FX_FLOAT)sample1 - (FX_FLOAT)sample);
       }
     }
     results[j] = PDF_Interpolate(encoded, 0, (FX_FLOAT)m_SampleMax,
                                  m_pDecodeInfo[j].decode_min,
                                  m_pDecodeInfo[j].decode_max);
   }
   return TRUE;
 }
 
-CPDF_ExpIntFunc::CPDF_ExpIntFunc()
-    : CPDF_Function(Type::kType2ExpotentialInterpolation) {
+CPDF_ExpIntFunc::CPDF_ExpIntFunc() {
   m_pBeginValues = NULL;
   m_pEndValues = NULL;
 }
 
 CPDF_ExpIntFunc::~CPDF_ExpIntFunc() {
   FX_Free(m_pBeginValues);
   FX_Free(m_pEndValues);
 }
 FX_BOOL CPDF_ExpIntFunc::v_Init(CPDF_Object* pObj) {
   CPDF_Dictionary* pDict = pObj->GetDict();
@@ skipping 26 matching lines @@
   for (uint32_t i = 0; i < m_nInputs; i++)
     for (uint32_t j = 0; j < m_nOrigOutputs; j++) {
       results[i * m_nOrigOutputs + j] =
           m_pBeginValues[j] +
           (FX_FLOAT)FXSYS_pow(inputs[i], m_Exponent) *
               (m_pEndValues[j] - m_pBeginValues[j]);
     }
   return TRUE;
 }
 
-CPDF_StitchFunc::CPDF_StitchFunc() : CPDF_Function(Type::kType3Stitching) {
+CPDF_StitchFunc::CPDF_StitchFunc() {
   m_pBounds = NULL;
   m_pEncode = NULL;
 }
 
 CPDF_StitchFunc::~CPDF_StitchFunc() {
-  for (auto& sub : m_pSubFunctions) {
-    delete sub;
-  }
   FX_Free(m_pBounds);
   FX_Free(m_pEncode);
 }
+
 FX_BOOL CPDF_StitchFunc::v_Init(CPDF_Object* pObj) {
   CPDF_Dictionary* pDict = pObj->GetDict();
   if (!pDict) {
     return FALSE;
   }
   if (m_nInputs != kRequiredNumInputs) {
     return FALSE;
   }
   CPDF_Array* pArray = pDict->GetArrayBy("Functions");
   if (!pArray) {
@@ skipping 14 matching lines @@
     // dimensionalities are the same.
     if (pFunc->CountInputs() != kRequiredNumInputs)
       return FALSE;
     if (pFunc->CountOutputs() != m_nOutputs) {
       if (m_nOutputs)
         return FALSE;
 
       m_nOutputs = pFunc->CountOutputs();
     }
 
-    m_pSubFunctions.push_back(pFunc.release());
+    m_pSubFunctions.push_back(std::move(pFunc));
   }
   m_pBounds = FX_Alloc(FX_FLOAT, nSubs + 1);
   m_pBounds[0] = m_pDomains[0];
   pArray = pDict->GetArrayBy("Bounds");
   if (!pArray)
     return FALSE;
   for (uint32_t i = 0; i < nSubs - 1; i++)
     m_pBounds[i + 1] = pArray->GetFloatAt(i);
   m_pBounds[nSubs] = m_pDomains[1];
   m_pEncode = FX_Alloc2D(FX_FLOAT, nSubs, 2);
   pArray = pDict->GetArrayBy("Encode");
   if (!pArray)
     return FALSE;
 
   for (uint32_t i = 0; i < nSubs * 2; i++)
     m_pEncode[i] = pArray->GetFloatAt(i);
   return TRUE;
 }
+
 FX_BOOL CPDF_StitchFunc::v_Call(FX_FLOAT* inputs, FX_FLOAT* outputs) const {
   FX_FLOAT input = inputs[0];
   size_t i;
   for (i = 0; i < m_pSubFunctions.size() - 1; i++) {
     if (input < m_pBounds[i + 1])
       break;
   }
-  if (!m_pSubFunctions[i]) {
-    return FALSE;
-  }
   input = PDF_Interpolate(input, m_pBounds[i], m_pBounds[i + 1],
                           m_pEncode[i * 2], m_pEncode[i * 2 + 1]);
   int nresults;
   m_pSubFunctions[i]->Call(&input, kRequiredNumInputs, outputs, nresults);
   return TRUE;
 }
 
 CPDF_Function* CPDF_Function::Load(CPDF_Object* pFuncObj) {
-  if (!pFuncObj) {
-    return NULL;
-  }
-  int type;
-  if (CPDF_Stream* pStream = pFuncObj->AsStream()) {
+  if (!pFuncObj)
+    return nullptr;
+
+  int type = -1;
+  if (CPDF_Stream* pStream = pFuncObj->AsStream())
     type = pStream->GetDict()->GetIntegerBy("FunctionType");
-  } else if (CPDF_Dictionary* pDict = pFuncObj->AsDictionary()) {
+  else if (CPDF_Dictionary* pDict = pFuncObj->AsDictionary())
     type = pDict->GetIntegerBy("FunctionType");
-  } else {
-    return NULL;
-  }
-  CPDF_Function* pFunc = NULL;
-  if (type == 0) {
-    pFunc = new CPDF_SampledFunc();
-  } else if (type == 2) {
-    pFunc = new CPDF_ExpIntFunc();
-  } else if (type == 3) {
-    pFunc = new CPDF_StitchFunc();
-  } else if (type == 4) {
-    pFunc = new CPDF_PSFunc();
-  } else {
-    return NULL;
-  }
-  if (!pFunc->Init(pFuncObj)) {
-    delete pFunc;
-    return NULL;
-  }
-  return pFunc;
+
+  std::unique_ptr<CPDF_Function> pFunc;
+  if (type == 0)
+    pFunc.reset(new CPDF_SampledFunc());
+  else if (type == 2)
+    pFunc.reset(new CPDF_ExpIntFunc());
+  else if (type == 3)
+    pFunc.reset(new CPDF_StitchFunc());
+  else if (type == 4)

[Tom Sepez, 2016/05/19 16:53:40]

Shouldn't this be kTypeFourPostScript and friends?  Maybe move the enum into the
.cpp file? Or a link to the pdf spec were it says "1" instead of something saner
so that using "1" matches the spec.

[Lei Zhang, 2016/05/19 21:16:19]

I put CPDF_Function::Type back. It's not obvious where else we are using ints to
represent this enum, so leaving it in the header for now.

+    pFunc.reset(new CPDF_PSFunc());
+
+  if (!pFunc || !pFunc->Init(pFuncObj))
+    return nullptr;
+  return pFunc.release();
 }
 
-CPDF_Function::CPDF_Function(Type type) : m_Type(type) {
+CPDF_Function::CPDF_Function() {
   m_pDomains = NULL;
   m_pRanges = NULL;
 }
 
 CPDF_Function::~CPDF_Function() {
   FX_Free(m_pDomains);
   FX_Free(m_pRanges);
 }
+
 FX_BOOL CPDF_Function::Init(CPDF_Object* pObj) {
   CPDF_Stream* pStream = pObj->AsStream();
   CPDF_Dictionary* pDict = pStream ? pStream->GetDict() : pObj->AsDictionary();
 
   CPDF_Array* pDomains = pDict->GetArrayBy("Domain");
   if (!pDomains)
     return FALSE;
 
   m_nInputs = pDomains->GetCount() / 2;
   if (m_nInputs == 0)
@@ skipping 16 matching lines @@
     return FALSE;
   if (m_pRanges && m_nOutputs > old_outputs) {
     m_pRanges = FX_Realloc(FX_FLOAT, m_pRanges, m_nOutputs * 2);
     if (m_pRanges) {
       FXSYS_memset(m_pRanges + (old_outputs * 2), 0,
                    sizeof(FX_FLOAT) * (m_nOutputs - old_outputs) * 2);
     }
   }
   return TRUE;
 }
+
 FX_BOOL CPDF_Function::Call(FX_FLOAT* inputs,
                             uint32_t ninputs,
                             FX_FLOAT* results,
                             int& nresults) const {
   if (m_nInputs != ninputs) {
     return FALSE;
   }
   nresults = m_nOutputs;
   for (uint32_t i = 0; i < m_nInputs; i++) {
     if (inputs[i] < m_pDomains[i * 2])
       inputs[i] = m_pDomains[i * 2];
     else if (inputs[i] > m_pDomains[i * 2 + 1])
       inputs[i] = m_pDomains[i * 2] + 1;
   }
   v_Call(inputs, results);
   if (m_pRanges) {
     for (uint32_t i = 0; i < m_nOutputs; i++) {
       if (results[i] < m_pRanges[i * 2])
         results[i] = m_pRanges[i * 2];
       else if (results[i] > m_pRanges[i * 2 + 1])
         results[i] = m_pRanges[i * 2 + 1];
     }
   }
   return TRUE;
 }