bugGNU Octave - Bugs: bug #35772, Nested Function Support

 
 

bug #35772: Nested Function Support

Submitted by:  Max Brister <fisheater>
Submitted on:  Fri 09 Mar 2012 01:42:12 PM UTC  
 
Category: InterpreterSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Feature Request
Status: FixedAssigned to: None
Originator Name: Max BristerOpen/Closed: Closed
Release: devOperating System: Any

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Sat 31 Mar 2012 06:48:07 AM UTC, comment #14:

jwe,

I have created a new version of the patch, nest3.patch, as requested. While creating the changelog I removed some pointless edits as well as fixing the expected error in a test case.

Thanks,
Max Brister

(file #25523)

Max Brister <fisheater>
Project Member
Fri 30 Mar 2012 09:53:35 PM UTC, comment #13:

Max,

Could you please take the new version of the patch that I've attached and add ChangeLog info to the commit message and resubmit it?

Thanks.

(file #25522)

John W. Eaton <jwe>
Project Administrator
Sun 18 Mar 2012 07:23:02 AM UTC, comment #12:

clear all is now bug 35881. segmentation fault wasn't reported - there seem to be quite a lot of them already, I wasn't sure without an extensive look if it won't be a duplicate.

Alon

Alon Levy <alon>
Sun 18 Mar 2012 07:11:45 AM UTC, comment #11:

Jordi, Max,

Thanks for pointing that out, I'll open separate bug reports.

Alon

Alon Levy <alon>
Sun 18 Mar 2012 01:00:16 AM UTC, comment #10:

I'm just saying that UB is UB and Max's patch is simply bringing out the UB in another part of the code.

Jordi GutiƩrrez Hermoso <jordigh>
Project Administrator
Sun 18 Mar 2012 12:53:20 AM UTC, comment #9:

Jordi,

This also happens on octave 3.6.1, and 14479:05b59be209ed without nest2.patch. I do not think this issue has anything to do with the nest2.patch. valgrind also shows this happening in a different place as Alon's issue. Maybe this belongs in another bug report? It does look quite bad.

Alon,
It looks like you have two related issues.

1) The crash
I am able to reproduce your crash on revision 05b59be209ed and octave 3.6.1. As nest2.patch has not been applied to either of these, this seems to be some underlying interpreter issue. Interestingly, there is no crash (for me at least) if I place the code in a file before executing.

2) Incorrect behavior when clear all is called
When looking into the bug further it appears that this is an issue with subfunctions as well. For example,
produces
when executed in octave 3.6.1 and revision 05b59be209ed. This leads me to believe that the bug is a related bug having to do with executing clear all when subfunctions exist. Fixing this problem for subfunctions should also fix the issue for nested functions.

I am willing to look into both of these issues, but I think they belong in a separate bug report.

Max

Max Brister <fisheater>
Project Member
Sat 17 Mar 2012 11:24:30 PM UTC, comment #8:

At revision 14479:05b59be209ed, when running

I get what looks like relevant output:

Jordi GutiƩrrez Hermoso <jordigh>
Project Administrator
Sat 17 Mar 2012 10:21:42 PM UTC, comment #7:

Hi Max,

I've tested your patch, I found two issues so far:

1. clear all removes the nested functions. Example that fails with unresolved function sub:
function f();
# 1
function sub();
endfunction
# 2
clear all
sub() # fails, unresolved symbol
endfunction

