bugGNU Octave - Bugs: bug #47917, dbstop skips nested functions

 
 

bug #47917: dbstop skips nested functions

Submitter:  Lachlan Andrew <lachlan>
Submitted:  Fri 13 May 2016 11:39:39 PM UTC
   
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  In Progress Assigned to:  lachlan
Originator Name:  Lachlan Open/Closed:  * Open
Release:  * dev Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 18 Jul 2016 06:40:04 PM UTC, comment #17: 

@Lachlan: Is file #37906 the final cset, or just for testing by Philip?  I notice that there is a block of code in  bp_table::do_get_breakpoint_list that is commented out with


/* .... */


That's fine for testing, but probably should be removed for production code.

Rik <rik5>
Group administrator
Sat 16 Jul 2016 09:46:48 AM UTC, comment #16: 

Lachlan,

I only used the last patch as I got the impression that the others have been pushed by Rik - see comment #7.
Is that correct?

Philip Nienhuis <philipnienhuis>
Group Member
Sat 16 Jul 2016 02:27:56 AM UTC, comment #15: 

Thanks for testing, philip.

There were three changeset (one for dbstatus, one to find nested functions, and one to show them).  Only one needed updating, so I only posted the new version of one.  I like to keep mq changesets small, since mq doesn't keep histories.

Here is a combined patch that contains all three changesets.

