[Pharo-dev] [Debugger] Is "run to here" broken?

Guillermo Polito guillermopolito at gmail.com
Wed Nov 15 04:47:58 EST 2017


Thomas, can you try writing some real test cases in SUnit?

I don't care if they are originally red (they will be because it is not
working).

The idea is that we capture all these different scenarios and have a way to
know automatically in the future if
 - we fixed all them
 - or we have a regression and broke some.

On Wed, Nov 15, 2017 at 10:44 AM, Thomas Dupriez <
thomas.dupriez at ens-paris-saclay.fr> wrote:

>
>
> Le 15/11/2017 à 01:19, Ben Coman a écrit :
>
>
>
> On 14 November 2017 at 23:55, Thomas Dupriez <thomas.dupriez at ens-paris-
> saclay.fr> wrote:
>
>> While working on writing a test, I discovered that just using stepThrough
>> instead of stepOver may not be the definitive solution.
>>
>> # Example1: When using the `value` message directly (instead of calling a
>> method that does that as in my previous example), the RunToHere moves the
>> debugSession to the `value` message and not the inside of the block.
>>
>> `meth2
>>     self halt.
>>     [
>>         1+1.
>>         2+2."< cursor before the RunToHere"
>>     ] ">"value"<result of the RunToHere".
>>     3+3.
>>     "Two stepInto are required after the RunToHere to reach the cursor
>> (the `+2` message)"`
>>
>> # Example2: Going back to using a dedicated method for evaluating the
>> block, but increasing the block's size. The RunToHere moves the
>> debugSession to the first message of the block instead of the pointed one.
>>
>> `meth3
>>     self halt.
>>     MyClass new evalBlock:
>>     [
>>         1">"+1"< result of the RunToHere".
>>         2+2."< cursor before the RunToHere"
>>     ].
>>     3+3.
>>     "One stepInto is required after the RunToHere to reach the cursor
>> (the `+2` message)"`
>>
>> cheers -thomas
>>
>
> In both cases, what happens with an additional step-Through?
>
> cheers -ben
>
>
> # Example1*:
> Operation: RunToHere with cursor at the end of the `2+2.` line + 1
> stepThrough.
> Result: DebugSession at the `+1`, first instruction of the block but not
> the intended position.
> Note: Doing an additional stepThrough brings the debugSession to the
> intended `+2`.
>
> `meth2
>     self halt.
>     [
>         1">"+1"<"."result of the RunToHere + 1 stepThrough"
>         2+2."< cursor before the RunToHere"
>     ] ">"value"<result of the RunToHere".
>     3+3.
>     "Two stepInto are required after the RunToHere to reach the cursor
> (the `+2` message)"`
>
> # Example2*:
> Operation: RunToHere with cursor at the end of the `2+2.` line + 1
> stepThrough
> Result: DebugSession at the intended `+2` position.
> Note: The additional stepThrough just moves the DebugSession to the next
> instruction. See Example3 for what happens when the target instruction is
> further away.
>
> `meth3
>     self halt.
>     MyClass new evalBlock:
>     [
>         1">"+1"< result of the RunToHere".
>         2">"+2"<result of RunToHere+1stepThrough"."< cursor before the
> RunToHere"
>     ].
>     3+3.
>     "One stepInto is required after the RunToHere to reach the cursor (the
> `+2` message)"`
>
>
> # Example3:
> Operation: RunToHere with cursor at the end of the `3+3.` line. + 1
> stepThrough.
> Result: DebugSession is at the `+2` instruction, not the intended `+3` one.
>
> `meth4
>     self halt.
>     MyClass new evalBlock:
>     [
>         1">"+1"< result of the RunToHere".
>         2">"+2"<result of RunToHere+1stepThrough".
>         3">"+3"<result of RunToHere+2stepThrough"."< cursor before the
> RunToHere"
>     ].
>     4+4.
>     "One stepInto is required after the RunToHere to reach the cursor (the
> `+2` message)"`
>
> cheers -thomas
>
> Le 13/11/2017 à 17:23, Thomas Dupriez a écrit :
>>
>>
>>
>> Le 13/11/2017 à 15:56, Ben Coman a écrit :
>>
>>
>>
>> On Mon, Nov 13, 2017 at 9:32 PM, Thomas Dupriez <
>> thomas.dupriez at ens-paris-saclay.fr> wrote:
>>
>>>
>>>
>>> Le 13/11/2017 à 14:08, Ben Coman a écrit :
>>>
>>>
>>>
>>> On Mon, Nov 13, 2017 at 8:40 PM, Thomas Dupriez <
>>> thomas.dupriez at ens-paris-saclay.fr> wrote:
>>>
>>>> I dug a bit in this issue. Here are the results:
>>>>
>>>>
>>>> # Problem raised by Stephanne
>>>> Code like the following is open in the debugger:
>>>>
>>>>     `myMethod
>>>>         1+1. <PROGRAMCOUNTER>
>>>>         MyClass new myEvalBlock: [
>>>>             2+2. <CURSOR>
>>>>         ].
>>>>         3+3.`
>>>>
>>>> The program counter is on the 1+1, the cursor is at the end of the 2+2
>>>> line.
>>>> Right-click, "Run to here".
>>>> -> the program counter moves to the 3+3, and does not stop at the 2+2
>>>> (where the cursor is). Even though the 2+2 does get evaluated (the
>>>> myEvalBlock method evaluates the blocks it receives).
>>>> If there was no code after the block, the debugger would jump back to
>>>> the caller of myMethod.
>>>>
>>>> # Why this happens
>>>> The implementation of RunToHere (source code below) is basically to
>>>> stepOver until the context is different or the source code position of the
>>>> program counter is higher or equal to the source code position of the
>>>> cursor/selection.
>>>>
>>>> Since the debugger uses stepOver, it executes myEvalBlock without
>>>> stopping, reaches the 3+3 and see it has gone further than the source code
>>>> position of the cursorm so it stops.
>>>>
>>>> Source code of DebugSession>>#runToSelection:inContext:
>>>>
>>>>     `runToSelection: selectionInterval inContext: aContext
>>>>         "Attempt to step over instructions in selectedContext until the
>>>>         execution reaches the selected instruction. This happens when
>>>> the
>>>>         program counter passes the begining of selectionInterval.
>>>>
>>>>         A not nill and valid interval is expected."
>>>>
>>>>         (self pcRangeForContext: aContext) first >= selectionInterval
>>>> first
>>>>             ifTrue: [ ^self ].
>>>>         self stepOver: aContext.
>>>>         [ aContext == self interruptedContext and: [ (self
>>>> pcRangeForContext: aContext) first < selectionInterval first ] ]
>>>>             whileTrue: [ self stepOver: aContext ]`
>>>>
>>>> # Observations and thoughts
>>>> - Replacing the stepOver with a stepInto -> This made the RunToHere
>>>> stops when resolving the 'new' message (because the context changes).
>>>>
>>>
>>> Thanks for looking into this Thomas.
>>> What happens if you use stepThrough rather than stepInto?
>>>
>>> cheers -ben
>>>
>>>
>>> Using `stepThrough: aContext` instead of `stepInto: aContext`, the
>>> RunToHere in the block stops at the intended place: the 2+2.
>>> Printing the sequence of `self pcRangeForContext: aContext` yields the
>>> following. It doesn't show precisely the stop in the block, and instead
>>> shows an interval encompassing the whole block. That's what I was using to
>>> see where the execution was going during the RunToHere loop so I guess
>>> that's not a precise enough indicator for this situation...
>>> (21 to: 22)(34 to: 36)(49 to: 60)(38 to: 60)(38 to: 60)
>>>
>>> I also tried the case where the block does not get evaluated (changing
>>> myEvalBlock so that it does nothing). In this situation, the RunToHere with
>>> a stepThrough ends up at the intended place, the 3+3.
>>>
>>> So... just use stepThrough for the RunToHere I guess?
>>>
>>
>> Seems like expected behaviour.
>> Could you create an Issue/Pull Request to provide some concrete code to
>> review?
>>
>> Now the trick will be if you can devise some tests to go with this.
>> For examples perhaps look at senders of newDebugSessionNamed:startedAt:
>> including SpecDebugger>>testBasic.
>>
>> cheers -ben
>>
>>
>>
>> I opened an issue on FogBugz: https://pharo.fogbugz.com/f/cases/20687/
>>
>> Attached are the file out of the test code (MyClass.st) and the fixed
>> runToSelection:inContext: (DebugSession-runToSelectioninContext).
>> I'll make a pull request tomorrow.
>>
>> For the test, I'm thinking about something like the following, that
>> checks the source code position of the pc after the runToSelection. (It's
>> not working yet)
>> Now that I think of it, the source code position of the pc will maybe be
>> the whole block (as experienced earlier) so this approach may end up not
>> working.
>> I'll come back to that tomorrow.
>>
>> Draft for a test:
>>     `testRunToSelectionInContext
>>     |context_ process_ session_|
>>     "The variable names have an underscore to distinguish them from those
>> declared in the setUp method of this TestCase (DebuggerModelTest), which
>> are not suited for this test (because not making a debug session out of
>> suitable code)"
>>     context_ := [1+1.
>>         self evalBlock:[2+2].
>>         3+3.
>>     ] asContext.
>>     process_ := Process
>>         forContext: context_
>>         priority: Processor userInterruptPriority.
>>
>>     session_:= process_ newDebugSessionNamed: 'test session' startedAt:
>> context_.
>>     session_ runToSelection: (Interval from: 28 to: 27) inContext:
>> context_.
>>     self assert: ((session_ pcRangeForContext: context_) first == 3)`
>>
>> cheers - Thomas
>>
>>
>>> Thomas
>>>
>>>
>>>
>>>
>>>
>>>> - Replacing the stepOver with stepInto and removing the equal condition
>>>> on contexts -> The RunToHere goes to the 3+3. Looking at the source code
>>>> position of the program counter, it doesn't enter the block and seems to
>>>> resolve it in a single step. I don't really get why that is, considering
>>>> using the stepInto button of the debugger does enter the block. Here is the
>>>> series of source code positions of the program counter during the
>>>> RunToHere: (21 to: 22)(34 to: 36)(34 to: 36)(49 to: 60)(38 to: 60)(38 to:
>>>> 60)(65 to: 66).
>>>> However, removing the equal condition on contexts means that if the
>>>> method call returns before reaching the cursor, it won't stop!
>>>>
>>>> - An idea could be to have the RunToHere place a metalink on the
>>>> selected node and let the execution run until it hits the metalink, which
>>>> then updates the debugger. Potential problems are that it implies
>>>> installing a metalink on a method that is already on the stack, which may
>>>> not be that easy to do properly (in particular, it affects the program
>>>> counter since it changes the bytecode), and there is the potential case
>>>> where the metalink is never reached (for example imagine the myEvalBlock:
>>>> method of my example is just storing the block and not evaluating it).
>>>>
>>>>
>>>> Cheers,
>>>> Thomas
>>>>
>>>>
>>>>
>>>> Le 09/11/2017 à 22:06, Stephane Ducasse a écrit :
>>>>
>>>>> Agreed. Thomas? It would be a good bone to ... (Un bon os a ronger) .
>>>>>
>>>>> Stef
>>>>>
>>>>> On Thu, Nov 9, 2017 at 4:32 AM, Tudor Girba <tudor at tudorgirba.com>
>>>>> wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> The basic tools, such as debugger, are expected to work. If something
>>>>>> does not work, it’s a bug.
>>>>>>
>>>>>> Cheers,
>>>>>> Doru
>>>>>>
>>>>>>
>>>>>> On Nov 8, 2017, at 11:59 PM, Tim Mackinnon <tim at testit.works>
>>>>>>> <tim at testit.works> wrote:
>>>>>>>
>>>>>>> I think it's broken in Pharo 6 too, as I often find it unreliable.
>>>>>>>
>>>>>>> It's hard to know what should work anymore - we really need a
>>>>>>> stabilisation release to let the dust settle.
>>>>>>>
>>>>>>> I'm always a bit reticent to report things as I'm not sure what you
>>>>>>> expect to work.
>>>>>>>
>>>>>>> Tim
>>>>>>>
>>>>>>> Sent from my iPhone
>>>>>>>
>>>>>>> On 8 Nov 2017, at 20:40, Stephane Ducasse <stepharo.self at gmail.com>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>> Hi
>>>>>>>>
>>>>>>>> I have the following method and I have my cursor -MY CURSOR HERE-
>>>>>>>> I select the menu run to here and .... I exit the method.
>>>>>>>> :(
>>>>>>>>
>>>>>>>> Is run to here working in Pharo 70?
>>>>>>>> I start to get worry about the number of bugs I get when using
>>>>>>>> Pharo70.
>>>>>>>>
>>>>>>>> Stef
>>>>>>>>
>>>>>>>>
>>>>>>>> fileOut
>>>>>>>> "File out the receiver, to a file whose name is a function of the
>>>>>>>> change-set name and a unique numeric tag."
>>>>>>>>
>>>>>>>> | nameToUse |
>>>>>>>> self halt.
>>>>>>>> self class promptForDefaultChangeSetDirectoryIfNecessary.
>>>>>>>> nameToUse := (self defaultChangeSetDirectory / self name , 'cs')
>>>>>>>> nextVersion basename.
>>>>>>>> UIManager default
>>>>>>>> showWaitCursorWhile:
>>>>>>>> [
>>>>>>>> | internalStream |
>>>>>>>> internalStream := (String new: 10000) writeStream.
>>>>>>>>
>>>>>>>> -MY CURSOR HERE-
>>>>>>>>
>>>>>>>> internalStream
>>>>>>>> header;
>>>>>>>> timeStamp.
>>>>>>>> self fileOutPreambleOn: internalStream.
>>>>>>>> self fileOutOn: internalStream.
>>>>>>>> self fileOutPostscriptOn: internalStream.
>>>>>>>> CodeExporter
>>>>>>>> writeSourceCodeFrom: internalStream
>>>>>>>> baseName: (nameToUse copyFrom: 1 to: nameToUse size - 3)
>>>>>>>> isSt: false ]
>>>>>>>>
>>>>>>>>
>>>>>>> --
>>>>>> www.tudorgirba.com
>>>>>> www.feenk.com
>>>>>>
>>>>>> "Value is always contextual."
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>>
>
>


-- 



Guille Polito

Research Engineer

Centre de Recherche en Informatique, Signal et Automatique de Lille

CRIStAL - UMR 9189

French National Center for Scientific Research - *http://www.cnrs.fr
<http://www.cnrs.fr>*


*Web:* *http://guillep.github.io* <http://guillep.github.io>

*Phone: *+33 06 52 70 66 13
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.pharo.org/pipermail/pharo-dev_lists.pharo.org/attachments/20171115/2f4a388b/attachment-0002.html>


More information about the Pharo-dev mailing list