XFA: merge patch from CL 729293003, use FX_ArraySize for safety

Note that the merge of this fix to XFA found six bugs. Five
were fixed in https://codereview.chromium.org/826573003 and
one was fixed in https://codereview.chromium.org/831293002.
These bugs are now impossible to compile.

Replace manual/error-prone/hard-to-verify arraysize calculations with
safe FX_ArraySize macro.

pdfium has numerous places where the number of elements in an array is
calculated with expressions like:

    sizeof(cFormats)/sizeof(FX_LPCWSTR)

This is suboptimal because it is verbose, it is easy to get wrong, and
it cannot be determined through casual inspection whether the code is
correct. It will give incorrect results if cFormats is a pointer instead
of an array and it will give incorrect results if FX_LPCWSTR is not the
type of the array elements.

The FX_WSTRC macro in fx_string.h which I fixed was particularly scary
because it would silently misbehave if passed a pointer.

The FX_ArraySize macro which I have added and started using (taken from
arraysize in v8's macros.h) is easier to use and will always give correct
results. If passed a pointer it will fail to compile.

For this change I only fixed instances of sizeof(FX_LPCWSTR). There
appear to be about 150 other places in the pdfium code that could
benefit from using FX_ArraySize.

TBR=bo_xu@foxitsoftware.com, tsepez@chromium.org

Review URL: https://codereview.chromium.org/818193004

4 files changed, 26 insertions, 10 deletions

diff --git a/core/include/fxcrt/fx_basic.h b/core/include/fxcrt/fx_basic.h
index 0c84f540e9..2a77e6e947 100644
--- a/core/include/fxcrt/fx_basic.h
+++ b/core/include/fxcrt/fx_basic.h
@@ -18,6 +18,22 @@
 #ifndef _FX_STREAM_H_
 #include "fx_stream.h"
 #endif
+
+// The FX_ArraySize(arr) macro returns the # of elements in an array arr.
+// The expression is a compile-time constant, and therefore can be
+// used in defining new arrays, for example.  If you use FX_ArraySize on
+// a pointer by mistake, you will get a compile-time error.
+//
+// One caveat is that FX_ArraySize() doesn't accept any array of an
+// anonymous type or a type defined inside a function.
+#define FX_ArraySize(array) (sizeof(ArraySizeHelper(array)))
+
+// This template function declaration is used in defining FX_ArraySize.
+// Note that the function doesn't need an implementation, as we only
+// use its type.
+template <typename T, size_t N>
+char (&ArraySizeHelper(T (&array)[N]))[N];
+
 class CFX_BinaryBuf : public CFX_Object
 {
 public:
diff --git a/core/include/fxcrt/fx_string.h b/core/include/fxcrt/fx_string.h
index d6701e3d44..91032f9d97 100644
--- a/core/include/fxcrt/fx_string.h
+++ b/core/include/fxcrt/fx_string.h
@@ -604,7 +604,7 @@ private:
     }
 };
 typedef const CFX_WideStringC&	FX_WSTR;
