NaCl cleanup: Remove now-unneeded initialisation of NaCl libraries

NaCl cleanup: Remove now-unneeded initialisation of NaCl libraries

The renderer now no longer uses these NaCl libraries, so we don't need
to initialise them on startup in module_ppapi.cc.

This leaves UrandomFD() unused, so we can remove it.

BUG= https://bugs.chromium.org/p/nativeclient/issues/detail?id=2832
TEST=e.g. NaClBrowserTestPnacl.PPAPICore (tests PNaCl translation)

Committed: https://crrev.com/bba02708e8a255c5f9e2898debf68ed0b037b38f
Cr-Commit-Position: refs/heads/master@{#372098}

[Mark Seaborn, 2016-01-27 23:28:08]

The CQ bit was checked by mseaborn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-01-27 23:28:51]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1646733002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1646733002/1

[commit-bot: I haz the power, 2016-01-28 00:17:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-28 00:17:56]

Dry run: This issue passed the CQ dry run.

[Mark Seaborn, 2016-01-28 02:46:48]

Description was changed from

==========
NaCl cleanup: Remove now-unneeded initialisation of NaCl libraries
==========

to

==========
NaCl cleanup: Remove now-unneeded initialisation of NaCl libraries

The renderer now no longer uses these NaCl libraries, so we don't need
to initialise them on startup in module_ppapi.cc.

This leaves UrandomFD() unused, so we can remove it.

BUG= https://bugs.chromium.org/p/nativeclient/issues/detail?id=2832
TEST=e.g. NaClBrowserTestPnacl.PPAPICore (tests PNaCl translation)
==========

[Mark Seaborn, 2016-01-28 02:46:48]

mseaborn@chromium.org changed reviewers:
+ bbudge@chromium.org

[bbudge, 2016-01-28 13:26:12]

lgtm

[Mark Seaborn, 2016-01-28 16:21:10]

The CQ bit was checked by mseaborn@chromium.org

[commit-bot: I haz the power, 2016-01-28 16:22:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1646733002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1646733002/1

[commit-bot: I haz the power, 2016-01-28 16:28:21]

Description was changed from

==========
NaCl cleanup: Remove now-unneeded initialisation of NaCl libraries

The renderer now no longer uses these NaCl libraries, so we don't need
to initialise them on startup in module_ppapi.cc.

This leaves UrandomFD() unused, so we can remove it.

BUG= https://bugs.chromium.org/p/nativeclient/issues/detail?id=2832
TEST=e.g. NaClBrowserTestPnacl.PPAPICore (tests PNaCl translation)
==========

to

==========
NaCl cleanup: Remove now-unneeded initialisation of NaCl libraries

The renderer now no longer uses these NaCl libraries, so we don't need
to initialise them on startup in module_ppapi.cc.

This leaves UrandomFD() unused, so we can remove it.

BUG= https://bugs.chromium.org/p/nativeclient/issues/detail?id=2832
TEST=e.g. NaClBrowserTestPnacl.PPAPICore (tests PNaCl translation)
==========

[commit-bot: I haz the power, 2016-01-28 16:28:22]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-01-28 16:30:13]

Description was changed from

==========
NaCl cleanup: Remove now-unneeded initialisation of NaCl libraries

The renderer now no longer uses these NaCl libraries, so we don't need
to initialise them on startup in module_ppapi.cc.

This leaves UrandomFD() unused, so we can remove it.

BUG= https://bugs.chromium.org/p/nativeclient/issues/detail?id=2832
TEST=e.g. NaClBrowserTestPnacl.PPAPICore (tests PNaCl translation)
==========

to

==========
NaCl cleanup: Remove now-unneeded initialisation of NaCl libraries

The renderer now no longer uses these NaCl libraries, so we don't need
to initialise them on startup in module_ppapi.cc.

This leaves UrandomFD() unused, so we can remove it.

BUG= https://bugs.chromium.org/p/nativeclient/issues/detail?id=2832
TEST=e.g. NaClBrowserTestPnacl.PPAPICore (tests PNaCl translation)

