Bug 26869 - vi(1) crashes in viewing a file with long lines
Summary: vi(1) crashes in viewing a file with long lines
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 4.3-STABLE
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2001-04-26 06:40 UTC by alexs
Modified: 2007-02-21 10:31 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description alexs 2001-04-26 06:40:01 UTC
vi(1) starts to consume memory, after some time computer freezes and after
that core of process is dumped.

Fix: 

Have no time right now to make a fix, sorry
How-To-Repeat: 
Take file Eagle.xpm from the wdm port and type:
vi Eagle.xpm
:210
:set le
$
PgUp
PgUp
PgUp

soon editor freezes, in some time you will received 

Segmentation fault (core dumped)

Coredump can be obtained by request <g>.
Comment 1 ashp freebsd_committer freebsd_triage 2002-01-17 00:58:36 UTC
State Changed
From-To: open->feedback

I cannot reproduce this bug under 4.5-RC.  Is this still a problem for you 
with the latest -STABLE?
Comment 2 alex.neyman 2002-01-17 09:45:41 UTC
On 17 January 2002 03:59, ashp@FreeBSD.org wrote:
> I cannot reproduce this bug under 4.5-RC.  Is this still a problem
> for you with the latest -STABLE?

Yes, its still a problem. Reproduced on 4.5-RC cvsupped Jan 10, 2002.
It is easily reproduceable using the example in PR, it crashed 
everywhere I tried it.

