bugGNU nano - Bugs: bug #51770, nano locks up when tab size is...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #51770: nano locks up when tab size is larger than screen width

Submitter:  David Lawrence Ramsey <dolorous>
Submitted:  Wed 16 Aug 2017 10:14:30 PM UTC
   
 
Severity:  5 - Blocker Status:  Fixed
Assigned to:  dolorous Open/Closed:  Closed

Jump to the original submission

Tue 22 Aug 2017 02:58:06 PM UTC, comment #12: 

The problem was not limited to git: 2.8.6 will immediately lock up whenever the tabsize is ten percent larger than the screen.

Benno Schulenberg <bens>
Group administrator
Thu 17 Aug 2017 07:38:23 PM UTC, comment #11: 

Argh, no, the lockup from the other bug was caused by my previously incorrect fix for it.  (All these tab issues are making me a bit confused.)  I guess this bug does occur in 2.8.6 after all, but no matter, it's being fixed.

David Lawrence Ramsey <dolorous>
Group Member
Thu 17 Aug 2017 07:33:00 PM UTC, comment #10: 

Good to know on your terminal size.  I think that lockup may be the other bug, which I'll report momentarily (I haven't built 2.8.6 with debugging info to check with gdb).

David Lawrence Ramsey <dolorous>
Group Member
Thu 17 Aug 2017 07:16:30 PM UTC, comment #9: 

Well, I've just tried with 2.8.6, and nano locks up immediately after the first <Left> or <Right> when running:

  src/nano -T181 +2 NEWS --ignore

(I use a 90-column terminal by default).

Benno Schulenberg <bens>
Group administrator
Thu 17 Aug 2017 06:58:20 PM UTC, comment #8: 

Yes, that one.

David Lawrence Ramsey <dolorous>
Group Member
Thu 17 Aug 2017 06:24:41 PM UTC, comment #7: 

You mean bug #51671, right?  The support for two-column spaces.

Benno Schulenberg <bens>
Group administrator
Thu 17 Aug 2017 06:05:38 PM UTC, comment #6: 

Thank you.

This particular bug doesn't occur in nano 2.8.6, although overly long tabs do still make it malfunction in a different way.

(I'll file a bug report for that next, and I'm working on a fix for that.  For the record, the fix will be much easier to apply if the patch in bug #51761 is applied first, since the patch simplifies things to the point where chunk-straddling tabs are handled in only one place instead of two.  But that's a separate issue.)

David Lawrence Ramsey <dolorous>
Group Member
Thu 17 Aug 2017 05:50:00 PM UTC, comment #5: 

Thanks for the patches.  The final fix has been pushed as f08d169a.  The correction for <Left> and <Right> I have implemented a bit differently, in commit cf14ee2e.

If I understand well, the bug does not occur in 2.8.6?
(I don't have time to test at the moment.)

Benno Schulenberg <bens>
Group administrator
Thu 17 Aug 2017 04:41:35 PM UTC, comment #4: 

The other problem is that do_left() and do_right() are calling chunk_for() regardless of whether we're in softwrap mode or not, which is definitely an oversight.  The attached new patch fixes that.

Although the first patch is still needed; if I follow your reproduction recipe in comment #3, but add --softwrap after --constant, the lockup still occurs; with the first patch applied, it doesn't.  (There are other oddities with such long tabs in softwrap mode, but those are separate issues I'm working on.)

(file #41555)

David Lawrence Ramsey <dolorous>
Group Member
Thu 17 Aug 2017 04:16:39 PM UTC, comment #3: 

To be clear, my reproduction recipe is:

  stty cols 80 && src/nano --ignore --constant -T161 +2 NEWS

and then press <Right> three times.  (Two rights are fine, and you can then do <Up>s and <Down>s without a problem, but a <Left> or <Right> will result in a lockup.)

Benno Schulenberg <bens>
Group administrator
Thu 17 Aug 2017 04:05:15 PM UTC, comment #2: 

The patch is effective.  But... it strikes me as absurd: why does get_softwrap_breakpoint() get called when we're not in softwrap mode?  Where does that happen?

Benno Schulenberg <bens>
Group administrator
Thu 17 Aug 2017 02:54:04 AM UTC, comment #1: 

Another way to reproduce it is as follows:

1. Start nano in an 80x24 terminal.

2. Run:

./nano --ignore --softwrap

(The tab size is thus left at the default of 8.)

3. In the empty buffer, press Tab to add a tab of that width.

4. Resize the terminal horizontally to 1 column wide, then resize it again to 80 columns wide.  nano will lock up; according to gdb, it's stuck parsing characters via parse_mbchar() in get_softwrap_breakpoint() (at the place in the latter after is's moved forward to a chunk after the first one, where it's moving through the chunk, winio.c, line 3018).

The same experimental patch fixes this.  In this case, the difference with it applied is that leftedge progresses properly from 0 to 0 to 1 to 7 to 8; without it applied, the leftedge progresses from 0 to 0 to 7 to 8 and then loops back to 0.

David Lawrence Ramsey <dolorous>
Group Member
Wed 16 Aug 2017 10:14:30 PM UTC, original submission:  

This can be reproduced as follows:

1. Start nano in an 80x24 terminal.

2. Run:

./nano --ignore -T 161

(This seems to be the minimum size to trigger it on an 80-column terminal, since it's just over two full rows wide.)

3. In the empty buffer, press Tab to add a tab of that width.

4. Press Left.  nano will lock up; according to gdb, it's stuck parsing characters via parse_mbchar() in get_softwrap_breakpoint() (at the place in the latter where it's moving forward to a chunk after the first one, winio.c, line 3004).

This also occurs with bigger tabs (for example, if you use -T 900 above) and regardless of whether you use --softwrap.  Although if you use -T 900 or --softwrap, the lockup will occur as soon as you press Tab, instead of when you try to move left afterwards.

In the process of trying to debug this, I've found that the attached experimental patch seems to fix it.  I'm not entirely sure why.  The only difference with it applied is that, in get_softwrap_breakpoint() with a 161-column tab, leftedge progresses properly from 0 to 80 to 160; without it applied, leftedge progresses from 0 to 80 and gets stuck at 80.  Maybe column isn't being properly added to in all cases without the patch?

(Additionally, even with the patch applied and the problem fixed, softwrap navigation of such huge tabs is problematic, because of the usual combined problem where softwrap navigation assumes you're moving a line or less, the tab takes up more than a line, and the cursor can't be placed in the middle of the tab.  I have no idea what to do about this, however, and it's a separate issue.)

David Lawrence Ramsey <dolorous>
Group Member

 

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

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by bens (Posted a comment)
  • -email is unavailable- added by dolorous (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.

     

    Follow 10 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-08-28 bens Open/ClosedOpen Closed
    2017-08-22 bens Severity4 - Important 5 - Blocker
        Summarywith current git, nano can lock up with overly large tab sizes relative to screen width nano locks up when tab size is larger than screen width
    2017-08-17 bens Severity3 - Normal 4 - Important
    2017-08-17 bens StatusIn Progress Fixed
        Assigned toNone dolorous
    2017-08-17 dolorous Attached File- Added 0001-weeding-don-t-call-chunk_for-outside-of-softwrap-mod.patch, #41555
    2017-08-17 bens StatusNone In Progress
    2017-08-17 dolorous Summarywith current git, nano can lock up with overly large tab sizes with current git, nano can lock up with overly large tab sizes relative to screen width
    2017-08-16 dolorous Attached File- Added fix-tab-lockup.patch, #41549

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code