Fix GCC build

Fix GCC build

-Wno-sign-compare also for GCC.

-Wno-error=strict-overflow to ignore the following warning.

In file included from
../../third_party/pdfium/core/include/fpdfapi/cpdf_object.h:10:0,
                 from
../../third_party/pdfium/core/include/fpdfapi/cpdf_stream.h:10,
                 from
../../third_party/pdfium/core/include/fpdfapi/fpdf_resource.h:12,
                 from
../../third_party/pdfium/core/fpdfapi/fpdf_font/font_int.h:13,
                 from
../../third_party/pdfium/core/fpdfapi/fpdf_font/fpdf_font_cid.cpp:7:
../../third_party/pdfium/core/include/fxcrt/fx_string.h: In function
'CFX_ByteString {anonymous}::CMap_GetString(const CFX_ByteStringC&)':
../../third_party/pdfium/core/include/fxcrt/fx_string.h:111:19: warning:
assuming signed overflow does not occur when assuming that (X - c) > X is always
false [-Wstrict-overflow]
     if (count < 0 || count > m_Length - index) {

BUG=589724
TEST=build with GCC
R=tsepez@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/3420909bbb47d6e47d6c561cbcce06d056fdf0a3

[hashimoto, 2016-03-11 05:51:40]

hashimoto@chromium.org changed reviewers:
+ dpranke@chromium.org

[Dirk Pranke, 2016-03-11 21:21:29]

dpranke@chromium.org changed reviewers:
+ brucedawson@chromium.org, tsepez@chromium.org

[Dirk Pranke, 2016-03-11 21:21:30]

adding tsepez, since I'm not an owner in pdfium.

Also adding brucedawson, since I think he's been stumbling over similar things
w/ vs2015 ...

https://codereview.chromium.org/1785943002/diff/1/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/1785943002/diff/1/BUILD.gn#newcode68
BUILD.gn:68: if (is_posix) {
This needs to be is_posix || is_clang, since there's also a win clang.

[brucedawson, 2016-03-11 22:13:02]

I don't feel qualified to comment. I don't see any problems but ???

[hashimoto, 2016-03-14 06:11:31]

https://codereview.chromium.org/1785943002/diff/1/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/1785943002/diff/1/BUILD.gn#newcode68
BUILD.gn:68: if (is_posix) {
On 2016/03/11 21:21:30, Dirk Pranke wrote:
> This needs to be is_posix || is_clang, since there's also a win clang.

I know nothing about win clang, but isn't it supposed be compatible with MSVC
thus covered by the "if (is_win)" above?

[Tom Sepez, 2016-03-14 16:46:54]

https://codereview.chromium.org/1785943002/diff/1/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/1785943002/diff/1/BUILD.gn#newcode353
BUILD.gn:353: configs -= [ "//build/config/compiler:chromium_code" ]
Can we just fix the underlying code itself?

[Tom Sepez, 2016-03-14 17:28:55]

On 2016/03/14 16:46:54, Tom Sepez wrote:
> https://codereview.chromium.org/1785943002/diff/1/BUILD.gn
> File BUILD.gn (right):
> 
> https://codereview.chromium.org/1785943002/diff/1/BUILD.gn#newcode353
> BUILD.gn:353: configs -= [ "//build/config/compiler:chromium_code" ]
> Can we just fix the underlying code itself?

See https://codereview.chromium.org/1802553004/, which should fix your offset
outside of bounds error.  
We really want to keep -Werror, and this should be possible once this lands.
The sign-compare suppression is fine, once the appropriate conditional is
determined.

[Dirk Pranke, 2016-03-14 20:12:58]

On 2016/03/14 06:11:31, hashimoto wrote:
> https://codereview.chromium.org/1785943002/diff/1/BUILD.gn
> File BUILD.gn (right):
> 
> https://codereview.chromium.org/1785943002/diff/1/BUILD.gn#newcode68
> BUILD.gn:68: if (is_posix) {
> On 2016/03/11 21:21:30, Dirk Pranke wrote:
> > This needs to be is_posix || is_clang, since there's also a win clang.
> 
> I know nothing about win clang, but isn't it supposed be compatible with MSVC
> thus covered by the "if (is_win)" above?

There is a clang-cl that is compatible w/ MSVC, but we don't use it in our
clang-on-win builds,
we use the normal is_clang path where possible.

[hashimoto, 2016-03-15 05:41:25]

On 2016/03/14 17:28:55, Tom Sepez wrote:
> On 2016/03/14 16:46:54, Tom Sepez wrote:
> > https://codereview.chromium.org/1785943002/diff/1/BUILD.gn
> > File BUILD.gn (right):
> > 
> > https://codereview.chromium.org/1785943002/diff/1/BUILD.gn#newcode353
> > BUILD.gn:353: configs -= [ "//build/config/compiler:chromium_code" ]
> > Can we just fix the underlying code itself?
Because there was nothing wrong with the code (i.e. |key|'s emptiness is checked
before the warned code), I thought this should be handled at GN level.
> 
> See https://codereview.chromium.org/1802553004/, which should fix your offset
> outside of bounds error.  
> We really want to keep -Werror, and this should be possible once this lands.
> The sign-compare suppression is fine, once the appropriate conditional is
> determined.
Thanks for the fix.

[hashimoto, 2016-03-15 05:43:00]

A new patch set with "-Wno-error=strict-overflow" to suppress another warning in
fpdfapi (pasted below).
PTAL.

[2532/2714] CXX obj/third_party/pdfium/fpdfapi/fpdf_font_cid.o
In file included from
../../third_party/pdfium/core/include/fpdfapi/cpdf_object.h:10:0,
                 from
../../third_party/pdfium/core/include/fpdfapi/cpdf_stream.h:10,
                 from
../../third_party/pdfium/core/include/fpdfapi/fpdf_resource.h:12,
                 from
../../third_party/pdfium/core/fpdfapi/fpdf_font/font_int.h:13,
                 from
../../third_party/pdfium/core/fpdfapi/fpdf_font/fpdf_font_cid.cpp:7:
../../third_party/pdfium/core/include/fxcrt/fx_string.h: In function
'CFX_ByteString {anonymous}::CMap_GetString(const CFX_ByteStringC&)':
../../third_party/pdfium/core/include/fxcrt/fx_string.h:111:19: warning:
assuming signed overflow does not occur when assuming that (X - c) > X is always
false [-Wstrict-overflow]
     if (count < 0 || count > m_Length - index) {

https://codereview.chromium.org/1785943002/diff/1/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/1785943002/diff/1/BUILD.gn#newcode68
BUILD.gn:68: if (is_posix) {
On 2016/03/14 06:11:31, hashimoto wrote:
> On 2016/03/11 21:21:30, Dirk Pranke wrote:
> > This needs to be is_posix || is_clang, since there's also a win clang.
> 
> I know nothing about win clang, but isn't it supposed be compatible with MSVC
> thus covered by the "if (is_win)" above?

Thanks for the clarification, done.

[hashimoto, 2016-03-15 05:43:55]

Description was changed from

==========
Fix GCC build

-Wno-sign-compare also for GCC.

Make fpdfapi no_chromium_code to avoid failing on an unsuppressible
warning (pasted below).

../../third_party/pdfium/core/src/fpdfapi/fpdf_parser/cpdf_syntax_parser.cpp:
In member function 'CPDF_Object*
CPDF_SyntaxParser::GetObject(CPDF_IndirectObjectHolder*, FX_DWORD,
FX_DWORD, FX_BOOL)':
../../third_party/pdfium/core/src/fpdfapi/fpdf_parser/cpdf_syntax_parser.cpp:463:70:
warning: offset outside bounds of constant string
       CFX_ByteStringC keyNoSlash(key.c_str() + 1, key.GetLength() - 1)

BUG=589724
TEST=build with GCC
==========

to

==========
Fix GCC build

-Wno-sign-compare also for GCC.

-Wno-error=strict-overflow to ignore the following warning.

In file included from
../../third_party/pdfium/core/include/fpdfapi/cpdf_object.h:10:0,
                 from
../../third_party/pdfium/core/include/fpdfapi/cpdf_stream.h:10,
                 from
../../third_party/pdfium/core/include/fpdfapi/fpdf_resource.h:12,
                 from
../../third_party/pdfium/core/fpdfapi/fpdf_font/font_int.h:13,
                 from
../../third_party/pdfium/core/fpdfapi/fpdf_font/fpdf_font_cid.cpp:7:
../../third_party/pdfium/core/include/fxcrt/fx_string.h: In function
'CFX_ByteString {anonymous}::CMap_GetString(const CFX_ByteStringC&)':
../../third_party/pdfium/core/include/fxcrt/fx_string.h:111:19: warning:
assuming signed overflow does not occur when assuming that (X - c) > X is always
false [-Wstrict-overflow]
     if (count < 0 || count > m_Length - index) {

BUG=589724
TEST=build with GCC
==========

[Dirk Pranke, 2016-03-15 15:37:47]

I defer to tsepez on this. The BUILD.gn changes are fine if it's okay to
suppress the warnings.

[Tom Sepez, 2016-03-15 15:57:20]

On 2016/03/15 15:37:47, Dirk Pranke wrote:
> I defer to tsepez on this. The BUILD.gn changes are fine if it's okay to
> suppress the warnings.

LGTM.  This one is a case of the optimizer being too smart for its own good. 
The check is fine in most cases, but when called passing an expression involving
the length of the object, becomes redundant, but only in that case.

[hashimoto, 2016-03-16 04:31:29]

On 2016/03/15 15:57:20, Tom Sepez wrote:
> On 2016/03/15 15:37:47, Dirk Pranke wrote:
> > I defer to tsepez on this. The BUILD.gn changes are fine if it's okay to
> > suppress the warnings.
> 
> LGTM.  This one is a case of the optimizer being too smart for its own good. 
> The check is fine in most cases, but when called passing an expression
involving
> the length of the object, becomes redundant, but only in that case.

Thanks for reviewing.
It seems I'm not in the list of approved committers so I cannot land this CL
myself.
Could anyone do it on my behalf (or can I be a pdfium committer)?

BTW, "gn gen" on ToT is failing because of
https://codereview.chromium.org/1797423002/.
It claims include_dirs is an undefined variable.

[dsinclair, 2016-03-16 14:21:03]

On 2016/03/16 04:31:29, hashimoto wrote:
> On 2016/03/15 15:57:20, Tom Sepez wrote:
> > On 2016/03/15 15:37:47, Dirk Pranke wrote:
> > > I defer to tsepez on this. The BUILD.gn changes are fine if it's okay to
> > > suppress the warnings.
> > 
> > LGTM.  This one is a case of the optimizer being too smart for its own good.

> > The check is fine in most cases, but when called passing an expression
> involving
> > the length of the object, becomes redundant, but only in that case.
> 
> Thanks for reviewing.
> It seems I'm not in the list of approved committers so I cannot land this CL
> myself.
> Could anyone do it on my behalf (or can I be a pdfium committer)?
> 
> BTW, "gn gen" on ToT is failing because of
> https://codereview.chromium.org/1797423002/.
> It claims include_dirs is an undefined variable.

This should be fixed with 30410cecd0f2653556094a20c2564170be127127

[hashimoto, 2016-03-17 05:34:22]

Could anyone submit this change on my behalf?
It seems I'm not allowed to "git cl land" on pdfium.

[hashimoto, 2016-03-18 09:23:54]

tsepez@, could you land this change on behalf of me?
Seems I'm not recognized as a pdfium committer.

Thanks!

[Tom Sepez, 2016-03-18 15:57:40]

On 2016/03/18 09:23:54, hashimoto wrote:
> tsepez@, could you land this change on behalf of me?
> Seems I'm not recognized as a pdfium committer.
> 
> Thanks!

Will do.

[Tom Sepez, 2016-03-18 16:00:36]

Description was changed from

==========
Fix GCC build

-Wno-sign-compare also for GCC.

-Wno-error=strict-overflow to ignore the following warning.

In file included from
../../third_party/pdfium/core/include/fpdfapi/cpdf_object.h:10:0,
                 from
../../third_party/pdfium/core/include/fpdfapi/cpdf_stream.h:10,
                 from
../../third_party/pdfium/core/include/fpdfapi/fpdf_resource.h:12,
                 from
../../third_party/pdfium/core/fpdfapi/fpdf_font/font_int.h:13,
                 from
../../third_party/pdfium/core/fpdfapi/fpdf_font/fpdf_font_cid.cpp:7:
../../third_party/pdfium/core/include/fxcrt/fx_string.h: In function
'CFX_ByteString {anonymous}::CMap_GetString(const CFX_ByteStringC&)':
../../third_party/pdfium/core/include/fxcrt/fx_string.h:111:19: warning:
assuming signed overflow does not occur when assuming that (X - c) > X is always
false [-Wstrict-overflow]
     if (count < 0 || count > m_Length - index) {

BUG=589724
TEST=build with GCC
==========

to

==========
Fix GCC build

-Wno-sign-compare also for GCC.

-Wno-error=strict-overflow to ignore the following warning.

In file included from
../../third_party/pdfium/core/include/fpdfapi/cpdf_object.h:10:0,
                 from
../../third_party/pdfium/core/include/fpdfapi/cpdf_stream.h:10,
                 from
../../third_party/pdfium/core/include/fpdfapi/fpdf_resource.h:12,
                 from
../../third_party/pdfium/core/fpdfapi/fpdf_font/font_int.h:13,
                 from
../../third_party/pdfium/core/fpdfapi/fpdf_font/fpdf_font_cid.cpp:7:
../../third_party/pdfium/core/include/fxcrt/fx_string.h: In function
'CFX_ByteString {anonymous}::CMap_GetString(const CFX_ByteStringC&)':
../../third_party/pdfium/core/include/fxcrt/fx_string.h:111:19: warning:
assuming signed overflow does not occur when assuming that (X - c) > X is always
false [-Wstrict-overflow]
     if (count < 0 || count > m_Length - index) {

BUG=589724
TEST=build with GCC
R=tsepez@chromium.org

Committed:
https://pdfium.googlesource.com/pdfium/+/3420909bbb47d6e47d6c561cbcce06d056fd...
==========

[Tom Sepez, 2016-03-18 16:00:40]

Committed patchset #2 (id:20001) manually as
3420909bbb47d6e47d6c561cbcce06d056fdf0a3 (presubmit successful).