f()
error: `sub' undefined near line 5 column 1
error: called from:
error: f at line 5, column 1

2. segmentation fault with an almost identical code to the above (just moving clear all from #2 to #1):

function f();
# 1
clear all
function sub();
endfunction
# 2
sub() # fails, unresolved symbol
endfunction

f()

#0 0x0000000000000041 in ?? ()
#1 0x00007fd910b0c1a1 in tree_evaluator::visit_function_def (this=<optimized out>, cmd=...) at ../../src/pt-eval.cc:515
#2 0x00007fd910b0b525 in tree_evaluator::visit_statement (this=0x7fd911224bf8, stmt=...) at ../../src/pt-eval.cc:706
#3 0x00007fd910b0ad0e in visit_statement_list (lst=..., this=0x7fd911224bf8) at ../../src/pt-eval.cc:772
#4 tree_evaluator::visit_statement_list (this=0x7fd911224bf8, lst=...) at ../../src/pt-eval.cc:753
#5 0x00007fd910a92013 in octave_user_function::do_multi_index_op (this=0x1104e00, nargout=0, args=..., lvalue_list=<optimized out>) at ../../src/ov-usr-fcn.cc:479
#6 0x00007fd910a8f1ff in octave_user_function::subsref (this=0x1104e00, type="(", idx=Python Exception <type 'exceptions.IndexError'> list index out of range:
std::list, nargout=0, lvalue_list=lvalue_list@entry=0x0) at ../../src/ov-usr-fcn.cc:329
#7 0x00007fd910a8f3cc in octave_user_function::subsref (this=<optimized out>, type=..., idx=..., nargout=<optimized out>) at ../../src/ov-usr-fcn.cc:312
#8 0x00007fd910a9c638 in octave_value::subsref (this=<optimized out>, type=..., idx=..., nargout=<optimized out>) at ../../src/ov.cc:1203
#9 0x00007fd910a9c6a5 in octave_value::subsref (this=<optimized out>, type=..., idx=..., nargout=<optimized out>, lvalue_list=<optimized out>) at ../../src/ov.cc:1214
#10 0x00007fd910b132eb in tree_index_expression::rvalue (this=<optimized out>, nargout=0, lvalue_list=lvalue_list@entry=0x0) at ../../src/pt-idx.cc:414
#11 0x00007fd910b13a3b in tree_index_expression::rvalue (this=<optimized out>, nargout=<optimized out>) at ../../src/pt-idx.cc:284
#12 0x00007fd910b114b8 in tree_index_expression::rvalue1 (this=0x11026a0, nargout=0) at ../../src/pt-idx.cc:425
#13 0x00007fd910b0b5f4 in tree_evaluator::visit_statement (this=0x7fd911224bf8, stmt=...) at ../../src/pt-eval.cc:736
#14 0x00007fd910b0ad0e in visit_statement_list (lst=..., this=0x7fd911224bf8) at ../../src/pt-eval.cc:772
#15 tree_evaluator::visit_statement_list (this=0x7fd911224bf8, lst=...) at ../../src/pt-eval.cc:753
#16 0x00007fd9109aed34 in main_loop () at ../../src/toplev.cc:595
#17 0x00007fd91095689f in octave_main (argc=1, argv=<optimized out>, embedded=0) at ../../src/octave.cc:943
#18 0x00000039c2421735 in __libc_start_main (main=0x400750 <main>, argc=1, ubp_av=0x7fff71c7b338, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_e
nd=0x7fff71c7b328) at libc-start.c:226
#19 0x0000000000400781 in _start ()

I think this backtrace is almost useless but I need to rebuild with -O0 to have any parameters. It's just showing a fault during evaluation of the user function.

Other then that I do need nested function support :(

Thanks for all the great work,
Alon

Alon Levy <alon>
Sat 17 Mar 2012 09:15:03 PM UTC, comment #6:

The revised nested function patch has been completed. It should be much more stable, however it has the following issues:
- handles to nested functions are not supported.
- errors are not properly issued if a nested child declares a nonlocal variable as global or persistent. (should not cause a crash though, and existing matlab code should not be affected by this)

I will post on the mailing list explaining the reasons behind the approach I used.

Alon,
The bug you found is probably due to the nested function patch. It is amazing how many interesting errors can be caused by changing the core interpreter functionality.

Max

(file #25387)

Max Brister <fisheater>
Project Member
Sat 17 Mar 2012 01:46:28 AM UTC, comment #5:

Great.

I just found another bug but not really related, although it could be at the same code area, just thought you might be interested but really I'm writing it so as to not forget :)

If you run the following line multiple times:
- with mercurial head plus your old patch it fails after about 5 times with memory corruption (glibc double linked list corrupted, stack trace not interesting so I won't post, also this is the wrong bug to post it on..)
- with 3.6.1 (fedora latest on fedora 17) it doesn't fail immediately even after 40 invocations, but fails when I quit the interpreter, also with double linked list corruption
- on both occasions the failure is inside get_cwd, which is why I keep repeating it probably has nothing to do with nested functions.

The magic line to repeat to get a crash immediately or on quiting the interpreter:

function f(); function t(x); x=5; disp(x); endfunction; x=7; t(7); endfunction; f()

(no EOLs in there, just a single line)

Happy Hacking,
Alon

Alon Levy <alon>
Sat 17 Mar 2012 01:29:25 AM UTC, comment #4:

Yes, I forgot to take into account formal parameters correctly in my initial patch. I now have test cases for this (as well as several other things I forgot to take into account previously). I should be done with the revised patch either tonight or early tomorrow morning.

Max

Max Brister <fisheater>
Project Member
Sat 17 Mar 2012 01:24:51 AM UTC, comment #3:

OK, great to know. So you might want to use the following test, your first patch fails it: (fails the first, second is same just using a rename to pinpoint the error - it seems the nested f redefined x, and in your do_update_nested I think you unmark x as nonlocal, but forget to mark it back as normal (or maybe the unmark is not being called), thereby causing it to be unresolved in the parent function (just a theory, didn't check yet - and if your revised patch would be available in a day or two I will patiently wait :)

octave:1> function test(x); function f(x); endfunction; f(x); disp(x); endfunction
octave:2> test(11)
error: `x' undefined near line 1 column 58
error: evaluating argument list element number 1
error: called from:
error: test at line 1, column 53
octave:2> function test(x); function f(y); endfunction; f(x); disp(x); endfunction
octave:3> test(11)
11