-- 
<------------------------->
 ) May the Sun and Water (   Regards, Alexey V. Neyman
 ) always fall upon you! (   mailto:alex.neyman@auriga.ru
<------------------------->
Comment 3 Sheldon Hearn 2002-01-17 17:21:11 UTC
On Thu, 17 Jan 2002 01:50:02 PST, "Alexey V. Neyman" wrote:

>  > I cannot reproduce this bug under 4.5-RC.  Is this still a problem
>  > for you with the latest -STABLE?
>  
>  Yes, its still a problem. Reproduced on 4.5-RC cvsupped Jan 10, 2002.
>  It is easily reproduceable using the example in PR, it crashed 
>  everywhere I tried it.

There's a fix for a similar problem already in -CURRENT.  Please try the
following patch.  If it works, this is the same bug described in these
PRs:

	23912
	33648

If the patch doesn't work for you, you could also try the patch in PR
12801.

Ciao,
Sheldon.

Index: vs_relative.c
===================================================================
RCS file: /home/ncvs/src/contrib/nvi/vi/vs_relative.c,v
retrieving revision 1.1.1.1
retrieving revision 1.1.1.2
diff -u -d -r1.1.1.1 -r1.1.1.2
--- vs_relative.c	1 Nov 1996 06:45:33 -0000	1.1.1.1
+++ vs_relative.c	7 Jan 2002 14:26:12 -0000	1.1.1.2
@@ -111,6 +111,15 @@
 	int ch, leftright, listset;
 	char *p;
 
+	/*
+	 * Initialize the screen offset.
+	 */
+	scno = 0;
+
+	/* Leading number if O_NUMBER option set. */
+	if (O_ISSET(sp, O_NUMBER))
+		scno += O_NUMBER_LENGTH;
+
 	/* Need the line to go any further. */
 	if (lp == NULL) {
 		(void)db_get(sp, lno, 0, &lp, &len);
@@ -122,7 +131,7 @@
 	if (lp == NULL) {
 done:		if (diffp != NULL)		/* XXX */
 			*diffp = 0;
-		return (0);
+		return scno;
 	}
 
 	/* Store away the values of the list and leftright edit options. */
@@ -130,15 +139,10 @@
 	leftright = O_ISSET(sp, O_LEFTRIGHT);
 
 	/*
-	 * Initialize the pointer into the buffer and screen and current
-	 * offsets.
+	 * Initialize the pointer into the buffer and current offset.
 	 */
 	p = lp;
-	curoff = scno = 0;
-
-	/* Leading number if O_NUMBER option set. */
-	if (O_ISSET(sp, O_NUMBER))
-		scno += O_NUMBER_LENGTH;
+	curoff = 0;
 
 	/* Macro to return the display length of any signal character. */
 #define	CHLEN(val) (ch = *(u_char *)p++) == '\t' &&			\
Comment 4 alex.neyman 2002-01-17 17:45:03 UTC
On 17 January 2002 20:21, Sheldon Hearn wrote:
> There's a fix for a similar problem already in -CURRENT.  Please try
> the following patch.  If it works, this is the same bug described in
> these
> PRs:
> 
> 	23912
> 	33648
> 
> If the patch doesn't work for you, you could also try the patch in PR
> 12801.

Although the problem sounds similar, neither of these patches dispose 
of it.

-- 
<------------------------->
 ) May the Sun and Water (   Regards, Alexey V. Neyman
 ) always fall upon you! (   mailto:alex.neyman@auriga.ru
<------------------------->
Comment 5 Sheldon Hearn 2002-01-17 19:01:17 UTC
On Thu, 17 Jan 2002 20:45:03 +0300, "Alexey V. Neyman" wrote:

> Although the problem sounds similar, neither of these patches dispose 
> of it.

Okay, in that case the PR needs to remain in feedback state while you
liase with the nvi maintainers.  See http://bostic.com/vi/ for more
information.

Please copy your feedback to <bug-followup@freebsd.org>, using the
subject line of this message.

Ciao,
Sheldon.
Comment 6 alex.neyman 2002-01-18 00:41:41 UTC
Please try the following patch (I'll contact Keith Bostic later today). 

Index: vs_line.c
===================================================================
RCS file: /usr/local/cvsroot/src/contrib/nvi/vi/vs_line.c,v
retrieving revision 1.2
diff -u -r1.2 vs_line.c
--- vs_line.c	2 Aug 1998 15:18:44 -0000	1.2
+++ vs_line.c	18 Jan 2002 00:17:07 -0000
@@ -317,6 +317,8 @@
 	 * Don't fill anything in unless it's the right line and the right
 	 * character, and the right part of the character...
 	 */
+	if (sp->cno >= len)
+		sp->cno = len - 1;
 	if (yp == NULL ||
 	    smp->lno != sp->lno || sp->cno < offset_in_line ||
 	    offset_in_line + cols_per_screen < sp->cno) {

_5(?h
-- 
<------------------------->
 ) May the Sun and Water (   Regards, Alexey V. Neyman
 ) always fall upon you! (   mailto:alex.neyman@auriga.ru
<------------------------->
Comment 7 alex.neyman 2002-01-18 01:16:48 UTC
Damn, sent to freebsd-bugs@ instead of bug-followup@.

Index: vs_line.c
===================================================================
RCS file: /usr/local/cvsroot/src/contrib/nvi/vi/vs_line.c,v
retrieving revision 1.2
diff -u -r1.2 vs_line.c
--- vs_line.c	2 Aug 1998 15:18:44 -0000	1.2
+++ vs_line.c	18 Jan 2002 00:49:03 -0000
@@ -324,8 +324,11 @@
 		/* If the line is on the screen, quit. */
 		if (is_cached)
 			goto ret1;
-	} else
+	} else {
+		if (sp->cno >= len)
+			sp->cno = len - 1;
 		cno_cnt = (sp->cno - offset_in_line) + 1;
+	}
 
 	/* This is the loop that actually displays characters. */
 	ecbp = (cbp = cbuf) + sizeof(cbuf) - 1;


-- 
<------------------------->
 ) May the Sun and Water (   Regards, Alexey V. Neyman
 ) always fall upon you! (   mailto:alex.neyman@auriga.ru
<------------------------->
Comment 8 alex.neyman 2002-01-19 12:52:54 UTC
Forwarding to Audit-Trail.
The patch solves the problem for me either.

----------  Forwarded Message  ----------
Subject: Re: :set leftright fix? (FreeBSD PR 26869)
Date: Sat, 19 Jan 2002 13:30:15 +0100
From: Sven Verdoolaege <skimo@kotnet.org>
To: "Alexey V. Neyman" <alex.neyman@auriga.ru>
Cc: bostic@abyssinian.sleepycat.com


On Sat, Jan 19, 2002 at 08:08:00AM +0300, Alexey V. Neyman wrote:
> I agree that the correct fix would be to prevent sp->cno from being
> set to invalid value instead of adjusting it in vs_line(). However,
> its more a stopgap fix.

Well, the problem really is the 
	rp->cno = smp->c_sboff;
line in vs_smap.c
If no part of the line is currently visible, c_sboff will point
past the end of the line.

I thought I could just set cno to 0, because the latest posix
draft I have indicates that all the scolling command will move
to nonblank, but they don't mention the leftright option
and this is were the behaviour differs.
Nvi will move to the first position of the current horizontal
part of the screen in this case.

My current patch is appended below. Not really proud of it either.
I special case offset in character 255 to indicate that there
is no character on the screen and in that case we go to the
first nonblank even with the leftright option set.
(The patch should apply against 1.79 with some offsets).

> 
> > Have you tried 1.81.5 ?
> 
> No, I haven't. As far as I know it requires widechar support which is 
> not in freebsd-stable.

You have been misinformed.

skimo

Index: vs_line.c
===================================================================
RCS file: /b/CVSROOT/vi/vi/vs_line.c,v
retrieving revision 10.37
diff -u -r10.37 vs_line.c
--- vs_line.c	2001/08/30 13:56:07	10.37
+++ vs_line.c	2002/01/19 12:18:57
@@ -273,7 +273,10 @@
 		cols_per_screen = sp->cols;
 
 		/* Put starting info for this line in the cache. */
-		if (scno != skip_cols) {
+		if (offset_in_line >= len) {
+			smp->c_sboff = offset_in_line;
+			smp->c_scoff = 255;
+		} else if (scno != skip_cols) {
 			smp->c_sboff = offset_in_line;
 			smp->c_scoff =
 			    offset_in_char = chlen - (scno - skip_cols);
Index: vs_smap.c
===================================================================
RCS file: /b/CVSROOT/vi/vi/vs_smap.c,v
retrieving revision 10.29
diff -u -r10.29 vs_smap.c
--- vs_smap.c	2001/06/25 15:19:38	10.29
+++ vs_smap.c	2002/01/19 12:18:57
@@ -727,7 +727,7 @@
 	if (!SMAP_CACHE(smp) && vs_line(sp, smp, NULL, NULL))
 		return (1);
 	rp->lno = smp->lno;
-	rp->cno = smp->c_sboff;
+	rp->cno = smp->c_scoff == 255 ? 0 : smp->c_sboff;
 	return (0);
 }
 
@@ -941,7 +941,7 @@
 	if (!SMAP_CACHE(smp) && vs_line(sp, smp, NULL, NULL))
 		return (1);
 	rp->lno = smp->lno;
-	rp->cno = smp->c_sboff;
+	rp->cno = smp->c_scoff == 255 ? 0 : smp->c_sboff;
 	return (0);
 }
 


-------------------------------------------------------

-- 
<------------------------->
 ) May the Sun and Water (   Regards, Alexey V. Neyman
 ) always fall upon you! (   mailto:alex.neyman@auriga.ru
<------------------------->
Comment 9 Sheldon Hearn freebsd_committer freebsd_triage 2002-01-31 13:57:51 UTC
State Changed
From-To: feedback->analyzed

Committed in -CURRENT, with plans to MFC to -STABLE in a month. 


Comment 10 Sheldon Hearn freebsd_committer freebsd_triage 2002-01-31 13:57:51 UTC
Responsible Changed
From-To: freebsd-bugs->sheldonh

I have to remember to MFC this.
Comment 11 matt 2002-06-13 22:54:25 UTC
What an obscure problem!

The problem can easily be repeated for any file that has at least 10 (or so)
lines less than 80 characters followed by 10 (or so) lines greater than 80
characters.  XPM files are perfect for this.

When the "cursor" is on the first >80 character line, with left/right
scrolling on, and positioned at the end of the line, a pageup operation
(into the <80 line region) will coredump.  However, lineup operations will
not.  Very strange.

The coredump is caused by vs_refresh() getting in an endless loop and
running the system out of stack space (and/or other memory resources).  If
re-compiled with -DDEBUG, vi will abort() immediately after the failing
pageup, thanks to a failed sanity check in vs_refresh() that ensures that a
valid cursor value returned by vs_line().

I've taken a look at the nvi-devel sources (1.81.4) and can't see anything
immediately obvious that would indicate that this bug has been fixed.
However, since I'm not running -CURRENT, I haven't been able to actually
test this out.

I'll keep on hacking away at this.

--
Matt Emmerton
Comment 12 Sheldon Hearn freebsd_committer freebsd_triage 2004-02-17 11:19:07 UTC
Responsible Changed
From-To: sheldonh->freebsd-bugs

Release.
Comment 13 Sheldon Hearn freebsd_committer freebsd_triage 2004-02-17 11:19:07 UTC
Responsible Changed
From-To: sheldonh->freebsd-bugs

Release.
Comment 14 lists 2005-02-15 00:17:37 UTC
Hello,

following up on old PRs.

http://www.FreeBSD.org/cgi/query-pr.cgi?pr=26869
| How-To-Repeat
| 
| Take file Eagle.xpm from the wdm port and type:
| vi Eagle.xpm
| :210
| :set le
| $
| PgUp
| PgUp
| PgUp
| 
| soon editor freezes, in some time you will received 
| 
| Segmentation fault (core dumped)

Doesn't do anything nasty on my 5.3-STABLE.


| State-Changed-From-To: feedback->analyzed 
| State-Changed-By: sheldonh 
| State-Changed-When: Thu Jan 31 05:57:51 PST 2002 
| State-Changed-Why:  
| Committed in -CURRENT, with plans to MFC to -STABLE in a month. 

Seems this PR can be closed now. Could anyone take care of that,
please?
 
Thanks!
Mario
Comment 15 Remko Lodder freebsd_committer freebsd_triage 2007-02-21 10:31:44 UTC
State Changed
From-To: analyzed->closed

There are reports that this had been resolved, close the ticket