Disabled the -fvisibility=hidden flag for tcmalloc....

Disabled the -fvisibility=hidden flag for tcmalloc.
This is necessary to fix the crashes of binaries linked with tcmalloc.

[Mark Mentovai, 2009-10-29 13:48:42]

Yes, this is OK.

[Evan Martin, 2009-10-29 15:48:25]

We want to be able to build with both tcmalloc and visbility=hidden.  I don't
think this solves the problem.

This is ok if you just want to use tcmalloc for testing purposes, but if you
intend to eventually turn on tcmalloc always I think this is not the proper fix.

[Mark Mentovai, 2009-10-29 15:51:16]

This change only restores default visibility in the tcmalloc library.

Personally, I'm OK with it, although the better fix would be to mark
the symbols that need default visibility explicitly in the source.  We
could take a local patch temporarily, but you'd ideally coordinate
this with the tcmalloc developers upstream.

Mark

Evan wrote:
> We want to be able to build with both tcmalloc and visbility=3Dhidden. =
=A0I
> don't
> think this solves the problem.
>
> This is ok if you just want to use tcmalloc for testing purposes, but if =
you
> intend to eventually turn on tcmalloc always I think this is not the prop=
er
> fix.
>
> http://codereview.chromium.org/343037

[willchan no longer on Chromium, 2009-10-29 15:58:57]

Agreed with Mark.  Note that we should do a local patch for now because it'll
take awhile for another google-perftools release since a new version already
just came out recently.

On 2009/10/29 15:51:16, Mark Mentovai wrote:
> This change only restores default visibility in the tcmalloc library.
> 
> Personally, I'm OK with it, although the better fix would be to mark
> the symbols that need default visibility explicitly in the source.  We
> could take a local patch temporarily, but you'd ideally coordinate
> this with the tcmalloc developers upstream.
> 
> Mark
> 
> Evan wrote:
> > We want to be able to build with both tcmalloc and visbility=3Dhidden. =
> =A0I
> > don't
> > think this solves the problem.
> >
> > This is ok if you just want to use tcmalloc for testing purposes, but if =
> you
> > intend to eventually turn on tcmalloc always I think this is not the prop=
> er
> > fix.
> >
> > http://codereview.chromium.org/343037

[not_the_right_glider, 2009-10-29 17:14:08]

On 2009/10/29 15:51:16, Mark Mentovai wrote:
> This change only restores default visibility in the tcmalloc library.
> 
> Personally, I'm OK with it, although the better fix would be to mark
> the symbols that need default visibility explicitly in the source.  We
> could take a local patch temporarily, but you'd ideally coordinate
> this with the tcmalloc developers upstream.
I'm already doing it. In fact, their answer is more like "no, we don't want to
protect against the non-standard flags".
I guess we'd better keep this patch until we work out a better solution.
> 
> Mark
> 
> Evan wrote:
> > We want to be able to build with both tcmalloc and visbility=3Dhidden. =
> =A0I
> > don't
> > think this solves the problem.
> >
> > This is ok if you just want to use tcmalloc for testing purposes, but if =
> you
> > intend to eventually turn on tcmalloc always I think this is not the prop=
> er
> > fix.
> >
> > http://codereview.chromium.org/343037

[not_the_right_glider, 2009-10-29 17:17:16]

On 2009/10/29 15:58:57, willchan wrote:
> Agreed with Mark.  Note that we should do a local patch for now because it'll
> take awhile for another google-perftools release since a new version already
> just came out recently.
It actually is possible to push changes to perftools (r77 is finally here). But
those should be discussed and justified.
> 
> On 2009/10/29 15:51:16, Mark Mentovai wrote:
> > This change only restores default visibility in the tcmalloc library.
> > 
> > Personally, I'm OK with it, although the better fix would be to mark
> > the symbols that need default visibility explicitly in the source.  We
> > could take a local patch temporarily, but you'd ideally coordinate
> > this with the tcmalloc developers upstream.
> > 
> > Mark
> > 
> > Evan wrote:
> > > We want to be able to build with both tcmalloc and visbility=3Dhidden. =
> > =A0I
> > > don't
> > > think this solves the problem.
> > >
> > > This is ok if you just want to use tcmalloc for testing purposes, but if =
> > you
> > > intend to eventually turn on tcmalloc always I think this is not the prop=
> > er
> > > fix.
> > >
> > > http://codereview.chromium.org/343037