Alon Levy <alon>
Sat 17 Mar 2012 01:19:21 AM UTC, comment #2:

Alon,

I am currently working on a revised patch. The revised patch takes a slightly different implementation approach, and should be done soon. The revised patch should also fix several formatting/stylistic issues.

Max

Max Brister <fisheater>
Project Member
Sat 17 Mar 2012 12:55:54 AM UTC, comment #1:

Hi Max,

Thanks very much for writing this, I'm testing it right now. Some whitespace got into your patch in finish_function, maybe you'd like to know:

+ else
+ { <<<<<< white space
+ symbol_table::install_subfunction (nm, octave_value (fcn),
+ primary_fcn_scope);

Alon

Alon Levy <alon>
Fri 09 Mar 2012 01:42:12 PM UTC, original submission:

Currently octave coerces nested functions into subfunctions. I have developed a patch which should fix this.

Max Brister <fisheater>
Project Member

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #25523:  nest3.patch added by fisheater (43KiB - text/x-patch)
file #25522:  nested-functions-patch.txt added by jwe (38KiB - text/plain)
file #25387:  nest2.patch added by fisheater (40KiB - text/x-patch)
file #25307:  nest.patch added by fisheater (22KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by rik5 (Updated the item)
  • -unavailable- added by jwe (Updated the item)
  • -unavailable- added by jordigh (Posted a comment)
  • -unavailable- added by alon (Posted a comment)
  • -unavailable- added by fisheater (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 8 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 11 Apr 2012 06:11:44 AM UTCrik5StatusPatch Submitted=>Fixed
      Open/ClosedOpen=>Closed
    Sat 31 Mar 2012 06:48:07 AM UTCfisheaterAttached File-=>Added nest3.patch, #25523
    Fri 30 Mar 2012 09:53:35 PM UTCjweAttached File-=>Added nested-functions-patch.txt, #25522
    Sun 18 Mar 2012 04:44:07 PM UTCrik5Carbon-CopyRemoved 72865=>-
    Sat 17 Mar 2012 09:15:03 PM UTCfisheaterAttached File-=>Added nest2.patch, #25387
    Fri 09 Mar 2012 05:39:32 PM UTCrik5StatusNone=>Patch Submitted
    Fri 09 Mar 2012 01:42:12 PM UTCfisheaterAttached File-=>Added nest.patch, #25307

    Back to the top


    Powered by Savane 3.1-cleanup1