Add a GN flag to allow sanitizer builds with full debugging symbols.

Add a GN flag to allow sanitizer builds with full debugging symbols.

R=thakis@chromium.org
BUG=692620
Review-Url: https://codereview.chromium.org/2843773003
Cr-Commit-Position: refs/heads/master@{#467178}
Committed: https://chromium.googlesource.com/chromium/src/+/2a7ed44b0ffb4f4e8f70dd5c0c82ad0046d84964

[Oliver Chang, 2017-04-25 22:32:50]

Description was changed from

==========
Add a GN flag to allow sanitizer builds with full debugging symbols.

R=thakis@chromium.org,inferno@chromium.org
BUG=692620
==========

to

==========
Add a GN flag to allow sanitizer builds with full debugging symbols.

R=thakis@chromium.org,mmoroz@chromium.org
BUG=692620
==========

[Oliver Chang, 2017-04-25 22:32:50]

ochang@chromium.org changed reviewers:
+ mmoroz@chromium.org
- inferno@chromium.org

[Oliver Chang, 2017-04-25 22:47:11]

Description was changed from

==========
Add a GN flag to allow sanitizer builds with full debugging symbols.

R=thakis@chromium.org,mmoroz@chromium.org
BUG=692620
==========

to

==========
Add a GN flag to allow sanitizer builds with full debugging symbols.

R=thakis@chromium.org
BUG=692620
==========

[Oliver Chang, 2017-04-25 22:47:11]

ochang@chromium.org changed reviewers:
- mmoroz@chromium.org

[Oliver Chang, 2017-04-25 22:47:59]

PTAL

[Nico, 2017-04-25 22:50:34]

Is this really something people use frequently? For one-offs, one can just
uncomment that line in .gn locally, right?

If we really need this, please call the variable sanitizer_full_symbols and set
it to false by default (else minimium_symbols=false sounds like double negation,
which is confusing)

[Oliver Chang, 2017-04-25 23:01:44]

On 2017/04/25 22:50:34, Nico wrote:
> Is this really something people use frequently? For one-offs, one can just
> uncomment that line in .gn locally, right?

We've had a number of people (who fix ClusterFuzz bugs) request this.

> 
> If we really need this, please call the variable sanitizer_full_symbols and
set
> it to false by default (else minimium_symbols=false sounds like double
negation,
> which is confusing)

The intention of this flag is to have an option to not override whatever symbol
level the user has chosen. I initially went with something along the lines of
"sanitizer_no_override_symbol_level = false", which was ugly :)

I'm fine with |sanitizer_full_symbols|, but it implies we'll always set "-g2" in
that case. Does that seem better to you?

[Nico, 2017-04-25 23:02:45]

sanitizer_keep_symbols?

On Tue, Apr 25, 2017 at 7:01 PM, <ochang@chromium.org> wrote:

> On 2017/04/25 22:50:34, Nico wrote:
> > Is this really something people use frequently? For one-offs, one can
> just
> > uncomment that line in .gn locally, right?
>
> We've had a number of people (who fix ClusterFuzz bugs) request this.
>
> >
> > If we really need this, please call the variable sanitizer_full_symbols
> and
> set
> > it to false by default (else minimium_symbols=false sounds like double
> negation,
> > which is confusing)
>
> The intention of this flag is to have an option to not override whatever
> symbol
> level the user has chosen. I initially went with something along the lines
> of
> "sanitizer_no_override_symbol_level = false", which was ugly :)
>
> I'm fine with |sanitizer_full_symbols|, but it implies we'll always set
> "-g2" in
> that case. Does that seem better to you?
>
> https://codereview.chromium.org/2843773003/
>

-- 
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.

[Oliver Chang, 2017-04-25 23:12:38]

On 2017/04/25 23:02:45, Nico wrote:
> sanitizer_keep_symbols?

Thanks, renamed!
> 
> On Tue, Apr 25, 2017 at 7:01 PM, <mailto:ochang@chromium.org> wrote:
> 
> > On 2017/04/25 22:50:34, Nico wrote:
> > > Is this really something people use frequently? For one-offs, one can
> > just
> > > uncomment that line in .gn locally, right?
> >
> > We've had a number of people (who fix ClusterFuzz bugs) request this.
> >
> > >
> > > If we really need this, please call the variable sanitizer_full_symbols
> > and
> > set
> > > it to false by default (else minimium_symbols=false sounds like double
> > negation,
> > > which is confusing)
> >
> > The intention of this flag is to have an option to not override whatever
> > symbol
> > level the user has chosen. I initially went with something along the lines
> > of
> > "sanitizer_no_override_symbol_level = false", which was ugly :)
> >
> > I'm fine with |sanitizer_full_symbols|, but it implies we'll always set
> > "-g2" in
> > that case. Does that seem better to you?
> >
> > https://codereview.chromium.org/2843773003/
> >
> 
> -- 
> 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 mailto:chromium-reviews+unsubscribe@chromium.org.

[Nico, 2017-04-25 23:15:12]

lgtm :-/

[Oliver Chang, 2017-04-25 23:17:32]

The CQ bit was checked by ochang@chromium.org

[commit-bot: I haz the power, 2017-04-25 23:18:16]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Oliver Chang, 2017-04-25 23:21:06]

On 2017/04/25 23:15:12, Nico wrote:
> lgtm :-/

thanks, why the ":-/" ? :-/

[Nico, 2017-04-25 23:24:27]

On Tue, Apr 25, 2017 at 7:21 PM, <ochang@chromium.org> wrote:

> On 2017/04/25 23:15:12, Nico wrote:
> > lgtm :-/
>
> thanks, why the ":-/" ? :-/
>

I don't like adding setting flags. We have way too many tweakables.

-- 
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.

[commit-bot: I haz the power, 2017-04-26 00:23:15]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1493162252650900,
"parent_rev": "c874a28819750c3da98cbc8abc8ace690bfa9e1e", "commit_rev":
"2a7ed44b0ffb4f4e8f70dd5c0c82ad0046d84964"}

[commit-bot: I haz the power, 2017-04-26 00:23:45]

Description was changed from

==========
Add a GN flag to allow sanitizer builds with full debugging symbols.

R=thakis@chromium.org
BUG=692620
==========

to

==========
Add a GN flag to allow sanitizer builds with full debugging symbols.

R=thakis@chromium.org
BUG=692620

Review-Url: https://codereview.chromium.org/2843773003
Cr-Commit-Position: refs/heads/master@{#467178}
Committed:
https://chromium.googlesource.com/chromium/src/+/2a7ed44b0ffb4f4e8f70dd5c0c82...
==========

[commit-bot: I haz the power, 2017-04-26 00:23:46]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/2a7ed44b0ffb4f4e8f70dd5c0c82...