[Bug fortran/66528] New: unbalanced IF/ENDIF with -fmax-errors=1 causes invalid free

[Bug fortran/66528] New: unbalanced IF/ENDIF with -fmax-errors=1 causes invalid free

[tkoenig at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66528

            Bug ID: 66528
           Summary: unbalanced IF/ENDIF with -fmax-errors=1 causes invalid
                    free
           Product: gcc
           Version: 6.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: tkoenig at gcc dot gnu.org
  Target Milestone: ---

May be related to PR 66175

ig25@linux-fd1f:~/Krempel/Tree> gfortran -fmax-errors=1 unbal.f90 
unbal.f90:5:5:

   end
     1
Error: END IF-Anweisung bei (1) erwartet
Kompilierung wegen -fmax-errors=1 beendet.
*** Error in `/home/ig25/lib/gcc/x86_64-unknown-linux-gnu/6.0.0/f951': free():
invalid pointer: 0x0000000001cfefa8 ***
======= Backtrace: =========
/lib64/libc.so.6(+0x7364f)[0x7fb028db564f]
/lib64/libc.so.6(+0x78eae)[0x7fb028dbaeae]
/home/ig25/lib/gcc/x86_64-unknown-linux-gnu/6.0.0/f951(_Z17diagnostic_finishP18diagnostic_context+0x5c)[0x126b11c]
/home/ig25/lib/gcc/x86_64-unknown-linux-gnu/6.0.0/f951(_Z30diagnostic_action_after_outputP18diagnostic_context12diagnostic_t+0x1a2)[0x126bf12]
/home/ig25/lib/gcc/x86_64-unknown-linux-gnu/6.0.0/f951(_Z15gfc_error_checkv+0x6c)[0x63fb0c]
/home/ig25/lib/gcc/x86_64-unknown-linux-gnu/6.0.0/f951[0x687e48]
/home/ig25/lib/gcc/x86_64-unknown-linux-gnu/6.0.0/f951[0x689741]
/home/ig25/lib/gcc/x86_64-unknown-linux-gnu/6.0.0/f951[0x68ba0d]
/home/ig25/lib/gcc/x86_64-unknown-linux-gnu/6.0.0/f951[0x68c431]
/home/ig25/lib/gcc/x86_64-unknown-linux-gnu/6.0.0/f951[0x68ca07]
/home/ig25/lib/gcc/x86_64-unknown-linux-gnu/6.0.0/f951(_Z14gfc_parse_filev+0x848)[0x68e168]
/home/ig25/lib/gcc/x86_64-unknown-linux-gnu/6.0.0/f951[0x6ce643]
/home/ig25/lib/gcc/x86_64-unknown-linux-gnu/6.0.0/f951[0xbca44f]
/home/ig25/lib/gcc/x86_64-unknown-linux-gnu/6.0.0/f951(_ZN6toplev4mainEiPPc+0x73d)[0x61033d]
/home/ig25/lib/gcc/x86_64-unknown-linux-gnu/6.0.0/f951(main+0x2a)[0x6126ea]
/lib64/libc.so.6(__libc_start_main+0xf5)[0x7fb028d63be5]
/home/ig25/lib/gcc/x86_64-unknown-linux-gnu/6.0.0/f951[0x612ad5]
======= Memory map: ========

[Bug fortran/66528] unbalanced IF/ENDIF with -fmax-errors=1 causes invalid free

[tkoenig at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66528

--- Comment #1 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
Much better with a test case, of course.

program main
  read (*,*) n
  if (n<0) then
    print *,foo
  end
    print *,bar
end program main

[Bug fortran/66528] [6 Regression] unbalanced IF/ENDIF with -fmax-errors=1 causes invalid free

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66528

Dominique d'Humieres <dominiq at lps dot ens.fr> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P4
             Status|UNCONFIRMED                 |NEW
      Known to work|                            |5.1.0
           Keywords|                            |error-recovery
   Last reconfirmed|                            |2015-06-13
                 CC|                            |pault at gcc dot gnu.org
     Ever confirmed|0                           |1
            Summary|unbalanced IF/ENDIF with    |[6 Regression] unbalanced
                   |-fmax-errors=1 causes       |IF/ENDIF with
                   |invalid free                |-fmax-errors=1 causes
                   |                            |invalid free
      Known to fail|                            |6.0

--- Comment #2 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
> Much better with a test case, of course.

Indeed!-) Revision r223447 (2015-05-20) gives

Error: END IF statement expected at (1)
compilation terminated due to -fmax-errors=1.

Revision r223694 (2015-05-26) gives

compilation terminated due to -fmax-errors=1.
f951(2326,0x7fff7bef0300) malloc: *** error for object 0x1412fd9a8: pointer
being freed was not allocated
*** set a breakpoint in malloc_error_break to debug
(null):0: confused by earlier errors, bailing out
f951(2326,0x7fff7bef0300) malloc: *** error for object 0x142002200: pointer
being freed was not allocated
*** set a breakpoint in malloc_error_break to debug

Usual suspect r223677 (pr66082).

[Bug fortran/66528] [6 Regression] unbalanced IF/ENDIF with -fmax-errors=1 causes invalid free

[manu at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66528

--- Comment #4 from Manuel López-Ibáñez <manu at gcc dot gnu.org> ---
(In reply to Thomas Koenig from comment #3)
> (In reply to Dominique d'Humieres from comment #2)
> 
> > Usual suspect r223677 (pr66082).
> 
> I don't believe that a change to trans-array.c can cause
> a parsing failure.  I would rather suspect r223614 .

Yes, this is my fault. diagnostic_finish tries to free the output_buffer, but
the error_buffer is statically allocated. I think this should be enough:

--- error.c     (revision 223651)
+++ error.c     (working copy)
@@ -1379,12 +1379,12 @@ gfc_error_check (void)
       output_buffer *tmp_buffer = pp->buffer;
       pp->buffer = pp_error_buffer;
       pp_really_flush (pp);
       ++errorcount;
       gcc_assert (gfc_output_buffer_empty_p (pp_error_buffer));
-      diagnostic_action_after_output (global_dc, DK_ERROR);
       pp->buffer = tmp_buffer;
+      diagnostic_action_after_output (global_dc, DK_ERROR);
       return true;
     }

   return false;
 }

However, a better fix may be to make the error_buffer also dynamically
allocated like the warning_buffer. Not sure why I did the change.

(It would be nice to have a testcase testing this in the regression testsuite.)


> Here is the first error reported by valgrind:
> 
> ==1154== Invalid free() / delete / delete[] / realloc()
> ==1154==    at 0x4C28ADC: free (in
> /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==1154==    by 0x126B11B: diagnostic_finish(diagnostic_context*)
> (diagnostic.c:230)
> ==1154==    by 0x126BF11:
> diagnostic_action_after_output(diagnostic_context*, diagnostic_t)
> (diagnostic.c:566)
> ==1154==    by 0x63FB0B: gfc_error_check() (error.c:1384)
> ==1154==    by 0x687E47: decode_statement() (parse.c:554)
> ==1154==    by 0x689740: next_statement() (parse.c:1048)
> ==1154==    by 0x68BA0C: parse_executable(gfc_statement) (parse.c:4593)
> ==1154==    by 0x68C430: parse_executable(gfc_statement) (parse.c:3519)
> ==1154==    by 0x68CA06: parse_progunit(gfc_statement) (parse.c:4976)
> ==1154==    by 0x68E167: gfc_parse_file() (parse.c:5424)
> ==1154==    by 0x6CE642: gfc_be_parse_file() (f95-lang.c:215)
> ==1154==    by 0xBCA44E: compile_file() (toplev.c:560)
> ==1154==  Address 0x1cfefa8 is 8 bytes inside data symbol "_ZL12error_buffer"
>From gcc-bugs-return-488958-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Sun Jun 14 17:06:42 2015
Return-Path: <gcc-bugs-return-488958-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 130405 invoked by alias); 14 Jun 2015 17:06:42 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 130352 invoked by uid 48); 14 Jun 2015 17:06:38 -0000
From: "ebotcazou at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug ada/66242] Front-end error if exception propagation disabled
Date: Sun, 14 Jun 2015 17:06:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: ada
X-Bugzilla-Version: 4.9.1
X-Bugzilla-Keywords:
X-Bugzilla-Severity: enhancement
X-Bugzilla-Who: ebotcazou at gcc dot gnu.org
X-Bugzilla-Status: NEW
X-Bugzilla-Resolution:
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields: cf_gcctarget bug_status cf_reconfirmed_on cc cf_gcchost everconfirmed bug_severity
Message-ID: <bug-66242-4-42MYhNPLzO@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-66242-4@http.gcc.gnu.org/bugzilla/>
References: <bug-66242-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2015-06/txt/msg01290.txt.bz2
Content-length: 693

https://gcc.gnu.org/bugzilla/show_bug.cgi?idf242

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|arm-eabi,                   |
                   |x86_64-apple-darwin13       |
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2015-06-14
                 CC|                            |ebotcazou at gcc dot gnu.org
               Host|x86_64-apple-darwin13       |
     Ever confirmed|0                           |1
           Severity|normal                      |enhancement

[Bug fortran/66528] [6 Regression] unbalanced IF/ENDIF with -fmax-errors=1 causes invalid free

[tkoenig at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66528

--- Comment #5 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
(In reply to Manuel López-Ibáñez from comment #4)
> diagnostic_finish tries to free the output_buffer,
> but the error_buffer is statically allocated. I think this should be enough:
> 
> --- error.c     (revision 223651)
> +++ error.c     (working copy)
> @@ -1379,12 +1379,12 @@ gfc_error_check (void)
>        output_buffer *tmp_buffer = pp->buffer;
>        pp->buffer = pp_error_buffer;
>        pp_really_flush (pp);
>        ++errorcount;
>        gcc_assert (gfc_output_buffer_empty_p (pp_error_buffer));
> -      diagnostic_action_after_output (global_dc, DK_ERROR);
>        pp->buffer = tmp_buffer;
> +      diagnostic_action_after_output (global_dc, DK_ERROR);
>        return true;
>      }
> 
>    return false;
>  }
> 
> However, a better fix may be to make the error_buffer also dynamically
> allocated like the warning_buffer. Not sure why I did the change.

I think either version would be OK, but I agree that making
the error_buffer dynamic would be better.

> (It would be nice to have a testcase testing this in the regression
> testsuite.)

Easy enough to generate from the test case in comment #2.
>From gcc-bugs-return-489219-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Wed Jun 17 16:31:56 2015
Return-Path: <gcc-bugs-return-489219-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 104244 invoked by alias); 17 Jun 2015 16:31:55 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 104206 invoked by uid 48); 17 Jun 2015 16:31:51 -0000
From: "olegendo at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug target/65317] [SH] Shifts used instead of and with const_int
Date: Wed, 17 Jun 2015 16:31:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: target
X-Bugzilla-Version: 5.0
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: olegendo at gcc dot gnu.org
X-Bugzilla-Status: UNCONFIRMED
X-Bugzilla-Resolution:
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields:
Message-ID: <bug-65317-4-srESBEJd47@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-65317-4@http.gcc.gnu.org/bugzilla/>
References: <bug-65317-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2015-06/txt/msg01551.txt.bz2
Content-length: 654

https://gcc.gnu.org/bugzilla/show_bug.cgi?ide317

--- Comment #1 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Oleg Endo from comment #0)
> It seems that it's better to allow any constant for the *andsi_compact
> pattern and split out the constant load if it doesn't fit into K08.  An and
> with constant 0x80000000 could be treated as a special case to emit the
> shorter sequence:
>         shll     r4
>         movt     r0
>         rts
>         rotr     r0

On SH4A the following sequence is better, because imm8 constant loads are in
the MT group:

         shll     r4
         mov      #0,r0
         rts
         rotcr    r0

[Bug fortran/66528] [6 Regression] unbalanced IF/ENDIF with -fmax-errors=1 causes invalid free

[manu at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66528

--- Comment #6 from Manuel López-Ibáñez <manu at gcc dot gnu.org> ---
Author: manu
Date: Wed Jun 24 22:16:42 2015
New Revision: 224926

URL: https://gcc.gnu.org/viewcvs?rev=224926&root=gcc&view=rev
Log:
The problem is that diagnostic_action_after_output tries to delete the active
pretty-printer which tries to delete its output_buffer, which is normally
dynamically allocated via placement-new, but the output_buffer used by the
error_buffer of Fortran is statically allocated. Being statically allocated
simplifies a lot pushing/poping several instances of error_buffer.

The solution is to reset the active output_buffer back to the default one
before calling diagnostic_action_after_output. This is a bit ugly, because this
function does use the output_buffer, however, at the point that Fortran calls
it, both are in an equivalent state, thus there is no visible difference.

gcc/testsuite/ChangeLog:

2015-06-24  Manuel López-Ibáñez  <manu@gcc.gnu.org>

        PR fortran/66528
        * gfortran.dg/maxerrors.f90: New test.

gcc/fortran/ChangeLog:

2015-06-24  Manuel López-Ibáñez  <manu@gcc.gnu.org>

        PR fortran/66528
        * error.c (gfc_warning_check): Restore the default output_buffer
        before calling diagnostic_action_after_output.
        (gfc_error_check): Likewise.
        (gfc_diagnostics_init): Add comment.

Added:
    trunk/gcc/testsuite/gfortran.dg/maxerrors.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/error.c
    trunk/gcc/testsuite/ChangeLog
>From gcc-bugs-return-490139-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Wed Jun 24 22:18:28 2015
Return-Path: <gcc-bugs-return-490139-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 76449 invoked by alias); 24 Jun 2015 22:18:28 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 76012 invoked by uid 48); 24 Jun 2015 22:18:24 -0000
From: "manu at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug fortran/66528] [6 Regression] unbalanced IF/ENDIF with -fmax-errors=1 causes invalid free
Date: Wed, 24 Jun 2015 22:18:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: fortran
X-Bugzilla-Version: 6.0
X-Bugzilla-Keywords: error-recovery
X-Bugzilla-Severity: normal
X-Bugzilla-Who: manu at gcc dot gnu.org
X-Bugzilla-Status: RESOLVED
X-Bugzilla-Resolution: FIXED
X-Bugzilla-Priority: P4
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields: bug_status resolution
Message-ID: <bug-66528-4-fYaEwO8cDw@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-66528-4@http.gcc.gnu.org/bugzilla/>
References: <bug-66528-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2015-06/txt/msg02471.txt.bz2
Content-length: 474

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66528

Manuel López-Ibáñez <manu at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #7 from Manuel López-Ibáñez <manu at gcc dot gnu.org> ---
Fixed for GCC 6
>From gcc-bugs-return-490140-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Wed Jun 24 22:19:33 2015
Return-Path: <gcc-bugs-return-490140-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 78565 invoked by alias); 24 Jun 2015 22:19:33 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 78505 invoked by uid 48); 24 Jun 2015 22:19:28 -0000
From: "kkojima at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug target/66563] [4.9 Regression] ICE (segmentation fault) on sh4-linux-gnu
Date: Wed, 24 Jun 2015 22:19:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: target
X-Bugzilla-Version: 4.9.2
X-Bugzilla-Keywords: ice-on-valid-code
X-Bugzilla-Severity: normal
X-Bugzilla-Who: kkojima at gcc dot gnu.org
X-Bugzilla-Status: UNCONFIRMED
X-Bugzilla-Resolution:
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: 4.9.3
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields:
Message-ID: <bug-66563-4-RAGg8pRIfp@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-66563-4@http.gcc.gnu.org/bugzilla/>
References: <bug-66563-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2015-06/txt/msg02472.txt.bz2
Content-length: 595

https://gcc.gnu.org/bugzilla/show_bug.cgi?idf563

--- Comment #35 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
(In reply to Oleg Endo from comment #33)
> I see, thanks.  In this case, could you please add a comment e.g.:
>
> ;; Loads of the GOTPC relocation values must not be optimized away
> ;; by e.g. any kind of CSE and must stay as they are.  Although there
> ;; are other various ways to ensure this, we use an artificial counter
> ;; operand to generate unique symbols.
> (define_expand "GOTaddr2picreg"

Good thinking.  I've committed the patch with your comment on trunk.