Re: [v8-dev] Re: Heap verification: what needs to be done in MyType::MyTypeVerify()?

2016-10-24 Thread Adam Klein
After offline discussion with Hannes, and reading *Space::Verify(), it does
seem like most (if not all) VerifyPointer() calls in objects-debug.cc are
redundant (since the spaces use VerifyPointersVisitor to iterate object
bodies and verify all of them).

On Thu, Oct 13, 2016 at 5:27 PM, Adam Klein  wrote:

> Hmm, am still confused. Will pick your brain when I'm in MUC next week :)
>
> On Thu, Oct 13, 2016 at 6:11 AM, Hannes Payer  wrote:
>
>> This one you need because it verifies the content of the object fields,
>> i.e. the invariants.
>>
>> On Wed, Oct 12, 2016 at 7:01 PM Adam Klein  wrote:
>>
>>> Thanks, Hannes. Are the calls to VerifyPointer() in AccessorPairVerify
>>> also redundant?
>>>
>>> On Wed, Oct 12, 2016 at 12:50 AM, 'Hannes Payer' via v8-dev <
>>> v8-dev@googlegroups.com> wrote:
>>>
>>> Interesting observation.
>>>
>>> The recursion in Box with ObjectVerify seems indeed redundant since the
>>> referenced object will be found by the verifier later.
>>> It should be fine if we just verify basic invariants in object-debug.cc,
>>> given that there is an iterator that feeds the verifier with objects (which
>>> is the case for the heap verifier).
>>>
>>> On Wed, Oct 12, 2016 at 1:41 AM Adam Klein  wrote:
>>>
>>> +some GC folks directly
>>>
>>> On Tue, Oct 4, 2016 at 11:51 AM, Adam Klein  wrote:
>>>
>>> It seems there's some confusion (likely on my part) about what code
>>> needs to be added to objects-debug.cc when adding a new HeapObject.
>>> Consider two different Structs, AccessorPair and Box:
>>>
>>> void AccessorPair::AccessorPairVerify() {
>>>   CHECK(IsAccessorPair());
>>>   VerifyPointer(getter());
>>>   VerifyPointer(setter());
>>> }
>>>
>>> void Box::BoxVerify() {
>>>   CHECK(IsBox());
>>>   value()->ObjectVerify();
>>> }
>>>
>>> AccessorPair only calls VerifyPointer, which checks that the fields is
>>> either a Smi or is a pointer to a HeapObject that lives in the Heap.
>>> Meanwhile Box recurses into ObjectVerify on the thing it wraps. Which is
>>> correct?
>>>
>>> Reading Heap::Verify, it seems we call Verify on each space, which calls
>>> ObjectVerify() followed by iterating the body with a VerifyPointersVisitor.
>>> Which looks like it does something very similar to VerifyPointer(). So the
>>> call to VerifyPointer() in AccessorPairVerify looks redundant.
>>>
>>> What about the call to ObjectVerify() in BoxVerify? I expect we'll end
>>> up eventually calling ObjectVerify whenever the heap iteration hits
>>> Box::value() (and as per the previous paragraph, assuming we've already
>>> verified that the pointer is _in_ the heap).
>>>
>>> So do we really need anything in object-debug.cc other than specific
>>> invariants about the contents? Say, a field that ought to be one of a few
>>> object types.
>>>
>>> - Adam
>>>
>>>
>>> --
>>> --
>>> v8-dev mailing list
>>> v8-dev@googlegroups.com
>>> http://groups.google.com/group/v8-dev
>>> ---
>>> You received this message because you are subscribed to the Google
>>> Groups "v8-dev" group.
>>> To unsubscribe from this group and stop receiving emails from it, send
>>> an email to v8-dev+unsubscr...@googlegroups.com.
>>> For more options, visit https://groups.google.com/d/optout.
>>>
>>> --
>>> --
>>> v8-dev mailing list
>>> v8-dev@googlegroups.com
>>> http://groups.google.com/group/v8-dev
>>> ---
>>> You received this message because you are subscribed to the Google
>>> Groups "v8-dev" group.
>>> To unsubscribe from this group and stop receiving emails from it, send
>>> an email to v8-dev+unsubscr...@googlegroups.com.
>>> For more options, visit https://groups.google.com/d/optout.
>>>
>>>
>>>
>