-#define FX_WSTRC(wstr) CFX_WideStringC(wstr, sizeof(wstr) / sizeof(FX_WCHAR) - 1)
+#define FX_WSTRC(wstr) CFX_WideStringC(wstr, FX_ArraySize(wstr) - 1)
 struct CFX_StringDataW {
 
     long		m_nRefs;
diff --git a/fpdfsdk/include/javascript/JS_Define.h b/fpdfsdk/include/javascript/JS_Define.h
index 8ead492548..6b7af41d5f 100644
--- a/fpdfsdk/include/javascript/JS_Define.h
+++ b/fpdfsdk/include/javascript/JS_Define.h
@@ -599,7 +599,7 @@ if (JS_DefineGlobalConst(pRuntime,JS_WIDESTRING(const_name),JS_NewString(pRuntim
 /* ======================================== GLOBAL ARRAYS ============================================ */
 
 #define DEFINE_GLOBAL_ARRAY(pRuntime)\
-int size = sizeof(ArrayContent) / sizeof(FX_LPCWSTR);\
+int size = FX_ArraySize(ArrayContent);\
 \
 CJS_Array array(pRuntime);\
 for (int i=0; i<size; i++) array.SetElement(i,CJS_Value(pRuntime,ArrayContent[i]));\
diff --git a/fpdfsdk/src/javascript/PublicMethods.cpp b/fpdfsdk/src/javascript/PublicMethods.cpp
index 7d652496af..3eb8a09ee7 100644
--- a/fpdfsdk/src/javascript/PublicMethods.cpp
+++ b/fpdfsdk/src/javascript/PublicMethods.cpp
@@ -1679,11 +1679,11 @@ FX_BOOL CJS_PublicMethods::AFDate_Format(OBJ_METHOD_PARAMS)
 		L"yy-mm-dd", L"mmm-yy", L"mmmm-yy", L"mmm d, yyyy", L"mmmm d, yyyy",
 		L"m/d/yy h:MM tt", L"m/d/yy HH:MM" };
 
-	ASSERT(iIndex < sizeof(cFormats)/sizeof(FX_LPCWSTR));
+	ASSERT(iIndex < FX_ArraySize(cFormats));
 
 	if (iIndex < 0)
 		iIndex = 0;
-	if (iIndex >= sizeof(cFormats)/sizeof(FX_LPCWSTR))
+	if (iIndex >= FX_ArraySize(cFormats))
 		iIndex = 0;
 	CJS_Parameters newParams;
 	CJS_Value val(isolate,cFormats[iIndex]);
@@ -1710,11 +1710,11 @@ FX_BOOL CJS_PublicMethods::AFDate_Keystroke(OBJ_METHOD_PARAMS)
 		L"yy-mm-dd", L"mmm-yy", L"mmmm-yy", L"mmm d, yyyy", L"mmmm d, yyyy",
 		L"m/d/yy h:MM tt", L"m/d/yy HH:MM" };
 
-	ASSERT(iIndex<sizeof(cFormats)/sizeof(FX_LPCWSTR));
+	ASSERT(iIndex<FX_ArraySize(cFormats));
 
 	if (iIndex < 0)
 		iIndex = 0;
-	if (iIndex >= sizeof(cFormats)/sizeof(FX_LPCWSTR))
+	if (iIndex >= FX_ArraySize(cFormats))
 		iIndex = 0;
 	CJS_Parameters newParams;
 	CJS_Value val(isolate,cFormats[iIndex]);
@@ -1738,11 +1738,11 @@ FX_BOOL CJS_PublicMethods::AFTime_Format(OBJ_METHOD_PARAMS)
 	int iIndex = params[0];
 	FX_LPCWSTR cFormats[] = {L"HH:MM", L"h:MM tt", L"HH:MM:ss", L"h:MM:ss tt"};
 
-	ASSERT(iIndex<sizeof(cFormats)/sizeof(FX_LPCWSTR));
+	ASSERT(iIndex<FX_ArraySize(cFormats));
 
 	if (iIndex < 0)
 		iIndex = 0;
-	if (iIndex >= sizeof(cFormats)/sizeof(FX_LPCWSTR))
+	if (iIndex >= FX_ArraySize(cFormats))
 		iIndex = 0;
 	CJS_Parameters newParams;
 	CJS_Value val(isolate,cFormats[iIndex]);
@@ -1764,11 +1764,11 @@ FX_BOOL CJS_PublicMethods::AFTime_Keystroke(OBJ_METHOD_PARAMS)
 	int iIndex = params[0];
 	FX_LPCWSTR cFormats[] = {L"HH:MM", L"h:MM tt", L"HH:MM:ss", L"h:MM:ss tt"};
 
-	ASSERT(iIndex<sizeof(cFormats)/sizeof(FX_LPCWSTR));
+	ASSERT(iIndex<FX_ArraySize(cFormats));
 
 	if (iIndex < 0)
 		iIndex = 0;
-	if (iIndex >= sizeof(cFormats)/sizeof(FX_LPCWSTR))
+	if (iIndex >= FX_ArraySize(cFormats))
 		iIndex = 0;
 	CJS_Parameters newParams;
 	CJS_Value val(isolate,cFormats[iIndex]);