(file #37906)

Lachlan Andrew <lachlan>
Fri 15 Jul 2016 06:59:08 PM UTC, comment #14: 

With that latest patch:

../dev_VE/libinterp/octave-value/ov-usr-fcn.cc: In function 'std::string walk_nested_fcns(const octave_user_function*, const octave_user_function*, std::string)':
../dev_VE/libinterp/octave-value/ov-usr-fcn.cc:317:40: error: 'nested_fcn_separator' was not declared in this scope
           std::string nm = left_path + nested_fcn_separator + q->first;
                                        ^
Makefile:19544: recipe for target 'libinterp/octave-value/libinterp_octave_value_liboctave_value_la-ov-usr-fcn.lo' failed
make[2]: *** [libinterp/octave-value/libinterp_octave_value_liboctave_value_la-ov-usr-fcn.lo] Error 1


(Mageia 5)

Philip Nienhuis <philipnienhuis>
Group Member
Fri 15 Jul 2016 11:57:14 AM UTC, comment #13: 

Here is a refreshed patch to show nested function names.

(file #37888)

Lachlan Andrew <lachlan>
Thu 07 Jul 2016 04:47:43 PM UTC, comment #12: 

Cool.  At least Matlab didn't decide to switch the nested function hierarchy separator between Windows and Unix platforms.

Rik <rik5>
Group administrator
Thu 07 Jul 2016 04:19:32 PM UTC, comment #11: 

Setting breakpoint in GUI ("Enable breakpoint on current line"

>> cd c:\tmp
>> a
>> dbstatus
Breakpoint for a>c/d is on line 14.


Then using command line syntax:

>> dbstop in a
>> dbstop in a>b
>> dbstop in a>c
>> dbstop in a>c>d
Error using dbstop
Cannot find function "c>d" within "a".

>> dbstop in a>c/d
>> dbstop in a>c\d
Error using dbstop
Cannot find function "c\d" within "a".

>> dbstatus
Breakpoint for a is on line 6.

Breakpoint for a>b is on line 4.

Breakpoint for a>c is on line 11.

Breakpoint for a>c/d is on line 14.


ML 2016b prerelease

I might have time to try the patches this weekend, maybe tomorrow

Philip Nienhuis <philipnienhuis>
Group Member
Thu 07 Jul 2016 02:00:51 PM UTC, comment #10: 

Dang.  Matlab has buried that syntax so deep I've never seen it.  Not critical for the 4.2 release, but probably a good thing to do.

Rik <rik5>
Group administrator
Thu 07 Jul 2016 12:56:23 PM UTC, comment #9: 

It turns out that Matlab's syntax is  a>c/d.  I'm not sure if that becomes  a>c\d  on Windows.

(Philip [added to Cc list], if you have access to Matlab on Windows, can you check what is reported if you set a breakpoint in function  d()  in Rik's file  a.m  and run dbstatus?  Thanks!)

The reason dbstop didn't find a>c/d or a>c>d is that the symbol table didn't support support that at all.  Three patches are attached.  One allows the syntax to be used.  One generates it for error messages.  One causes dbstatus to display breakpoints in nested functions correctly.


(file #37752, file #37753, file #37754)

Lachlan Andrew <lachlan>
Sat 21 May 2016 12:04:07 AM UTC, comment #8: 

Thanks, Rik.

I just tried to check whether dbstatus returns a>c>d or a>d (which is ambiguous, as d may also be nested in a different subfunction), and found that the new breakpoint doesn't show up at all :(

I'll fix dbstatus first, then look into using a>c>d syntax.

Lachlan Andrew <lachlan>
Fri 20 May 2016 06:11:30 PM UTC, comment #7: 

@Lachlan: I applied both of your patches.  On the second patch, I changed "FIXME --" to "FIXME:" since that is what we use.  I also changed the comments which referred to the old variable line so that they refer to the new variable lineno.

This may be reaching, but is there a way to set a debug point within a nested function of a subfunction using a string syntax?

For ordinary subfunctions, you can use the expression


dbstop in main_fcn>subfunction


to have the interpreter stop at the first line of the subfunction.  Consider the m-file a.m which is attached.


function a

  function b
    x = 1;
  end
  y = 1;

end

function c
  z = 1;

  function d
    w = 2;
  end

end


Within Octave the following all work:


dbstop in a        % main function
dbstop in a>b      % nested function within main function
dbstop in a>c      % subfunction of main function


However, I can't set a breakpoint on function d, a nested function of a subfunction.


octave:4> dbstop in a>d
error: add_breakpoint: unable to find function 'a>d'
octave:4> dbstop in a>c>d
error: add_breakpoint: unable to find function 'a>c>d'


The first thing to test is whether Matlab allows this.  If it doesn't then at least we are compatible and can set the breakpoint using a line number.


(file #37214)

Rik <rik5>
Group administrator
Wed 18 May 2016 03:26:57 AM UTC, comment #6: 

The attached patch has the recursive code to handle functions nested within subfunctions.  It should be applied on top of the .v2 patch in file #37185.

This time I've explicitly tested it on scripts, functions within scripts and functions within functions.

There is one idiosyncracy.  For


function a

  function b
    x = 1;
  end
  y = 1;
end

function c
  z = 1;
end


trying to set a breakpoint in line 2 (the blank line before b) will set the breakpoint on the next line in function a (since line 2 is in a), whereas trying to set a breakpoint in the blank line before c will set the breakpoint to the first executable line of c.

I think this is reasonable, or at least not worth the effort of changing.

(file #37193)

Lachlan Andrew <lachlan>
Tue 17 May 2016 12:50:48 PM UTC, comment #5: 

Thanks, Rik.  You're absolutely right. The attached changeset should fix that.

However, in testing it, I noticed that functions nested within subfunctions are still not covered, since they are not subfunctions of the main function.

If you apply the .v2 patch, please keep this bug report open while I write that larger patch.

Thanks also for applying all the other patches, and giving tips along the way -- I've been meaning to find out the C++ version of MAXINT for ages!

(file #37185)

Lachlan Andrew <lachlan>
Mon 16 May 2016 04:58:29 AM UTC, comment #4: 

The new issue is probably the result of this construct


          // The breakpoint is either in the subfunction found above,
          // or in the main function, which we check now.
          if (main_fcn->is_user_function ())
            {
              int e = dynamic_cast<octave_user_function*> (main_fcn)
                      ->ending_line ();
              if (e >= lineno && e < earliest_end)
                dbg_fcn = main_fcn;
            }


I'm pretty sure a different case needs to exist for is_user_script ().


Rik <rik5>
Group administrator
Mon 16 May 2016 04:44:49 AM UTC, comment #3: 

Oops.  I have to reopen this.  Apparently you can no longer set breakpoints in scripts which don't have a function context.

For example, create the following file tst_dbstop.m


x = 1;
y = 2;
z = 3;

x + y + z


and then try


dbstop ("tst_dbstop", 2)


This returns an empty matrix and the breakpoint is not set.

Rik <rik5>
Group administrator
Mon 16 May 2016 03:51:59 AM UTC, comment #2: 

I updated the patch slightly.  Instead of using 1 << 30 as the initial earliest_end, I used std::numeric_limits<int>::max ().  According to the C++ standard, an int is only guaranteed to be 16 bits wide.  One would probably never encounter such a machine these days, but it seemed cleaner not to have a hardcoded value.  I also removed the BIST test.  In order for octave to find the function nested, it would need to be called nested.m rather than using something from tmpname().  I don't think that's a problem.  Once we get this right it doesn't need to be continually tested.

I checked it in here (http://hg.savannah.gnu.org/hgweb/octave/rev/4bf980861fd6).  Marked as fixed and closing report.

Rik <rik5>
Group administrator
Sat 14 May 2016 12:04:48 AM UTC, comment #1: 

Confirmed.  I didn't check that your root cause analysis is correct, but you're right that the output should be 2 5 8 14.

Rik <rik5>
Group administrator
Fri 13 May 2016 11:39:39 PM UTC, original submission:  

To reproduce this bug, create the file nested.m containing


function nested
  a = 1;

  function inner
    b = 2;
  end

  d = 3;

end

function subfunction

  c = 3;
end


and issue the command


dbstop nested 1 4 7 11


The results should be 2 5 8 14, but instead is 2 8 8 0.

The reason is that add_breakpoint stops once it reaches a (sub)function containing a line number at least equal to the requested line.  That means it can only set breakpoints in a single (sub)function per invocation.

More importantly, because the "end" of an outer function is processed before a nested function is, breakpoints cannot be set in nested functions at all.

I'll post a patch once I have a bug number.  It first checks which (sub)function contains each requested line, and then walks only the selected function.

Lachlan Andrew <lachlan>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

 

Depends on the following items: None found

Digest:
   bug dependencies.

 

Carbon-Copy List
  • -email is unavailable- added by lachlan
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by lachlan (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 20 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-09-12 mtmiller Dependencies- bugs #49068 is dependent
    2016-07-16 lachlan Attached File- Added bug_47917_three_in_one.diff, #37906
    2016-07-15 lachlan Attached File- Added bug_47917_show_nested_functions.v2.cset, #37888
    2016-07-07 lachlan Attached File- Added bug_47917_find_nested_functions.cset, #37752
        Attached File- Added bug_47917_dbstatus_nested_functions.cset, #37753
        Attached File- Added bug_47917_show_nested_functions.cset, #37754
        Carbon-Copy- Added philipnienhuis
    2016-05-20 rik5 StatusPatch Submitted In Progress
    2016-05-20 rik5 Attached File- Added a.m, #37214
    2016-05-18 lachlan Attached File- Added bug_47917_dbstop_nested_recursive.cset, #37193
        StatusIn Progress Patch Submitted
    2016-05-17 lachlan Attached File- Added bug_47917_dbstop_nested.v2.cset, #37185
    2016-05-16 rik5 StatusFixed In Progress
        Open/ClosedClosed Open
    2016-05-16 rik5 StatusPatch Submitted Fixed
        Open/ClosedOpen Closed
    2016-05-14 lachlan Attached File- Added bug_47917_dbstop_nested.cset, #37150
        StatusConfirmed Patch Submitted
    2016-05-14 rik5 StatusNone Confirmed
        Assigned toNone lachlan

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code