1198980 - JS::ubi::Node::identifier and JS::ubi::StackFrame::identifier should be uint64_t instead of uintptr_t

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Otherwise bad things start happening when we are inspecting an offline heap snapshot that was created on a 64bit machine from a 32bit machine.

Comment 1

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: Make JS::ubi::*::identifier be uint64_t instead of uintptr_t

Comment 2

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

This also fixes bug 1194985.

Comment 4

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=86b14f524ffe

Comment 5

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Ok, I think I figured out why there are gtest failures.

I think we need to also make JS::ubi::Node::size return uint64_t. Right now it returns (platform dependent) size_t and so it suffers the same problems that identifier and uintptr_t suffered: if we serialize a heap snapshot on a platform with a 64bit size_t and then deserialize on a platform with a 32bit size_t we don't have enough bits to represent the original size. I don't think we would ever have a single ubi::Node whose size is greater than 2**32, but whatever. FWIW, we always store the size as a uint64 inside the core dump protobuf files.

I'll have a new patch up in a bit (unfortunately, trees are closed so no try push yet).

Comment 6

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: Make JS::ubi::*::identifier be uint64_t instead of uintptr_t

Comment 7

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

New try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=408bd39e1cea

Comment 8

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Looks like we finally got those gtests passing on 32 bit platforms :)

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a45c3750a16c

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a45c3750a16c