From owner-svn-src-head@freebsd.org Thu Dec 1 01:32:14 2016 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 67AC6C5DDFA; Thu, 1 Dec 2016 01:32:14 +0000 (UTC) (envelope-from pfg@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4243B1986; Thu, 1 Dec 2016 01:32:14 +0000 (UTC) (envelope-from pfg@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id uB11WDSr046955; Thu, 1 Dec 2016 01:32:13 GMT (envelope-from pfg@FreeBSD.org) Received: (from pfg@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id uB11WD9v046953; Thu, 1 Dec 2016 01:32:13 GMT (envelope-from pfg@FreeBSD.org) Message-Id: <201612010132.uB11WD9v046953@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: pfg set sender to pfg@FreeBSD.org using -f From: "Pedro F. Giffuni" Date: Thu, 1 Dec 2016 01:32:13 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r309341 - head/usr.bin/indent X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 01 Dec 2016 01:32:14 -0000 Author: pfg Date: Thu Dec 1 01:32:13 2016 New Revision: 309341 URL: https://svnweb.freebsd.org/changeset/base/309341 Log: indent(1): Avoid out of bound access of array in_buffer Work-around a somewhat complex interaction within the code. From Piotr's commit [1]: When pr_comment() calls dump_line() for the first line of a multiline comment, it doesn't include any indentation - it starts with the "/*". This is consistent for both boxed and not boxed comments. Where the logic diverges is in how it treats the rest of the lines of the comment. For box comments indent assumes that it must not change anything, so lines are dumped as they were, including the indentation where it exists. For the rest of comments, it will first remove the indentation to store plain text of the comment and then add it again where indent thinks it's appropriate -- this is part of comment re-indenting process. For continuations of multi-line comments, the code that handles comments in dump_line() will use pad_output() to create indentation from the beginning of the line (what indent calls the first column) and then write string pointed by s_com afterwards. But if it's a box comment, the string will include original indentation, unless it's the first line of the comment. This is why tab characters from s_com have to be considered when calculating how much padding is needed and the "while (*com_st == '\t') com_st++, target += 8;" does that. In dump_line(), /target/ is initially set to ps.com_col, so it always assumes that indentation needs to be produced in this function, regardless of which line of a box comment it is. But for the first line of a box comment it is not true, so pr_comment() signals it by setting ps.n_comment_delta, the negative comment delta, to a negative number which is then added to /target/ in dump_line() on all lines except the first one, so that the function produces adequate indentation in this special case. The bug was in how that negative offset was calculated: pr_comment() used count_spaces() on in_buffer, which pr_comment() expected to contain non-null terminated sequence of characters, originating from whatever originally was on the left side of the comment. Understanding that count_spaces() requires a string, pr_comment() temporarily set buf_ptr[-2] to 0 in hope that it would nul-terminate the right thing in in_buffer and calling count_spaces() would be safe and do the expected thing. This was false whenever buf_ptr would point into save_com, an entirely different char array than in_buffer. The short-term fix is to recognize whether buf_ptr points into in_buffer or save_com. Reference: [1] https://github.com/pstef/freebsd_indent/commit/ea486a2aa3b056b146bdfbb8e94843159750f200 Taken from: Piotr Stefaniak Modified: head/usr.bin/indent/io.c head/usr.bin/indent/pr_comment.c Modified: head/usr.bin/indent/io.c ============================================================================== --- head/usr.bin/indent/io.c Wed Nov 30 22:20:23 2016 (r309340) +++ head/usr.bin/indent/io.c Thu Dec 1 01:32:13 2016 (r309341) @@ -225,8 +225,9 @@ dump_line(void) char *com_st = s_com; target += ps.comment_delta; - while (*com_st == '\t') - com_st++, target += 8; /* ? */ + while (*com_st == '\t') /* consider original indentation in + * case this is a box comment */ + com_st++, target += 8; while (target <= 0) if (*com_st == ' ') target++, com_st++; Modified: head/usr.bin/indent/pr_comment.c ============================================================================== --- head/usr.bin/indent/pr_comment.c Wed Nov 30 22:20:23 2016 (r309340) +++ head/usr.bin/indent/pr_comment.c Thu Dec 1 01:32:13 2016 (r309341) @@ -147,9 +147,16 @@ pr_comment(void) } } if (ps.box_com) { - buf_ptr[-2] = 0; - ps.n_comment_delta = 1 - count_spaces(1, in_buffer); - buf_ptr[-2] = '/'; + /* + * Find out how much indentation there was originally, because that + * much will have to be ignored by pad_output() in dump_line(). This + * is a box comment, so nothing changes -- not even indentation. + * + * The comment we're about to read usually comes from in_buffer, + * unless it has been copied into save_com. + */ + char *start = buf_ptr >= save_com && buf_ptr < save_com + sc_size ? bp_save : buf_ptr; + ps.n_comment_delta = 1 - count_spaces_until(1, in_buffer, start - 2); } else { ps.n_comment_delta = 0;