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

 
 

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

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

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)

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>
Project 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>
Project AdministratorIn charge of this item.
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>
Project AdministratorIn charge of this item.
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>
Project Administrator
Thu 17 Aug 2017 06:58:20 PM UTC, comment #8:

Yes, that one.

David Lawrence Ramsey <dolorous>
Project AdministratorIn charge of this item.
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>
Project 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>
Project AdministratorIn charge of this item.
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>
Project 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>
Project AdministratorIn charge of this item.
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>
Project 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>
Project 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>
Project AdministratorIn charge of this item.
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>
Project AdministratorIn charge of this item.

 

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

Attach File(s):
   
   
Comment:
   

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by bens (Posted a comment)
  • -unavailable- added by dolorous (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 logged-in users can vote.

     

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

     

     

    Follow 10 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Mon 28 Aug 2017 07:10:06 PM UTCbensOpen/ClosedOpen=>Closed
    Tue 22 Aug 2017 02:58:06 PM UTCbensSeverity4 - 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
    Thu 17 Aug 2017 07:16:30 PM UTCbensSeverity3 - Normal=>4 - Important
    Thu 17 Aug 2017 05:50:00 PM UTCbensStatusIn Progress=>Fixed
      Assigned toNone=>dolorous
    Thu 17 Aug 2017 04:41:35 PM UTCdolorousAttached File-=>Added 0001-weeding-don-t-call-chunk_for-outside-of-softwrap-mod.patch, #41555
    Thu 17 Aug 2017 04:05:15 PM UTCbensStatusNone=>In Progress
    Thu 17 Aug 2017 02:54:04 AM UTCdolorousSummarywith 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
    Wed 16 Aug 2017 10:14:30 PM UTCdolorousAttached File-=>Added fix-tab-lockup.patch, #41549

    Back to the top


    Powered by Savane 3.1-cleanup1