Committed: https://crrev.com/bba02708e8a255c5f9e2898debf68ed0b037b38f
Cr-Commit-Position: refs/heads/master@{#372098}
==========

[commit-bot: I haz the power, 2016-01-28 16:30:14]

Patchset 1 (id:??) landed as
https://crrev.com/bba02708e8a255c5f9e2898debf68ed0b037b38f
Cr-Commit-Position: refs/heads/master@{#372098}

[fserb, 2016-01-28 18:27:57]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/1646903002/ by fserb@chromium.org.

The reason for reverting is: Possibly causing linking failure on main chrome
waterfall.  Reverting...
.

[Mark Seaborn, 2016-01-28 18:34:22]

On 28 January 2016 at 10:27, <fserb@chromium.org> wrote:

> A revert of this CL (patchset #1 id:1) has been created
inhttps://codereview.chromium.org/1646903002/ by fserb@chromium.org.
>
> The reason for reverting is: Possibly causing linking failure on main chrome
> waterfall.  Reverting...
>
> Please provide more information.  What was the failure you saw?

Mark

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

[Mark Seaborn, 2016-01-28 18:50:02]

On 28 January 2016 at 10:34, Mark Seaborn <mseaborn@chromium.org> wrote:

> On 28 January 2016 at 10:27, <fserb@chromium.org> wrote:
>
>> A revert of this CL (patchset #1 id:1) has been created
inhttps://codereview.chromium.org/1646903002/ by fserb@chromium.org.
>>
>> The reason for reverting is: Possibly causing linking failure on main chrome
>> waterfall.  Reverting...
>>
>> Please provide more information.  What was the failure you saw?
>

Are you referring to this failure? --
https://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Linux...

The failure there was:

[20052/20056] ACTION linux_packages_beta_rpm:
rpm_packages_beta_83861dcaa45ceadab6d1e8f993e5d3f3
FAILED: cd ../../chrome; flock -- /tmp/linux_package_lock bash
../out/Release/installer/rpm/build.sh -o../out/Release -b ../out/Release -a
ia32 -d google_chrome -c unstable
Staging common install files in '/tmp/rpm.build.kaA1I5'...
Staging RPM install files in '/tmp/rpm.build.kaA1I5'...
Packaging i386...
3d2
< ld-linux.so.2(GLIBC_2.3)

ERROR: Shared library dependencies changed!
If this is intentional, please update:
chrome/installer/linux/rpm/expected_deps_i386
chrome/installer/linux/rpm/expected_deps_x86_64


It looks like this means that "ld-linux.so.2" disappeared as a dependency.

Mark

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

[chromium-reviews, 2016-01-28 18:54:24]

yep, that was the failure we are tracking.
this was the only CL that had deps changes from the list.
But we have no idea how this disappeared.


On Thu, Jan 28, 2016 at 1:50 PM Mark Seaborn <mseaborn@chromium.org> wrote:

> On 28 January 2016 at 10:34, Mark Seaborn <mseaborn@chromium.org> wrote:
>
>> On 28 January 2016 at 10:27, <fserb@chromium.org> wrote:
>>
>>> A revert of this CL (patchset #1 id:1) has been created
inhttps://codereview.chromium.org/1646903002/ by fserb@chromium.org.
>>>
>>> The reason for reverting is: Possibly causing linking failure on main
chrome
>>> waterfall.  Reverting...
>>>
>>> Please provide more information.  What was the failure you saw?
>>
>
> Are you referring to this failure? --
>
https://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Linux...
>
> The failure there was:
>
> [20052/20056] ACTION linux_packages_beta_rpm:
> rpm_packages_beta_83861dcaa45ceadab6d1e8f993e5d3f3
> FAILED: cd ../../chrome; flock -- /tmp/linux_package_lock bash
> ../out/Release/installer/rpm/build.sh -o../out/Release -b ../out/Release -a
> ia32 -d google_chrome -c unstable
> Staging common install files in '/tmp/rpm.build.kaA1I5'...
> Staging RPM install files in '/tmp/rpm.build.kaA1I5'...
> Packaging i386...
> 3d2
> < ld-linux.so.2(GLIBC_2.3)
>
> ERROR: Shared library dependencies changed!
> If this is intentional, please update:
> chrome/installer/linux/rpm/expected_deps_i386
> chrome/installer/linux/rpm/expected_deps_x86_64
>
>
> It looks like this means that "ld-linux.so.2" disappeared as a dependency.
>
> Mark
>
>

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

[chromium-reviews, 2016-01-28 19:05:50]

It seems this CL was the culprit indeed.
The build passed fine afterwards. I'm about to reopen the tree.

When you send this again, please remove the glibc dependency that gets
dropped with you change (a similar CL:
https://chromium.googlesource.com/chromium/src/+/387a930fde9d1e9b1b8bcbc79f89...
)

You probably repro the error and check the fix sending to the chrome linux
build bot that does the RPM.


On Thu, Jan 28, 2016 at 1:54 PM Fernando Serboncini <fserb@google.com>
wrote:

> yep, that was the failure we are tracking.
> this was the only CL that had deps changes from the list.
> But we have no idea how this disappeared.
>
>
> On Thu, Jan 28, 2016 at 1:50 PM Mark Seaborn <mseaborn@chromium.org>
> wrote:
>
>> On 28 January 2016 at 10:34, Mark Seaborn <mseaborn@chromium.org> wrote:
>>
>>> On 28 January 2016 at 10:27, <fserb@chromium.org> wrote:
>>>
>>>> A revert of this CL (patchset #1 id:1) has been created
inhttps://codereview.chromium.org/1646903002/ by fserb@chromium.org.
>>>>
>>>> The reason for reverting is: Possibly causing linking failure on main
chrome
>>>> waterfall.  Reverting...
>>>>
>>>> Please provide more information.  What was the failure you saw?
>>>
>>
>> Are you referring to this failure? --
>>
https://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Linux...
>>
>> The failure there was:
>>
>> [20052/20056] ACTION linux_packages_beta_rpm:
>> rpm_packages_beta_83861dcaa45ceadab6d1e8f993e5d3f3
>> FAILED: cd ../../chrome; flock -- /tmp/linux_package_lock bash
>> ../out/Release/installer/rpm/build.sh -o../out/Release -b ../out/Release -a
>> ia32 -d google_chrome -c unstable
>> Staging common install files in '/tmp/rpm.build.kaA1I5'...
>> Staging RPM install files in '/tmp/rpm.build.kaA1I5'...
>> Packaging i386...
>> 3d2
>> < ld-linux.so.2(GLIBC_2.3)
>>
>> ERROR: Shared library dependencies changed!
>> If this is intentional, please update:
>> chrome/installer/linux/rpm/expected_deps_i386
>> chrome/installer/linux/rpm/expected_deps_x86_64
>>
>>
>> It looks like this means that "ld-linux.so.2" disappeared as a dependency.
>>
>> Mark
>>
>>

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

[Mark Seaborn, 2016-01-28 19:24:16]

OK, thanks for confirming.

The build with the revert did indeed just succeed:
https://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Linux...

I looked into why the chrome executable gets a DT_NEEDED dependency on
ld-linux.so.

In a normal release build, it's because chrome references the symbol
"__libc_stack_end" which is defined in ld-linux.so:
$ nm -D out/Release/chrome | grep __libc_stack_end
                 U __libc_stack_end

The reference is from
third_party/WebKit/Source/platform/heap/StackFrameDepth.cpp, which contains:
    return __libc_stack_end;

If I replace that with "return 0", chrome's reference to __libc_stack_end
and its dependency on ld-linux.so both go away.

It's odd that the NaCl change would influence this part of WebKit.

Cheers,
Mark

On 28 January 2016 at 11:05, Fernando Serboncini <fserb@google.com> wrote:

> It seems this CL was the culprit indeed.
> The build passed fine afterwards. I'm about to reopen the tree.
>
> When you send this again, please remove the glibc dependency that gets
> dropped with you change (a similar CL:
>
https://chromium.googlesource.com/chromium/src/+/387a930fde9d1e9b1b8bcbc79f89...
> )
>
> You probably repro the error and check the fix sending to the chrome linux
> build bot that does the RPM.
>
>
> On Thu, Jan 28, 2016 at 1:54 PM Fernando Serboncini <fserb@google.com>
> wrote:
>
>> yep, that was the failure we are tracking.
>> this was the only CL that had deps changes from the list.
>> But we have no idea how this disappeared.
>>
>>
>> On Thu, Jan 28, 2016 at 1:50 PM Mark Seaborn <mseaborn@chromium.org>
>> wrote:
>>
>>> On 28 January 2016 at 10:34, Mark Seaborn <mseaborn@chromium.org> wrote:
>>>
>>>> On 28 January 2016 at 10:27, <fserb@chromium.org> wrote:
>>>>
>>>>> A revert of this CL (patchset #1 id:1) has been created
inhttps://codereview.chromium.org/1646903002/ by fserb@chromium.org.
>>>>>
>>>>> The reason for reverting is: Possibly causing linking failure on main
chrome
>>>>> waterfall.  Reverting...
>>>>>
>>>>> Please provide more information.  What was the failure you saw?
>>>>
>>>
>>> Are you referring to this failure? --
>>>
https://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Linux...
>>>
>>> The failure there was:
>>>
>>> [20052/20056] ACTION linux_packages_beta_rpm:
>>> rpm_packages_beta_83861dcaa45ceadab6d1e8f993e5d3f3
>>> FAILED: cd ../../chrome; flock -- /tmp/linux_package_lock bash
>>> ../out/Release/installer/rpm/build.sh -o../out/Release -b ../out/Release -a
>>> ia32 -d google_chrome -c unstable
>>> Staging common install files in '/tmp/rpm.build.kaA1I5'...
>>> Staging RPM install files in '/tmp/rpm.build.kaA1I5'...
>>> Packaging i386...
>>> 3d2
>>> < ld-linux.so.2(GLIBC_2.3)
>>>
>>> ERROR: Shared library dependencies changed!
>>> If this is intentional, please update:
>>> chrome/installer/linux/rpm/expected_deps_i386
>>> chrome/installer/linux/rpm/expected_deps_x86_64
>>>
>>>
>>> It looks like this means that "ld-linux.so.2" disappeared as a
>>> dependency.
>>>
>>> Mark
>>>
>>>

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

[Mark Seaborn, 2016-01-29 20:15:43]

Ah, I see what happened.  The "GLIBC_2.3" symbol version dependency came
from __tls_get_addr().  Explained in the reland here:
https://codereview.chromium.org/1651583002/

Mark

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

[chromium-reviews, 2016-01-29 21:21:11]

cool, thanks.

On Fri, Jan 29, 2016 at 3:15 PM Mark Seaborn <mseaborn@chromium.org> wrote:

> Ah, I see what happened.  The "GLIBC_2.3" symbol version dependency came
> from __tls_get_addr().  Explained in the reland here:
> https://codereview.chromium.org/1651583002/
>
> Mark
>
>

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