Failing tests

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
22 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Failing tests

Levente Uzonyi-2
Hi,

I'd like to have less failing tests in the trunk. For this, we have to
make decisions/anwser questions.

I recently changed DecompilerTests' superclass to LongTestCase which let
us decide if we want to run these or not (when selecting all tests in Test
Runner). By default they don't run, so they don't show up in recent
reports (http://wiki.squeak.org/squeak/6148).
Should we change the default, so that LongTestCases are run when selected
in the Test Runner? (Or one could add a chechbox to the gui which would
let the user to decide if she/he wants to run long tests? Or this could be
a preference, etc.)

Mirror primitives do not exists in the current vm. Will the vm have mirror
primitives? Should we keep the tests? Removing these would "fix" 6 errors.

Building a vm with the latest VMMaker lets 6 failing test pass
(AllocationTest, BitBltTests). It also avoids crashing linux as reported before
(http://lists.squeakfoundation.org/pipermail/squeak-dev/2009-December/142074.html).
And it also yields slight performance improvement because of the
primitiveNextPut fix. Why aren't vm releases available with these fixes?

There are two "great challenges" which would let most other failing tests
pass:
- fixing traits (the issues are solved in pharo, but one has to extract
and integrate them)
- fixing the decompiler (there's an issue with temporary variable
declarations in inlined blocks)
Is anybody working on these?

If all the above is done, we will only have "minor" issues, like:
- TextMorphTest>>#testInitialize which tries to add itself to the
"Scripting" flap which doesn't exists. (should we remove that part?)
- LocaleTest>>#testIsFontAvailable which probably has invalid assumptions
about the fonts.
- MorphicToolBuilderTests>>#testGetButtonSideEffectFree


Cheers,
Levente

Reply | Threaded
Open this post in threaded view
|

Re: Failing tests

Nicolas Cellier
2009/12/14 Levente Uzonyi <[hidden email]>:

> Hi,
>
> I'd like to have less failing tests in the trunk. For this, we have to make
> decisions/anwser questions.
>
> I recently changed DecompilerTests' superclass to LongTestCase which let
> us decide if we want to run these or not (when selecting all tests in Test
> Runner). By default they don't run, so they don't show up in recent reports
> (http://wiki.squeak.org/squeak/6148).
> Should we change the default, so that LongTestCases are run when selected in
> the Test Runner? (Or one could add a chechbox to the gui which would let the
> user to decide if she/he wants to run long tests? Or this could be a
> preference, etc.)
>
> Mirror primitives do not exists in the current vm. Will the vm have mirror
> primitives? Should we keep the tests? Removing these would "fix" 6 errors.
>
As a general policy, I would say don't remove failing tests.
Declare them as known failures

Nicolas

> Building a vm with the latest VMMaker lets 6 failing test pass
> (AllocationTest, BitBltTests). It also avoids crashing linux as reported
> before
> (http://lists.squeakfoundation.org/pipermail/squeak-dev/2009-December/142074.html).
> And it also yields slight performance improvement because of the
> primitiveNextPut fix. Why aren't vm releases available with these fixes?
>
> There are two "great challenges" which would let most other failing tests
> pass:
> - fixing traits (the issues are solved in pharo, but one has to extract and
> integrate them)
> - fixing the decompiler (there's an issue with temporary variable
> declarations in inlined blocks)
> Is anybody working on these?
>
> If all the above is done, we will only have "minor" issues, like:
> - TextMorphTest>>#testInitialize which tries to add itself to the
> "Scripting" flap which doesn't exists. (should we remove that part?)
> - LocaleTest>>#testIsFontAvailable which probably has invalid assumptions
> about the fonts.
> - MorphicToolBuilderTests>>#testGetButtonSideEffectFree
>
>
> Cheers,
> Levente
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Failing tests

Levente Uzonyi-2
On Mon, 14 Dec 2009, Nicolas Cellier wrote:

> 2009/12/14 Levente Uzonyi <[hidden email]>:
>>
>> Mirror primitives do not exists in the current vm. Will the vm have mirror
>> primitives? Should we keep the tests? Removing these would "fix" 6 errors.
>>
> As a general policy, I would say don't remove failing tests.
> Declare them as known failures
>

I agree, but if the squeakvm won't have mirror primitives, then these
tests are totally useless.


Levente

> Nicolas
>
>> Building a vm with the latest VMMaker lets 6 failing test pass
>> (AllocationTest, BitBltTests). It also avoids crashing linux as reported
>> before
>> (http://lists.squeakfoundation.org/pipermail/squeak-dev/2009-December/142074.html).
>> And it also yields slight performance improvement because of the
>> primitiveNextPut fix. Why aren't vm releases available with these fixes?
>>
>> There are two "great challenges" which would let most other failing tests
>> pass:
>> - fixing traits (the issues are solved in pharo, but one has to extract and
>> integrate them)
>> - fixing the decompiler (there's an issue with temporary variable
>> declarations in inlined blocks)
>> Is anybody working on these?
>>
>> If all the above is done, we will only have "minor" issues, like:
>> - TextMorphTest>>#testInitialize which tries to add itself to the
>> "Scripting" flap which doesn't exists. (should we remove that part?)
>> - LocaleTest>>#testIsFontAvailable which probably has invalid assumptions
>> about the fonts.
>> - MorphicToolBuilderTests>>#testGetButtonSideEffectFree
>>
>>
>> Cheers,
>> Levente
>>
>>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Failing tests

Igor Stasenko
2009/12/14 Levente Uzonyi <[hidden email]>:

> On Mon, 14 Dec 2009, Nicolas Cellier wrote:
>
>> 2009/12/14 Levente Uzonyi <[hidden email]>:
>>>
>>> Mirror primitives do not exists in the current vm. Will the vm have
>>> mirror
>>> primitives? Should we keep the tests? Removing these would "fix" 6
>>> errors.
>>>
>> As a general policy, I would say don't remove failing tests.
>> Declare them as known failures
>>
>
> I agree, but if the squeakvm won't have mirror primitives, then these tests
> are totally useless.
>

i think for such cases there should be a way to test if VM provides
new primitives, and if so, then run tests, otherwise just pass them
silently

>
> Levente
>
>> Nicolas
>>
>>> Building a vm with the latest VMMaker lets 6 failing test pass
>>> (AllocationTest, BitBltTests). It also avoids crashing linux as reported
>>> before
>>>
>>> (http://lists.squeakfoundation.org/pipermail/squeak-dev/2009-December/142074.html).
>>> And it also yields slight performance improvement because of the
>>> primitiveNextPut fix. Why aren't vm releases available with these fixes?
>>>
>>> There are two "great challenges" which would let most other failing tests
>>> pass:
>>> - fixing traits (the issues are solved in pharo, but one has to extract
>>> and
>>> integrate them)
>>> - fixing the decompiler (there's an issue with temporary variable
>>> declarations in inlined blocks)
>>> Is anybody working on these?
>>>
>>> If all the above is done, we will only have "minor" issues, like:
>>> - TextMorphTest>>#testInitialize which tries to add itself to the
>>> "Scripting" flap which doesn't exists. (should we remove that part?)
>>> - LocaleTest>>#testIsFontAvailable which probably has invalid assumptions
>>> about the fonts.
>>> - MorphicToolBuilderTests>>#testGetButtonSideEffectFree
>>>
>>>
>>> Cheers,
>>> Levente
>>>
>>>
>>
>>
>
>



--
Best regards,
Igor Stasenko AKA sig.

Reply | Threaded
Open this post in threaded view
|

Re: Failing tests

garduino
I re-run the tests to the #8475 (on windows xp) and documeted them on
the same page.

About the primitives, the suggestion of Igor sound reasonable if is
possible to implement.

About the LongTestCase, shouldn't be bad to have the option of execute
them on the TestRunner.

I will try to work a bit on some of these things when time permit.

See you.
Germán.



2009/12/14 Igor Stasenko <[hidden email]>:

> 2009/12/14 Levente Uzonyi <[hidden email]>:
>> On Mon, 14 Dec 2009, Nicolas Cellier wrote:
>>
>>> 2009/12/14 Levente Uzonyi <[hidden email]>:
>>>>
>>>> Mirror primitives do not exists in the current vm. Will the vm have
>>>> mirror
>>>> primitives? Should we keep the tests? Removing these would "fix" 6
>>>> errors.
>>>>
>>> As a general policy, I would say don't remove failing tests.
>>> Declare them as known failures
>>>
>>
>> I agree, but if the squeakvm won't have mirror primitives, then these tests
>> are totally useless.
>>
>
> i think for such cases there should be a way to test if VM provides
> new primitives, and if so, then run tests, otherwise just pass them
> silently
>>
>> Levente
>>
>>> Nicolas
>>>
>>>> Building a vm with the latest VMMaker lets 6 failing test pass
>>>> (AllocationTest, BitBltTests). It also avoids crashing linux as reported
>>>> before
>>>>
>>>> (http://lists.squeakfoundation.org/pipermail/squeak-dev/2009-December/142074.html).
>>>> And it also yields slight performance improvement because of the
>>>> primitiveNextPut fix. Why aren't vm releases available with these fixes?
>>>>
>>>> There are two "great challenges" which would let most other failing tests
>>>> pass:
>>>> - fixing traits (the issues are solved in pharo, but one has to extract
>>>> and
>>>> integrate them)
>>>> - fixing the decompiler (there's an issue with temporary variable
>>>> declarations in inlined blocks)
>>>> Is anybody working on these?
>>>>
>>>> If all the above is done, we will only have "minor" issues, like:
>>>> - TextMorphTest>>#testInitialize which tries to add itself to the
>>>> "Scripting" flap which doesn't exists. (should we remove that part?)
>>>> - LocaleTest>>#testIsFontAvailable which probably has invalid assumptions
>>>> about the fonts.
>>>> - MorphicToolBuilderTests>>#testGetButtonSideEffectFree
>>>>
>>>>
>>>> Cheers,
>>>> Levente
>>>>
>>>>
>>>
>>>
>>
>>
>
>
>
> --
> Best regards,
> Igor Stasenko AKA sig.
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Failing tests

garduino
In reply to this post by Levente Uzonyi-2
2009/12/14 Levente Uzonyi <[hidden email]>:
>
> If all the above is done, we will only have "minor" issues, like:
> - TextMorphTest>>#testInitialize which tries to add itself to the
> "Scripting" flap which doesn't exists. (should we remove that part?)

Why not register on Supplies and the test will be green?


Cheers.
Germán.

Reply | Threaded
Open this post in threaded view
|

Re: Failing tests

Levente Uzonyi-2
On Mon, 14 Dec 2009, Germán Arduino wrote:

> 2009/12/14 Levente Uzonyi <[hidden email]>:
>>
>> If all the above is done, we will only have "minor" issues, like:
>> - TextMorphTest>>#testInitialize which tries to add itself to the
>> "Scripting" flap which doesn't exists. (should we remove that part?)
>
> Why not register on Supplies and the test will be green?
>

It's already there, see the last registration in TextMorph class >>
#registerInFlapsRegistry.

One could fix this issue at least two different ways:
- remove the registrations for the scripting flap
- do nothing if someone tries to register something to a non-existsent flap

The question is: which one is correct/better? Or is there an even better
solution? (I don't use flaps at all and I don't know much about them, so I
can't decide.)


Levente

>
> Cheers.
> Germán.
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Failing tests

garduino
I mean the two registrations on the non-existent Scripting Flap:

                                        cl registerQuad: #(TextMorph exampleBackgroundLabel 'Background
Label' 'A piece of text that will occur on every card of the
background')
                                                forFlapNamed: 'Scripting'.
                                                cl registerQuad:
#(TextMorph exampleBackgroundField 'Background Field' 'A  data field
which will have a different value on every card of the background')
                                                forFlapNamed: 'Scripting'.

If you changes these two for "Supplies" the test go green. I'm not an
expert on Flaps neither but think that this change is minor and
shouldn't generate problems.

Cheers.
Germán.



2009/12/14 Levente Uzonyi <[hidden email]>:

> On Mon, 14 Dec 2009, Germán Arduino wrote:
>
>> 2009/12/14 Levente Uzonyi <[hidden email]>:
>>>
>>> If all the above is done, we will only have "minor" issues, like:
>>> - TextMorphTest>>#testInitialize which tries to add itself to the
>>> "Scripting" flap which doesn't exists. (should we remove that part?)
>>
>> Why not register on Supplies and the test will be green?
>>
>
> It's already there, see the last registration in TextMorph class >>
> #registerInFlapsRegistry.
>
> One could fix this issue at least two different ways:
> - remove the registrations for the scripting flap
> - do nothing if someone tries to register something to a non-existsent flap
>
> The question is: which one is correct/better? Or is there an even better
> solution? (I don't use flaps at all and I don't know much about them, so I
> can't decide.)
>
>
> Levente
>
>>
>> Cheers.
>> Germán.
>>
>
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Failing tests

Igor Stasenko
2009/12/14 Germán Arduino <[hidden email]>:

> I mean the two registrations on the non-existent Scripting Flap:
>
>                                        cl registerQuad: #(TextMorph            exampleBackgroundLabel  'Background
> Label' 'A piece of text that will occur on every card of the
> background')
>                                                forFlapNamed: 'Scripting'.
>                                                cl registerQuad:
> #(TextMorph             exampleBackgroundField          'Background Field'      'A  data field
> which will have a different value on every card of the background')
>                                                forFlapNamed: 'Scripting'.
>
> If you changes these two for "Supplies" the test go green. I'm not an
> expert on Flaps neither but think that this change is minor and
> shouldn't generate problems.
>

The question is, what this test trying to test. Adding stuff into flap?
Then why not create an anonymous flap instance and play with it and
then safely delete (GC) it at the end of test?

> Cheers.
> Germán.
>
>
>
> 2009/12/14 Levente Uzonyi <[hidden email]>:
>> On Mon, 14 Dec 2009, Germán Arduino wrote:
>>
>>> 2009/12/14 Levente Uzonyi <[hidden email]>:
>>>>
>>>> If all the above is done, we will only have "minor" issues, like:
>>>> - TextMorphTest>>#testInitialize which tries to add itself to the
>>>> "Scripting" flap which doesn't exists. (should we remove that part?)
>>>
>>> Why not register on Supplies and the test will be green?
>>>
>>
>> It's already there, see the last registration in TextMorph class >>
>> #registerInFlapsRegistry.
>>
>> One could fix this issue at least two different ways:
>> - remove the registrations for the scripting flap
>> - do nothing if someone tries to register something to a non-existsent flap
>>
>> The question is: which one is correct/better? Or is there an even better
>> solution? (I don't use flaps at all and I don't know much about them, so I
>> can't decide.)
>>
>>
>> Levente
>>
>>>
>>> Cheers.
>>> Germán.
>>>
>>
>>
>>
>>
>
>



--
Best regards,
Igor Stasenko AKA sig.

Reply | Threaded
Open this post in threaded view
|

Re: Failing tests

Andreas.Raab
In reply to this post by Levente Uzonyi-2
Levente Uzonyi wrote:
> I'd like to have less failing tests in the trunk. For this, we have to
> make decisions/anwser questions.

Thanks for taking the time to go through these tests carefully!

> Should we change the default, so that LongTestCases are run when
> selected in the Test Runner? (Or one could add a chechbox to the gui
> which would let the user to decide if she/he wants to run long tests? Or
> this could be a preference, etc.)

I would opt for a preference or a checkbox in the UI.

> Mirror primitives do not exists in the current vm. Will the vm have
> mirror primitives? Should we keep the tests? Removing these would "fix"
> 6 errors.

IIRC, the mirror primitive tests were designed to test that certain
primitives will be applied to the "last argument" instead of only to the
receiver. I.e., such that one could do both:

Object>>class
   "answer the class of the receiver"
   <primitive: 1234>

as well as:

Mirror>>classOf: anObject
   <primitive: 1234>

It would be good if we could keep those working (perhaps ask Eliot for
an opinion).

> Building a vm with the latest VMMaker lets 6 failing test pass
> (AllocationTest, BitBltTests). It also avoids crashing linux as reported
> before
> (http://lists.squeakfoundation.org/pipermail/squeak-dev/2009-December/142074.html).

I would opt to fail the crashing AllocationTests on Linux, i.e.,
something like:

     "the test currently crashes the vm on unix"
     Smalltalk platformName = 'unix' ifTrue:[^self assert: false].

mark it expected failure, and later, when we have fixed VMs, allow the
test to run.

> And it also yields slight performance improvement because of the
> primitiveNextPut fix. Why aren't vm releases available with these fixes?

Because nobody's been asking for then :-) A post to vm-dev might help.

> There are two "great challenges" which would let most other failing
> tests pass:
> - fixing traits (the issues are solved in pharo, but one has to extract
> and integrate them)
> - fixing the decompiler (there's an issue with temporary variable
> declarations in inlined blocks)
> Is anybody working on these?

I'm working on the former (time permitting) and I'd check with Eliot for
the latter.

> - MorphicToolBuilderTests>>#testGetButtonSideEffectFree

I can take care of that.

Cheers,
   - Andreas

Reply | Threaded
Open this post in threaded view
|

Re: Re: Failing tests

David T. Lewis
On Mon, Dec 14, 2009 at 08:23:27PM -0800, Andreas Raab wrote:

> Levente Uzonyi wrote:
>
> >Mirror primitives do not exists in the current vm. Will the vm have
> >mirror primitives? Should we keep the tests? Removing these would "fix"
> >6 errors.
>
> IIRC, the mirror primitive tests were designed to test that certain
> primitives will be applied to the "last argument" instead of only to the
> receiver. I.e., such that one could do both:
>
> Object>>class
>   "answer the class of the receiver"
>   <primitive: 1234>
>
> as well as:
>
> Mirror>>classOf: anObject
>   <primitive: 1234>
>
> It would be good if we could keep those working (perhaps ask Eliot for
> an opinion).

It would also be helpful to get a brief description that can be applied
to the (missing) class comment for MirrorPrimitiveTests. I found a brief
reference on Eliot's blog, from discussion with Florin Mateoc:

  Florin:
  3. bytecodes for fast and guaranteed access to an object's public
     and hidden fields (size, class, and identity hash). These can be
     expressed in source code with what you already have (e.g. in VW):

       thisContext _objectClass: anObject

     and so on, but it does not have to be translated to normal
     bytecodes/message sends, as this is one of the few instances where
     things can be statically compiled (if only we had the right bytecodes)

  Eliot:
     Right-oh. I call these Mirror Primitives after David and Gilad's
     work on Self.

But I don't understand this well enough to turn it into a class comment.

Dave


Reply | Threaded
Open this post in threaded view
|

Re: Re: Failing tests

Levente Uzonyi-2
In reply to this post by Andreas.Raab
On Mon, 14 Dec 2009, Andreas Raab wrote:

> Levente Uzonyi wrote:
>> I'd like to have less failing tests in the trunk. For this, we have to make
>> decisions/anwser questions.
>
> Thanks for taking the time to go through these tests carefully!
>
>> Should we change the default, so that LongTestCases are run when selected
>> in the Test Runner? (Or one could add a chechbox to the gui which would let
>> the user to decide if she/he wants to run long tests? Or this could be a
>> preference, etc.)
>
> I would opt for a preference or a checkbox in the UI.

I added a preference, maybe someone will update the gui.

>
>> Mirror primitives do not exists in the current vm. Will the vm have mirror
>> primitives? Should we keep the tests? Removing these would "fix" 6 errors.
>
> IIRC, the mirror primitive tests were designed to test that certain
> primitives will be applied to the "last argument" instead of only to the
> receiver. I.e., such that one could do both:
>
> Object>>class
>  "answer the class of the receiver"
>  <primitive: 1234>
>
> as well as:
>
> Mirror>>classOf: anObject
>  <primitive: 1234>
>
> It would be good if we could keep those working (perhaps ask Eliot for an
> opinion).
>

We should definitely keep the tests. After a bit of googling I found
Eliot's email which didn't get much attention:
http://lists.squeakfoundation.org/pipermail/vm-dev/2009-September/003161.html
This explains everything about mirror primitives. I wonder why these
changes aren't integrated into VMMaker. (Note that ContextPart >>
#objectClass: is missing from the attached source, I added an
implementation to the trunk). I marked the failing tests as expected
failures.

>> Building a vm with the latest VMMaker lets 6 failing test pass
>> (AllocationTest, BitBltTests). It also avoids crashing linux as reported
>> before
>> (http://lists.squeakfoundation.org/pipermail/squeak-dev/2009-December/142074.html).
>
> I would opt to fail the crashing AllocationTests on Linux, i.e., something
> like:
>
>    "the test currently crashes the vm on unix"
>    Smalltalk platformName = 'unix' ifTrue:[^self assert: false].
>
> mark it expected failure, and later, when we have fixed VMs, allow the test
> to run.
>

Done, feedback is welcome.

>> And it also yields slight performance improvement because of the
>> primitiveNextPut fix. Why aren't vm releases available with these fixes?
>
> Because nobody's been asking for then :-) A post to vm-dev might help.
>

I'm on it.

>> There are two "great challenges" which would let most other failing tests
>> pass:
>> - fixing traits (the issues are solved in pharo, but one has to extract and
>> integrate them)
>> - fixing the decompiler (there's an issue with temporary variable
>> declarations in inlined blocks)
>> Is anybody working on these?
>
> I'm working on the former (time permitting) and I'd check with Eliot for the
> latter.
>

Great. :)

>> - MorphicToolBuilderTests>>#testGetButtonSideEffectFree
>
> I can take care of that.
>

Cool. :)


Levente

> Cheers,
>  - Andreas
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Re: Failing tests

David T. Lewis
On Tue, Dec 15, 2009 at 08:35:55PM +0100, Levente Uzonyi wrote:

>
> We should definitely keep the tests. After a bit of googling I found
> Eliot's email which didn't get much attention:
> http://lists.squeakfoundation.org/pipermail/vm-dev/2009-September/003161.html
> This explains everything about mirror primitives. I wonder why these
> changes aren't integrated into VMMaker. (Note that ContextPart >>
> #objectClass: is missing from the attached source, I added an
> implementation to the trunk). I marked the failing tests as expected
> failures.
>

Thanks for the pointer. I somehow completely overlooked this posting
on the vm-dev list. I opened a Mantis issue for "Add Mirror Primitives
to the VM" to track it.

  http://bugs.squeak.org/view.php?id=7429

Dave


Reply | Threaded
Open this post in threaded view
|

Re: Re: Failing tests

Levente Uzonyi-2
On Tue, 15 Dec 2009, David T. Lewis wrote:

> On Tue, Dec 15, 2009 at 08:35:55PM +0100, Levente Uzonyi wrote:
>>
>> We should definitely keep the tests. After a bit of googling I found
>> Eliot's email which didn't get much attention:
>> http://lists.squeakfoundation.org/pipermail/vm-dev/2009-September/003161.html
>> This explains everything about mirror primitives. I wonder why these
>> changes aren't integrated into VMMaker. (Note that ContextPart >>
>> #objectClass: is missing from the attached source, I added an
>> implementation to the trunk). I marked the failing tests as expected
>> failures.
>>
>
> Thanks for the pointer. I somehow completely overlooked this posting
> on the vm-dev list. I opened a Mantis issue for "Add Mirror Primitives
> to the VM" to track it.
>
>  http://bugs.squeak.org/view.php?id=7429

Great, thanks. I would be cool if these changes could get into the next vm
release IMO.


Levente

>
> Dave
>
>
>

Reply | Threaded
Open this post in threaded view
|

Mirror prims

Bert Freudenberg
In reply to this post by David T. Lewis
On 16.12.2009, at 03:11, David T. Lewis wrote:

>
> On Tue, Dec 15, 2009 at 08:35:55PM +0100, Levente Uzonyi wrote:
>>
>> We should definitely keep the tests. After a bit of googling I found
>> Eliot's email which didn't get much attention:
>> http://lists.squeakfoundation.org/pipermail/vm-dev/2009-September/003161.html
>> This explains everything about mirror primitives. I wonder why these
>> changes aren't integrated into VMMaker. (Note that ContextPart >>
>> #objectClass: is missing from the attached source, I added an
>> implementation to the trunk). I marked the failing tests as expected
>> failures.
>>
>
> Thanks for the pointer. I somehow completely overlooked this posting
> on the vm-dev list. I opened a Mantis issue for "Add Mirror Primitives
> to the VM" to track it.
>
>  http://bugs.squeak.org/view.php?id=7429
>
> Dave


IIUC these new primitives break object encapsulation, which is fundamental change to the VM.

Do we care?

If so, do we still want them?

If yes, should there be a way to disable them, similarly to how we can disable file access?

- Bert -


Reply | Threaded
Open this post in threaded view
|

Re: Mirror prims

Levente Uzonyi-2
On Wed, 16 Dec 2009, Bert Freudenberg wrote:

> On 16.12.2009, at 03:11, David T. Lewis wrote:
>>
>> On Tue, Dec 15, 2009 at 08:35:55PM +0100, Levente Uzonyi wrote:
>>>
>>> We should definitely keep the tests. After a bit of googling I found
>>> Eliot's email which didn't get much attention:
>>> http://lists.squeakfoundation.org/pipermail/vm-dev/2009-September/003161.html
>>> This explains everything about mirror primitives. I wonder why these
>>> changes aren't integrated into VMMaker. (Note that ContextPart >>
>>> #objectClass: is missing from the attached source, I added an
>>> implementation to the trunk). I marked the failing tests as expected
>>> failures.
>>>
>>
>> Thanks for the pointer. I somehow completely overlooked this posting
>> on the vm-dev list. I opened a Mantis issue for "Add Mirror Primitives
>> to the VM" to track it.
>>
>>  http://bugs.squeak.org/view.php?id=7429
>>
>> Dave
>
>
> IIUC these new primitives break object encapsulation, which is fundamental change to the VM.

Most primitives already work like this in the current vm (at least the
MirrorPrimitiveTests pass), 2 work but leave garbage on the stack
(primitive 62 (#size) and 110 (#==)) and only one fails (primitive
100 #perform:withArguments:inSuperClass:).

>
> Do we care?
>
> If so, do we still want them?
>

We have them in some way, so fixing them is a good idea.


Levente

> If yes, should there be a way to disable them, similarly to how we can disable file access?
>
> - Bert -
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Mirror prims

Eliot Miranda-2
In reply to this post by Bert Freudenberg


On Wed, Dec 16, 2009 at 2:21 AM, Bert Freudenberg <[hidden email]> wrote:
On 16.12.2009, at 03:11, David T. Lewis wrote:
>
> On Tue, Dec 15, 2009 at 08:35:55PM +0100, Levente Uzonyi wrote:
>>
>> We should definitely keep the tests. After a bit of googling I found
>> Eliot's email which didn't get much attention:
>> http://lists.squeakfoundation.org/pipermail/vm-dev/2009-September/003161.html
>> This explains everything about mirror primitives. I wonder why these
>> changes aren't integrated into VMMaker. (Note that ContextPart >>
>> #objectClass: is missing from the attached source, I added an
>> implementation to the trunk). I marked the failing tests as expected
>> failures.
>>
>
> Thanks for the pointer. I somehow completely overlooked this posting
> on the vm-dev list. I opened a Mantis issue for "Add Mirror Primitives
> to the VM" to track it.
>
>  http://bugs.squeak.org/view.php?id=7429
>
> Dave


IIUC these new primitives break object encapsulation, which is fundamental change to the VM.

Do we care?

Very much so.  The implementation of the execution simulation machinery (the debugger) is incorrect without them.  For example, if you step through code on an encapsulator using the existing machinery every time an attempt is made to access an instance variable the message instVarAt: will get sent to the encapsulator, which will catch it in its doesNotUnderstand: method and answer an entirely erroneous value.

The execution simulation machinery *must not* send messages to access state for correctness.  The VM does not send messages to access state; the execution simulation machinery must mimic the VM.

This is not much worse a violation of encapsulation than instVarAt: and instVarAt:put:.  Yes, the facilities can be misused, but so can thisContext, instVarAt: and much other reflective machinery.  In practice they are not misused and their power by far repays their danger.

On the other hand, not being able to debug exotic code is a serious problem.  This is often the kind of code one most needs help with.


If so, do we still want them?

If yes, should there be a way to disable them, similarly to how we can disable file access?

In the short/medium term, unload the debugger, the compiler, and any other reflective machinery.

In the bright rosy future concoct a convincing story around capabilities or mirrors which carefully modulate use of these facilities so they can't be misused.
 

- Bert -





Reply | Threaded
Open this post in threaded view
|

Re: Mirror prims

Bert Freudenberg
On 16.12.2009, at 18:43, Eliot Miranda wrote:


On Wed, Dec 16, 2009 at 2:21 AM, Bert Freudenberg <[hidden email]> wrote:
On 16.12.2009, at 03:11, David T. Lewis wrote:
>
> On Tue, Dec 15, 2009 at 08:35:55PM +0100, Levente Uzonyi wrote:
>>
>> We should definitely keep the tests. After a bit of googling I found
>> Eliot's email which didn't get much attention:
>> http://lists.squeakfoundation.org/pipermail/vm-dev/2009-September/003161.html
>> This explains everything about mirror primitives. I wonder why these
>> changes aren't integrated into VMMaker. (Note that ContextPart >>
>> #objectClass: is missing from the attached source, I added an
>> implementation to the trunk). I marked the failing tests as expected
>> failures.
>>
>
> Thanks for the pointer. I somehow completely overlooked this posting
> on the vm-dev list. I opened a Mantis issue for "Add Mirror Primitives
> to the VM" to track it.
>
>  http://bugs.squeak.org/view.php?id=7429
>
> Dave


IIUC these new primitives break object encapsulation, which is fundamental change to the VM.

Do we care?

Very much so.  The implementation of the execution simulation machinery (the debugger) is incorrect without them.  For example, if you step through code on an encapsulator using the existing machinery every time an attempt is made to access an instance variable the message instVarAt: will get sent to the encapsulator, which will catch it in its doesNotUnderstand: method and answer an entirely erroneous value.

The execution simulation machinery *must not* send messages to access state for correctness.  The VM does not send messages to access state; the execution simulation machinery must mimic the VM.

This is not much worse a violation of encapsulation than instVarAt: and instVarAt:put:.

It is fundamentally different, because those methods actually have to be implemented in the object. My object can intercept #instVarAt: but not the mirror prims.

 Yes, the facilities can be misused, but so can thisContext, instVarAt: and much other reflective machinery.  In practice they are not misused and their power by far repays their danger.

On the other hand, not being able to debug exotic code is a serious problem.  This is often the kind of code one most needs help with.

Agreed, the facility is useful in debugging exotic code. Most code isn't exotic though. So my question stands if we want to compromise the whole system for these exotic situations. Since the facility is useful it might be okay to include if we can turn it off.


If so, do we still want them?

If yes, should there be a way to disable them, similarly to how we can disable file access?

In the short/medium term, unload the debugger, the compiler, and any other reflective machinery.

Won't help, since with access to that primitive my object can do pretty much anything.

In the bright rosy future concoct a convincing story around capabilities or mirrors which carefully modulate use of these facilities so they can't be misused.

That's exactly my point - I don't see how you could do a safe capability-based system with those primitives that can work around any encapsulation and hence can circumvent any capabilities.

- Bert -



Reply | Threaded
Open this post in threaded view
|

Re: Mirror prims

Colin Putney

On 2009-12-16, at 11:24 AM, Bert Freudenberg wrote:

> On 16.12.2009, at 18:43, Eliot Miranda wrote:
>
>> In the bright rosy future concoct a convincing story around capabilities or mirrors which carefully modulate use of these facilities so they can't be misused.
>
> That's exactly my point - I don't see how you could do a safe capability-based system with those primitives that can work around any encapsulation and hence can circumvent any capabilities.

Just make the primitives refuse to violate encapsulation for any receiver that's not a Mirror.

Colin
Reply | Threaded
Open this post in threaded view
|

Re: Mirror prims

Eliot Miranda-2
In reply to this post by Bert Freudenberg


On Wed, Dec 16, 2009 at 11:24 AM, Bert Freudenberg <[hidden email]> wrote:
On 16.12.2009, at 18:43, Eliot Miranda wrote:


On Wed, Dec 16, 2009 at 2:21 AM, Bert Freudenberg <[hidden email]> wrote:
On 16.12.2009, at 03:11, David T. Lewis wrote:
>
> On Tue, Dec 15, 2009 at 08:35:55PM +0100, Levente Uzonyi wrote:
>>
>> We should definitely keep the tests. After a bit of googling I found
>> Eliot's email which didn't get much attention:
>> http://lists.squeakfoundation.org/pipermail/vm-dev/2009-September/003161.html
>> This explains everything about mirror primitives. I wonder why these
>> changes aren't integrated into VMMaker. (Note that ContextPart >>
>> #objectClass: is missing from the attached source, I added an
>> implementation to the trunk). I marked the failing tests as expected
>> failures.
>>
>
> Thanks for the pointer. I somehow completely overlooked this posting
> on the vm-dev list. I opened a Mantis issue for "Add Mirror Primitives
> to the VM" to track it.
>
>  http://bugs.squeak.org/view.php?id=7429
>
> Dave


IIUC these new primitives break object encapsulation, which is fundamental change to the VM.

Do we care?

Very much so.  The implementation of the execution simulation machinery (the debugger) is incorrect without them.  For example, if you step through code on an encapsulator using the existing machinery every time an attempt is made to access an instance variable the message instVarAt: will get sent to the encapsulator, which will catch it in its doesNotUnderstand: method and answer an entirely erroneous value.

The execution simulation machinery *must not* send messages to access state for correctness.  The VM does not send messages to access state; the execution simulation machinery must mimic the VM.

This is not much worse a violation of encapsulation than instVarAt: and instVarAt:put:.

It is fundamentally different, because those methods actually have to be implemented in the object. My object can intercept #instVarAt: but not the mirror prims.

 Yes, the facilities can be misused, but so can thisContext, instVarAt: and much other reflective machinery.  In practice they are not misused and their power by far repays their danger.

On the other hand, not being able to debug exotic code is a serious problem.  This is often the kind of code one most needs help with.

Agreed, the facility is useful in debugging exotic code. Most code isn't exotic though. So my question stands if we want to compromise the whole system for these exotic situations. Since the facility is useful it might be okay to include if we can turn it off.

There is a better way, but it requires rewriting the primitives.  As a quick hack I simply used primitiveInstVarAt et al and made them pop the right number of arguments instead of assuming what their argumentCount was.  But if the mirror primitives were implemented in MethodContext they could fail if the object being operated on wasn't the receiver slot.  [Alternatively we could write the primitives to take the object to be operated on from the receiver slot, but that's trickier].  That way only a context could meddle with its receiver, which is no material change, given that that;s what contexts can do now.

Is that acceptable?


If so, do we still want them?

If yes, should there be a way to disable them, similarly to how we can disable file access?

In the short/medium term, unload the debugger, the compiler, and any other reflective machinery.

Won't help, since with access to that primitive my object can do pretty much anything.

In the bright rosy future concoct a convincing story around capabilities or mirrors which carefully modulate use of these facilities so they can't be misused.

That's exactly my point - I don't see how you could do a safe capability-based system with those primitives that can work around any encapsulation and hence can circumvent any capabilities.

Bert,  how do you stop me doing yourObject class compile: 'trojanHorse: index <primitive: 73> ^nil' ?  Being able to implement instVarAt: is no protection.  You need to do a lot more work than that.


- Bert -







12