-- 
-- 
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
--- 
You received this message because you are subscribed to the Google Groups 
"v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: [v8-dev] Re: Heap verification: what needs to be done in MyType::MyTypeVerify()?

2016-10-13 Thread Adam Klein
Hmm, am still confused. Will pick your brain when I'm in MUC next week :)

On Thu, Oct 13, 2016 at 6:11 AM, Hannes Payer  wrote:

> This one you need because it verifies the content of the object fields,
> i.e. the invariants.
>
> On Wed, Oct 12, 2016 at 7:01 PM Adam Klein  wrote:
>
>> Thanks, Hannes. Are the calls to VerifyPointer() in AccessorPairVerify
>> also redundant?
>>
>> On Wed, Oct 12, 2016 at 12:50 AM, 'Hannes Payer' via v8-dev <
>> v8-dev@googlegroups.com> wrote:
>>
>> Interesting observation.
>>
>> The recursion in Box with ObjectVerify seems indeed redundant since the
>> referenced object will be found by the verifier later.
>> It should be fine if we just verify basic invariants in object-debug.cc,
>> given that there is an iterator that feeds the verifier with objects (which
>> is the case for the heap verifier).
>>
>> On Wed, Oct 12, 2016 at 1:41 AM Adam Klein  wrote:
>>
>> +some GC folks directly
>>
>> On Tue, Oct 4, 2016 at 11:51 AM, Adam Klein  wrote:
>>
>> It seems there's some confusion (likely on my part) about what code needs
>> to be added to objects-debug.cc when adding a new HeapObject. Consider two
>> different Structs, AccessorPair and Box:
>>
>> void AccessorPair::AccessorPairVerify() {
>>   CHECK(IsAccessorPair());
>>   VerifyPointer(getter());
>>   VerifyPointer(setter());
>> }
>>
>> void Box::BoxVerify() {
>>   CHECK(IsBox());
>>   value()->ObjectVerify();
>> }
>>
>> AccessorPair only calls VerifyPointer, which checks that the fields is
>> either a Smi or is a pointer to a HeapObject that lives in the Heap.
>> Meanwhile Box recurses into ObjectVerify on the thing it wraps. Which is
>> correct?
>>
>> Reading Heap::Verify, it seems we call Verify on each space, which calls
>> ObjectVerify() followed by iterating the body with a VerifyPointersVisitor.
>> Which looks like it does something very similar to VerifyPointer(). So the
>> call to VerifyPointer() in AccessorPairVerify looks redundant.
>>
>> What about the call to ObjectVerify() in BoxVerify? I expect we'll end up
>> eventually calling ObjectVerify whenever the heap iteration hits
>> Box::value() (and as per the previous paragraph, assuming we've already
>> verified that the pointer is _in_ the heap).
>>
>> So do we really need anything in object-debug.cc other than specific
>> invariants about the contents? Say, a field that ought to be one of a few
>> object types.
>>
>> - Adam
>>
>>
>> --
>> --
>> v8-dev mailing list
>> v8-dev@googlegroups.com
>> http://groups.google.com/group/v8-dev
>> ---
>> You received this message because you are subscribed to the Google Groups
>> "v8-dev" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to v8-dev+unsubscr...@googlegroups.com.
>> For more options, visit https://groups.google.com/d/optout.
>>
>> --
>> --
>> v8-dev mailing list
>> v8-dev@googlegroups.com
>> http://groups.google.com/group/v8-dev
>> ---
>> You received this message because you are subscribed to the Google Groups
>> "v8-dev" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to v8-dev+unsubscr...@googlegroups.com.
>> For more options, visit https://groups.google.com/d/optout.
>>
>>
>>

-- 
-- 
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
--- 
You received this message because you are subscribed to the Google Groups 
"v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: [v8-dev] Re: Heap verification: what needs to be done in MyType::MyTypeVerify()?

2016-10-13 Thread Hannes Payer
This one you need because it verifies the content of the object fields,
i.e. the invariants.

On Wed, Oct 12, 2016 at 7:01 PM Adam Klein  wrote:

