Mon 16 May 2016 04:18:22 AM UTC, comment #10:
No comments, so I went ahead and applied the second solution from comment #9. See http://hg.savannah.gnu.org/hgweb/octave/rev/764e586dca4b.
Fixed, closing report.
|
Thu 12 May 2016 12:46:30 AM UTC, comment #9:
It isn't so hard to do. One way to eliminate the variable trailing context is to have explicit rules. This would catch the majority of instances, and is probably good enough given the 80/20 rule. For example, catching "get|set" followed by 0,1,2 spaces and a then a parenthesis would be:
I've tested this and it works.
Alternatively, remove the trailing context from the Flex pattern, but then you need to strip out the whitespace yourself.
This pattern works to stop the segfault.
However, the HANDLE_IDENTIFIER macro needs to be changed since the whitespace is now included in yytext.
I changed the code clause where get_set is true to this
and it works ("make check" passes).
|
Tue 10 May 2016 09:51:11 PM UTC, comment #8:
The REJECT feature is not used explicitly, but is triggered by this rule in lex.ll:
The problem is the variable-length trailing context (the whitespace followed by an open paren).
If we can change the way that get and set are handled for classdef methods like "get.property_name" to avoid using this trailing context, then this problem should go away.
There is one more use of trailing context in the lexer, but it doesn't involve variable length matching, so it isn't a problem.
Probably we just need a completely different way of recognizing these get/set identifiers in this context.
|
Mon 14 Sep 2015 08:02:59 AM UTC, comment #7:
If our scanner/lexer can't be modified to parse larger strings (e.g. not using REJECT as explained here [1]), at least (1) parsing a too large string should not lead to a segfault, (2) the manual should mention this limitation.
[1] http://flex.sourceforge.net/manual/Diagnostics.html#Diagnostics
|
Mon 14 Sep 2015 05:55:58 AM UTC, comment #6:
Savepath is easy to fix (see comment #1) but the segfault whith long strings is more serious. From a quick search, I found that this is a lexer issue that e.g. Doxygen also has (same limit of 16000 characters see [1]).
[1] see item 13: http://www.star.bnl.gov/public/comp/sofi/doxygen/faq.html
|
Sun 13 Sep 2015 10:59:53 PM UTC, comment #5:
Yes, see bug #45308 for my motivation for opening this bug. A novice user may not even know about the existence of ~/.octaverc, and may not know exactly what path is set to when savepath is called, but it could inadvertently "break" their Octave installation without any indication of what went wrong.
|
Sun 13 Sep 2015 09:31:07 PM UTC, comment #4:
Does anybody really use this function anyways? We have to have it for Matlab compatibility, but I don't think we need to spend much time perfecting it. A path of over 26,000 characters seems to me to be a problem the user should solve, rather than Octave.
|
Sun 13 Sep 2015 09:05:48 PM UTC, comment #3:
A simpler way to reproduce the same segfault:
For me it crashes above n of order 1e7.
|
Sun 13 Sep 2015 08:35:32 PM UTC, comment #2:
And for the segfault, I tried to copy the monolithic path in .octaverc from an editor, and paste it (around 26400 characters according to LibreOffice) in the terminal between two quotes and it also crashed Octave with the same error messages.
So it looks like long string inputs are not well treated in the terminal either.
|
Sun 13 Sep 2015 10:41:54 AM UTC, comment #1:
I can confirm this behavior.
Why not add each path entry individually (something like the attached patch)? Even though startup time is large, it works.
(file #34868)
|
Sat 12 Sep 2015 07:31:28 PM UTC, original submission:
The savepath() function stores the current load path in ~/.octaverc, but it stores the path as one long command. If the user's currently configured load path is huge, this will crash Octave on startup.
Test case:
Now exit and start Octave again. Octave crashes when trying to execute the long savepath command line with:
|