[not_the_right_glider, 2009-10-29 18:06:49]

Gentlemen,
is it ok to submit the CL assuming we'll find a better solution soon?
Note that currently the tcmalloc support is broken at all for Chromium, and this
CL fixes that.
On 2009/10/29 17:17:16, Alexander Potapenko wrote:
> On 2009/10/29 15:58:57, willchan wrote:
> > Agreed with Mark.  Note that we should do a local patch for now because
it'll
> > take awhile for another google-perftools release since a new version already
> > just came out recently.
> It actually is possible to push changes to perftools (r77 is finally here).
But
> those should be discussed and justified.
> > 
> > On 2009/10/29 15:51:16, Mark Mentovai wrote:
> > > This change only restores default visibility in the tcmalloc library.
> > > 
> > > Personally, I'm OK with it, although the better fix would be to mark
> > > the symbols that need default visibility explicitly in the source.  We
> > > could take a local patch temporarily, but you'd ideally coordinate
> > > this with the tcmalloc developers upstream.
> > > 
> > > Mark
> > > 
> > > Evan wrote:
> > > > We want to be able to build with both tcmalloc and visbility=3Dhidden. =
> > > =A0I
> > > > don't
> > > > think this solves the problem.
> > > >
> > > > This is ok if you just want to use tcmalloc for testing purposes, but if
=
> > > you
> > > > intend to eventually turn on tcmalloc always I think this is not the
prop=
> > > er
> > > > fix.
> > > >
> > > > http://codereview.chromium.org/343037

[willchan no longer on Chromium, 2009-10-29 18:13:03]

LGTM for the reasons you state.  Do you have commit access or does someone need
to submit for you?

On 2009/10/29 18:06:49, Alexander Potapenko wrote:
> Gentlemen,
> is it ok to submit the CL assuming we'll find a better solution soon?
> Note that currently the tcmalloc support is broken at all for Chromium, and
this
> CL fixes that.
> On 2009/10/29 17:17:16, Alexander Potapenko wrote:
> > On 2009/10/29 15:58:57, willchan wrote:
> > > Agreed with Mark.  Note that we should do a local patch for now because
> it'll
> > > take awhile for another google-perftools release since a new version
already
> > > just came out recently.
> > It actually is possible to push changes to perftools (r77 is finally here).
> But
> > those should be discussed and justified.
> > > 
> > > On 2009/10/29 15:51:16, Mark Mentovai wrote:
> > > > This change only restores default visibility in the tcmalloc library.
> > > > 
> > > > Personally, I'm OK with it, although the better fix would be to mark
> > > > the symbols that need default visibility explicitly in the source.  We
> > > > could take a local patch temporarily, but you'd ideally coordinate
> > > > this with the tcmalloc developers upstream.
> > > > 
> > > > Mark
> > > > 
> > > > Evan wrote:
> > > > > We want to be able to build with both tcmalloc and visbility=3Dhidden.
=
> > > > =A0I
> > > > > don't
> > > > > think this solves the problem.
> > > > >
> > > > > This is ok if you just want to use tcmalloc for testing purposes, but
if
> =
> > > > you
> > > > > intend to eventually turn on tcmalloc always I think this is not the
> prop=
> > > > er
> > > > > fix.
> > > > >
> > > > > http://codereview.chromium.org/343037

[Timur Iskhodzhanov, 2009-10-29 18:19:07]