> Thanks, Hannes. Are the calls to VerifyPointer() in AccessorPairVerify
> also redundant?
>
> On Wed, Oct 12, 2016 at 12:50 AM, 'Hannes Payer' via v8-dev <
> v8-dev@googlegroups.com> wrote:
>
> Interesting observation.
>
> The recursion in Box with ObjectVerify seems indeed redundant since the
> referenced object will be found by the verifier later.
> It should be fine if we just verify basic invariants in object-debug.cc,
> given that there is an iterator that feeds the verifier with objects (which
> is the case for the heap verifier).
>
> On Wed, Oct 12, 2016 at 1:41 AM Adam Klein  wrote:
>
> +some GC folks directly
>
> On Tue, Oct 4, 2016 at 11:51 AM, Adam Klein  wrote:
>
> It seems there's some confusion (likely on my part) about what code needs
> to be added to objects-debug.cc when adding a new HeapObject. Consider two
> different Structs, AccessorPair and Box:
>
> void AccessorPair::AccessorPairVerify() {
>   CHECK(IsAccessorPair());
>   VerifyPointer(getter());
>   VerifyPointer(setter());
> }
>
> void Box::BoxVerify() {
>   CHECK(IsBox());
>   value()->ObjectVerify();
> }
>
> AccessorPair only calls VerifyPointer, which checks that the fields is
> either a Smi or is a pointer to a HeapObject that lives in the Heap.
> Meanwhile Box recurses into ObjectVerify on the thing it wraps. Which is
> correct?
>
> Reading Heap::Verify, it seems we call Verify on each space, which calls
> ObjectVerify() followed by iterating the body with a VerifyPointersVisitor.
> Which looks like it does something very similar to VerifyPointer(). So the
> call to VerifyPointer() in AccessorPairVerify looks redundant.
>
> What about the call to ObjectVerify() in BoxVerify? I expect we'll end up
> eventually calling ObjectVerify whenever the heap iteration hits
> Box::value() (and as per the previous paragraph, assuming we've already
> verified that the pointer is _in_ the heap).
>
> So do we really need anything in object-debug.cc other than specific
> invariants about the contents? Say, a field that ought to be one of a few
> object types.
>
> - Adam
>
>
> --
> --
> v8-dev mailing list
> v8-dev@googlegroups.com
> http://groups.google.com/group/v8-dev
> ---
> You received this message because you are subscribed to the Google Groups
> "v8-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to v8-dev+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>
> --
> --
> v8-dev mailing list
> v8-dev@googlegroups.com
> http://groups.google.com/group/v8-dev
> ---
> You received this message because you are subscribed to the Google Groups
> "v8-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to v8-dev+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>
>
>

-- 
-- 
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
--- 
You received this message because you are subscribed to the Google Groups 
"v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: [v8-dev] Re: Heap verification: what needs to be done in MyType::MyTypeVerify()?

2016-10-12 Thread Adam Klein
Thanks, Hannes. Are the calls to VerifyPointer() in AccessorPairVerify also
redundant?

On Wed, Oct 12, 2016 at 12:50 AM, 'Hannes Payer' via v8-dev <
v8-dev@googlegroups.com> wrote:

