Fix fuzzer build on Mac for ICU

Fix fuzzer build on Mac for ICU

Make icu depend on //buildtools/third_party/libc++abi
on mac for sanitizer builds.

BUG=636127
TEST=Libfuzzer Upload Mac ASan builds
R=mmoroz@chromium.org

Committed: https://chromium.googlesource.com/chromium/deps/icu/+/2341038bf72869a5683a893a2b319a48ffec7f62

[jungshik at Google, 2016-08-16 20:24:42]

jshin@chromium.org changed reviewers:
+ mmoroz@chromium.org

[jungshik at Google, 2016-08-16 20:24:43]

This is just what you talked about in the bug ( + condition). I'm not sure if
it's ok or even works.

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

https://codereview.chromium.org/2247953005/diff/1/BUILD.gn#newcode961
BUILD.gn:961: if (is_mac && (is_asan || is_lsan || is_msan || is_tsan)) {
I guess this is not a right condition to check?

[jungshik at Google, 2016-08-16 20:25:04]

Description was changed from

==========
ix fuzzer build on Mac for ICU

Make icu depend on //buildtools/third_party/libc++abi
on mac for sanitizer builds.

BUG=636127
TEST=Libfuzzer Upload Mac ASan builds
==========

to

==========
Fix fuzzer build on Mac for ICU

Make icu depend on //buildtools/third_party/libc++abi
on mac for sanitizer builds.

BUG=636127
TEST=Libfuzzer Upload Mac ASan builds
==========

[jungshik at Google, 2016-08-16 21:01:14]

On 2016/08/16 20:24:43, jungshik at google wrote:
> This is just what you talked about in the bug ( + condition). I'm not sure if
> it's ok or even works.
> 
> https://codereview.chromium.org/2247953005/diff/1/BUILD.gn
> File BUILD.gn (right):
> 
> https://codereview.chromium.org/2247953005/diff/1/BUILD.gn#newcode961
> BUILD.gn:961: if (is_mac && (is_asan || is_lsan || is_msan || is_tsan)) {
> I guess this is not a right condition to check?

Is there a way to tell that it's for fuzzing?

[inferno, 2016-08-16 23:16:41]

On 2016/08/16 21:01:14, jungshik at google wrote:
> On 2016/08/16 20:24:43, jungshik at google wrote:
> > This is just what you talked about in the bug ( + condition). I'm not sure
if
> > it's ok or even works.
> > 
> > https://codereview.chromium.org/2247953005/diff/1/BUILD.gn
> > File BUILD.gn (right):
> > 
> > https://codereview.chromium.org/2247953005/diff/1/BUILD.gn#newcode961
> > BUILD.gn:961: if (is_mac && (is_asan || is_lsan || is_msan || is_tsan)) {
> > I guess this is not a right condition to check?
> 
> Is there a way to tell that it's for fuzzing?

Can you just do use_sanitizer flag and remove Mac specific co edition. Why not
all platforms?

[jungshik at Google, 2016-08-16 23:34:09]

On 2016/08/16 23:16:41, inferno wrote:
> On 2016/08/16 21:01:14, jungshik at google wrote:
> > On 2016/08/16 20:24:43, jungshik at google wrote:
> > > This is just what you talked about in the bug ( + condition). I'm not sure
> if
> > > it's ok or even works.
> > > 
> > > https://codereview.chromium.org/2247953005/diff/1/BUILD.gn
> > > File BUILD.gn (right):
> > > 
> > > https://codereview.chromium.org/2247953005/diff/1/BUILD.gn#newcode961
> > > BUILD.gn:961: if (is_mac && (is_asan || is_lsan || is_msan || is_tsan)) {
> > > I guess this is not a right condition to check?
> > 
> > Is there a way to tell that it's for fuzzing?
> 
> Can you just do use_sanitizer flag and remove Mac specific co edition. Why not
> all platforms?

Ok. I'll use 'use_sanitizer'. I thought only Mac has this problem (bug 636127).
Isn't that the case?

[mmoroz, 2016-08-17 09:10:41]

On 2016/08/16 23:34:09, jungshik at google wrote:
> On 2016/08/16 23:16:41, inferno wrote:
> > On 2016/08/16 21:01:14, jungshik at google wrote:
> > > On 2016/08/16 20:24:43, jungshik at google wrote:
> > > > This is just what you talked about in the bug ( + condition). I'm not
sure
> > if
> > > > it's ok or even works.
> > > > 
> > > > https://codereview.chromium.org/2247953005/diff/1/BUILD.gn
> > > > File BUILD.gn (right):
> > > > 
> > > > https://codereview.chromium.org/2247953005/diff/1/BUILD.gn#newcode961
> > > > BUILD.gn:961: if (is_mac && (is_asan || is_lsan || is_msan || is_tsan))
{
> > > > I guess this is not a right condition to check?
> > > 
> > > Is there a way to tell that it's for fuzzing?
> > 
> > Can you just do use_sanitizer flag and remove Mac specific co edition. Why
not
> > all platforms?
> 
> Ok. I'll use 'use_sanitizer'. I thought only Mac has this problem (bug
636127).
> Isn't that the case?

I think this is fine to have Mac-specific condition, since we have problems only
on Mac. Looks like libc++abi is statically linked into libc++
(https://cs.chromium.org/chromium/src/buildtools/third_party/libc%2B%2B/BUILD....),
but also there are some Mac-specific flags, so something doesn't work at the
moment.

[mmoroz, 2016-08-17 09:11:18]

LGTM

[jungshik at Google, 2016-08-17 16:35:00]

Description was changed from

==========
Fix fuzzer build on Mac for ICU

Make icu depend on //buildtools/third_party/libc++abi
on mac for sanitizer builds.

BUG=636127
TEST=Libfuzzer Upload Mac ASan builds
==========

to

==========
Fix fuzzer build on Mac for ICU

Make icu depend on //buildtools/third_party/libc++abi
on mac for sanitizer builds.

BUG=636127
TEST=Libfuzzer Upload Mac ASan builds
R=mmoroz@chromium.org

Committed:
https://chromium.googlesource.com/chromium/deps/icu/+/2341038bf72869a5683a893...
==========

[jungshik at Google, 2016-08-17 16:35:01]

Committed patchset #3 (id:40001) manually as
2341038bf72869a5683a893a2b319a48ffec7f62 (presubmit successful).

[Nico, 2017-06-01 21:48:38]

thakis@chromium.org changed reviewers:
+ thakis@chromium.org

[Nico, 2017-06-01 21:48:39]

This doesn't seem right, we use system libc++ instead of the one in third_party
on Mac.

[mmoroz, 2017-06-02 15:00:02]

On 2017/06/01 21:48:39, Nico (vacation Jun 3-11) wrote:
> This doesn't seem right, we use system libc++ instead of the one in
third_party
> on Mac.

The build on mac is broken:
https://build.chromium.org/p/chromium.fyi/builders/Libfuzzer%20Upload%20Mac%2...

But I guess that might be caused by something else...

[chromium-reviews, 2017-06-02 15:16:29]

On Fri, Jun 2, 2017 at 11:00 AM, <mmoroz@chromium.org> wrote:

> On 2017/06/01 21:48:39, Nico (vacation Jun 3-11) wrote:
> > This doesn't seem right, we use system libc++ instead of the one in
> third_party
> > on Mac.
>
> The build on mac is broken:
> https://build.chromium.org/p/chromium.fyi/builders/
> Libfuzzer%20Upload%20Mac%20ASan?numbuilds=200
>
> But I guess that might be caused by something else..
>

I bet that's due to https://codereview.chromium.org/2914653002 , please
file a bug for that. (But this bug is likely _caused_ by the incorrect
dependency added in this cl.)


>
> https://codereview.chromium.org/2247953005/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.