I'll do that
On 2009/10/29 18:13:03, willchan wrote:
> LGTM for the reasons you state.  Do you have commit access or does someone
need
> to submit for you?
> 
> On 2009/10/29 18:06:49, Alexander Potapenko wrote:
> > Gentlemen,
> > is it ok to submit the CL assuming we'll find a better solution soon?
> > Note that currently the tcmalloc support is broken at all for Chromium, and
> this
> > CL fixes that.
> > On 2009/10/29 17:17:16, Alexander Potapenko wrote:
> > > On 2009/10/29 15:58:57, willchan wrote:
> > > > Agreed with Mark.  Note that we should do a local patch for now because
> > it'll
> > > > take awhile for another google-perftools release since a new version
> already
> > > > just came out recently.
> > > It actually is possible to push changes to perftools (r77 is finally
here).
> > But
> > > those should be discussed and justified.
> > > > 
> > > > On 2009/10/29 15:51:16, Mark Mentovai wrote:
> > > > > This change only restores default visibility in the tcmalloc library.
> > > > > 
> > > > > Personally, I'm OK with it, although the better fix would be to mark
> > > > > the symbols that need default visibility explicitly in the source.  We
> > > > > could take a local patch temporarily, but you'd ideally coordinate
> > > > > this with the tcmalloc developers upstream.
> > > > > 
> > > > > Mark
> > > > > 
> > > > > Evan wrote:
> > > > > > We want to be able to build with both tcmalloc and
visbility=3Dhidden.
> =
> > > > > =A0I
> > > > > > don't
> > > > > > think this solves the problem.
> > > > > >
> > > > > > This is ok if you just want to use tcmalloc for testing purposes,
but
> if
> > =
> > > > > you
> > > > > > intend to eventually turn on tcmalloc always I think this is not the
> > prop=
> > > > > er
> > > > > > fix.
> > > > > >
> > > > > > http://codereview.chromium.org/343037

[not_the_right_glider, 2009-10-29 18:19:12]

On 2009/10/29 18:13:03, willchan wrote:
> LGTM for the reasons you state.  Do you have commit access or does someone
need
> to submit for you?
Not yet.

Timur, could you submit this CL, please?
> 
> On 2009/10/29 18:06:49, Alexander Potapenko wrote:
> > Gentlemen,
> > is it ok to submit the CL assuming we'll find a better solution soon?
> > Note that currently the tcmalloc support is broken at all for Chromium, and
> this
> > CL fixes that.
> > On 2009/10/29 17:17:16, Alexander Potapenko wrote:
> > > On 2009/10/29 15:58:57, willchan wrote:
> > > > Agreed with Mark.  Note that we should do a local patch for now because
> > it'll
> > > > take awhile for another google-perftools release since a new version
> already
> > > > just came out recently.
> > > It actually is possible to push changes to perftools (r77 is finally
here).
> > But
> > > those should be discussed and justified.
> > > > 
> > > > On 2009/10/29 15:51:16, Mark Mentovai wrote:
> > > > > This change only restores default visibility in the tcmalloc library.
> > > > > 
> > > > > Personally, I'm OK with it, although the better fix would be to mark
> > > > > the symbols that need default visibility explicitly in the source.  We
> > > > > could take a local patch temporarily, but you'd ideally coordinate
> > > > > this with the tcmalloc developers upstream.
> > > > > 
> > > > > Mark
> > > > > 
> > > > > Evan wrote:
> > > > > > We want to be able to build with both tcmalloc and
visbility=3Dhidden.
> =
> > > > > =A0I
> > > > > > don't
> > > > > > think this solves the problem.
> > > > > >
> > > > > > This is ok if you just want to use tcmalloc for testing purposes,
but
> if
> > =
> > > > > you
> > > > > > intend to eventually turn on tcmalloc always I think this is not the
> > prop=
> > > > > er
> > > > > > fix.
> > > > > >
> > > > > > http://codereview.chromium.org/343037

[Timur Iskhodzhanov, 2009-10-29 18:23:20]

http://codereview.chromium.org/342039 is ready for your LGTM

[Evan Martin, 2009-12-01 23:08:16]

closing this since hte other change landed