Avoid macro redefinition in tools_sanity_unittest.cc

Avoid macro redefinition in tools_sanity_unittest.cc

This causes build failure by macro redefinition on ASan-enabled build without -w flag.

BUG=162783
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=283978

[tzik, 2014-07-14 12:25:52]

PTL

[Nico, 2014-07-16 19:02:29]

https://codereview.chromium.org/389313002/diff/1/base/tools_sanity_unittest.cc
File base/tools_sanity_unittest.cc (right):

https://codereview.chromium.org/389313002/diff/1/base/tools_sanity_unittest.c...
base/tools_sanity_unittest.cc:131: #endif // (defined(ADDRESS_SANITIZER) &&
defined(OS_IOS)) || defined(SYZYASAN)
Are you sure this endif should move to down here?

[tzik, 2014-07-17 06:05:42]

Updated!

Plus, I added a warning suppression removal for the issue.

https://codereview.chromium.org/389313002/diff/1/base/tools_sanity_unittest.cc
File base/tools_sanity_unittest.cc (right):

https://codereview.chromium.org/389313002/diff/1/base/tools_sanity_unittest.c...
base/tools_sanity_unittest.cc:131: #endif // (defined(ADDRESS_SANITIZER) &&
defined(OS_IOS)) || defined(SYZYASAN)
On 2014/07/16 19:02:29, Nico (away) wrote:
> Are you sure this endif should move to down here?

Ah, right. This should move above.
Done.

[Nico, 2014-07-17 16:08:55]

The cc file looks good to me now :-)

https://codereview.chromium.org/389313002/diff/40001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/389313002/diff/40001/build/common.gypi#newcod...
build/common.gypi:3281: ['os_posix==1 and (asan!=1 and msan!=1 and tsan!=1 and
lsan!=1 and ubsan!=1) and (OS!="linux" or clang!=1 or buildtype=="Official")', {
This used to not be in a buildtype=="Official" section, and the _FORTIFY_SOURCE
is what required the warning flag below. So I think you probably want to move
the flag you're removing below to up here.

[tzik, 2014-07-17 17:29:05]

https://codereview.chromium.org/389313002/diff/40001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/389313002/diff/40001/build/common.gypi#newcod...
build/common.gypi:3281: ['os_posix==1 and (asan!=1 and msan!=1 and tsan!=1 and
lsan!=1 and ubsan!=1) and (OS!="linux" or clang!=1 or buildtype=="Official")', {
On 2014/07/17 16:08:55, Nico (away) wrote:
> This used to not be in a buildtype=="Official" section, and the
_FORTIFY_SOURCE
> is what required the warning flag below. So I think you probably want to move
> the flag you're removing below to up here.

The -Wno-error flag was for asan==1 case, and this block seems to have asan!=1.
Shall I also update *san condition in this block to enable FORTIFY_SOURCE for
asan==1 && buildtype=="Official" case?

[Nico, 2014-07-17 17:33:45]

Oh, you're right. inferno: Do we do buildtype=Official asan builds for
clusterfuzz on linux?

[aarya, 2014-07-17 17:39:40]

On 2014/07/17 17:33:45, Nico (away) wrote:
> Oh, you're right. inferno: Do we do buildtype=Official asan builds for
> clusterfuzz on linux?

Nope, we don't have official asan on linux build.

[Nico, 2014-07-17 17:42:39]

lgtm

[tzik, 2014-07-18 00:52:23]

The CQ bit was checked by tzik@chromium.org

[commit-bot: I haz the power, 2014-07-18 00:53:38]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tzik@chromium.org/389313002/40001

[commit-bot: I haz the power, 2014-07-18 02:40:50]

Change committed as 283978

[Alexander Potapenko, 2014-07-18 07:04:57]

LGTM