> Interesting observation.
>
> The recursion in Box with ObjectVerify seems indeed redundant since the
> referenced object will be found by the verifier later.
> It should be fine if we just verify basic invariants in object-debug.cc,
> given that there is an iterator that feeds the verifier with objects (which
> is the case for the heap verifier).
>
> On Wed, Oct 12, 2016 at 1:41 AM Adam Klein  wrote:
>
>> +some GC folks directly
>>
>> On Tue, Oct 4, 2016 at 11:51 AM, Adam Klein  wrote:
>>
>> It seems there's some confusion (likely on my part) about what code needs
>> to be added to objects-debug.cc when adding a new HeapObject. Consider two
>> different Structs, AccessorPair and Box:
>>
>> void AccessorPair::AccessorPairVerify() {
>>   CHECK(IsAccessorPair());
>>   VerifyPointer(getter());
>>   VerifyPointer(setter());
>> }
>>
>> void Box::BoxVerify() {
>>   CHECK(IsBox());
>>   value()->ObjectVerify();
>> }
>>
>> AccessorPair only calls VerifyPointer, which checks that the fields is
>> either a Smi or is a pointer to a HeapObject that lives in the Heap.
>> Meanwhile Box recurses into ObjectVerify on the thing it wraps. Which is
>> correct?
>>
>> Reading Heap::Verify, it seems we call Verify on each space, which calls
>> ObjectVerify() followed by iterating the body with a VerifyPointersVisitor.
>> Which looks like it does something very similar to VerifyPointer(). So the
>> call to VerifyPointer() in AccessorPairVerify looks redundant.
>>
>> What about the call to ObjectVerify() in BoxVerify? I expect we'll end up
>> eventually calling ObjectVerify whenever the heap iteration hits
>> Box::value() (and as per the previous paragraph, assuming we've already
>> verified that the pointer is _in_ the heap).
>>
>> So do we really need anything in object-debug.cc other than specific
>> invariants about the contents? Say, a field that ought to be one of a few
>> object types.
>>
>> - Adam
>>
>>
>> --
>> --
>> v8-dev mailing list
>> v8-dev@googlegroups.com
>> http://groups.google.com/group/v8-dev
>> ---
>> You received this message because you are subscribed to the Google Groups
>> "v8-dev" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to v8-dev+unsubscr...@googlegroups.com.
>> For more options, visit https://groups.google.com/d/optout.
>>
> --
> --
> v8-dev mailing list
> v8-dev@googlegroups.com
> http://groups.google.com/group/v8-dev
> ---
> You received this message because you are subscribed to the Google Groups
> "v8-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to v8-dev+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

-- 
-- 
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
--- 
You received this message because you are subscribed to the Google Groups 
"v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: [v8-dev] Re: Heap verification: what needs to be done in MyType::MyTypeVerify()?

2016-10-12 Thread 'Hannes Payer' via v8-dev
Interesting observation.

The recursion in Box with ObjectVerify seems indeed redundant since the
referenced object will be found by the verifier later.
It should be fine if we just verify basic invariants in object-debug.cc,
given that there is an iterator that feeds the verifier with objects (which
is the case for the heap verifier).

On Wed, Oct 12, 2016 at 1:41 AM Adam Klein  wrote:

> +some GC folks directly
>
> On Tue, Oct 4, 2016 at 11:51 AM, Adam Klein  wrote:
>
> It seems there's some confusion (likely on my part) about what code needs
> to be added to objects-debug.cc when adding a new HeapObject. Consider two
> different Structs, AccessorPair and Box:
>
> void AccessorPair::AccessorPairVerify() {
>   CHECK(IsAccessorPair());
>   VerifyPointer(getter());
>   VerifyPointer(setter());
> }
>
> void Box::BoxVerify() {
>   CHECK(IsBox());
>   value()->ObjectVerify();
> }
>
> AccessorPair only calls VerifyPointer, which checks that the fields is
> either a Smi or is a pointer to a HeapObject that lives in the Heap.
> Meanwhile Box recurses into ObjectVerify on the thing it wraps. Which is
> correct?
>
> Reading Heap::Verify, it seems we call Verify on each space, which calls
> ObjectVerify() followed by iterating the body with a VerifyPointersVisitor.
> Which looks like it does something very similar to VerifyPointer(). So the
> call to VerifyPointer() in AccessorPairVerify looks redundant.
>
> What about the call to ObjectVerify() in BoxVerify? I expect we'll end up
> eventually calling ObjectVerify whenever the heap iteration hits
> Box::value() (and as per the previous paragraph, assuming we've already
> verified that the pointer is _in_ the heap).
>
> So do we really need anything in object-debug.cc other than specific
> invariants about the contents? Say, a field that ought to be one of a few
> object types.
>
> - Adam
>
>
> --
> --
> v8-dev mailing list
> v8-dev@googlegroups.com
> http://groups.google.com/group/v8-dev
> ---
> You received this message because you are subscribed to the Google Groups
> "v8-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to v8-dev+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

-- 
-- 
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
--- 
You received this message because you are subscribed to the Google Groups 
"v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.