bugGNU roff - Bugs: bug #61748, [grohtml] infinite loop

 
 

bug #61748: [grohtml] infinite loop

Submitter:  G. Branden Robinson <gbranden>
Submitted:  Thu 30 Dec 2021 09:32:35 PM UTC
   
 
Category:  Driver grohtml Severity:  4 - Important
Item Group:  Crash/Unresponsive Status:  Fixed
Privacy:  Public Assigned to:  gbranden
Open/Closed:  Closed Planned Release:  1.23.0
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 11 Jan 2022 09:42:02 PM UTC, comment #7: 

This issue with the mysterious diagnostic is now a separate ticket, bug #61798.  Please direct any follow-ups there.

G. Branden Robinson <gbranden>
Group administrator
Tue 04 Jan 2022 08:20:48 PM UTC, comment #6: 

Correction to comment #1
  String '10' comes from "grnexmpl.g", string 'g9' from "grn".
####
  Using programs from the repository.

  This produces no '\O' output:

soelim grnexmpl.me | grn -Thtml | eqn -Thtml |& less

  This produces '\O' output:

soelim grnexmpl.me | grn -Thtml | eqn -Tps:html |& less

  For example:

.i "spreading ratio" .
.nr 0C \n(.C
.cp 0
.ds 10
.eo
.as 10 "This reset phase is controlled by clocks
.ec
.as1 10 \O[HTML-IMAGE-INLINE-BEGIN]
.nr 0sfont \n[.f]
.ft
.nr 0spfont \n[.f]


  Using groff 1.22.4 (Debian/testing), that is with a prefix "/usr/bin"
to each command and in the directory /usr/share/doc/groff-base/examples,
shows the same results.

Bjarni Ingi Gislason <bjarniig>
Tue 04 Jan 2022 10:34:42 AM UTC, comment #5: 

comment #1:

>   I do not get any trouble processing the minimal example.
>
>   I do get errors when processing "doc/grnexmpl.me":
>
>   The pipe is
>
> soelim grnexmpl.me | grn -Thtml | eqn -Tps:html | pre-grohtml troff -b \
>   -ww -me -Thtml | post-grohtml
>
>   The strings "10", and "g9" come from "eqn" (eqn grnexmpl.me).
>
>   The "output suppression escape sequence" is "\O" and valid arguments
> are the numbers 0-4 and string "5...".
>
>   An exaple of the error messages is:
>
> troff: backtrace: './grnexmpl.me':44: string '10'
> troff: backtrace: file './grnexmpl.me':43
> troff:./grnexmpl.me:43: error: invalid argument 'H' to output suppression escape sequence
> troff: backtrace: './grnexmpl.me':44: string '10'
> troff: backtrace: file './grnexmpl.me':43
> troff:./grnexmpl.me:43: error: invalid argument 'H' to output suppression escape sequence
> troff: backtrace: './grnexmpl.me':44: string '10'
> troff: backtrace: file './grnexmpl.me':43
>
>   All messages are in the attachment.


I'm sorry to say I can no longer reproduce this--neither with groff
1.22.4 nor Git HEAD, which seems very strange to me because I didn't do
anything to even attempt to fix it, and I know I have seen this output
before.

So if someone can come up with a reproducer I can observe, I'd
appreciate it.

G. Branden Robinson <gbranden>
Group administrator
Sun 02 Jan 2022 03:40:14 PM UTC, comment #4: 


commit 57fac864952a671593647ae85751feb3a54971fd
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Fri Dec 31 12:11:19 2021 +1100

    [grohtml]: Fix Savannah #61748.

    * src/devices/grohtml/post-html.cpp (style::style): Add initializer to
      argumentless constructor to avoid later read from unitialized memory
      in `html_printer::do_font`.

    Fixes <https://savannah.gnu.org/bugs/?61748>.

commit 525fe85ec9de643999e44a77568d66a7913b77b1
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Sun Jan 2 00:25:36 2022 +1100

    [groff]: Add regression test for Savannah #61748.

    * src/roff/groff/tests/html_works_with_grn_and_eqn.sh: Do it.
    * src/roff/groff/groff.am (groff_TESTS): Run it.

    Test may loop infinitely at this commit; since the behavior depends on
    a read from uninitialized memory, I doubt that it is deterministic
    across system configurations.


G. Branden Robinson <gbranden>
Group administrator
Sun 02 Jan 2022 06:26:29 AM UTC, comment #3: 

Hi, Bjarni!

comment #1:

>   I do not get any trouble processing the minimal example.


Thanks for trying.  I hesitate to call the behavior
non-deterministic, because it was reproducible with 100%
reliability on my system, but there are obviously many factors
affecting the memory layout of an executable.

>   I do get errors when processing "doc/grnexmpl.me":
>
>   The pipe is
>
> soelim grnexmpl.me | grn -Thtml | eqn -Tps:html | pre-grohtml troff -b \
>   -ww -me -Thtml | post-grohtml
>
>   The strings "10", and "g9" come from "eqn" (eqn grnexmpl.me).
>
>   The "output suppression escape sequence" is "\O" and valid arguments
> are the numbers 0-4 and string "5...".
>
>   An exaple of the error messages is:
>
> troff: backtrace: './grnexmpl.me':44: string '10'
> troff: backtrace: file './grnexmpl.me':43
> troff:./grnexmpl.me:43: error: invalid argument 'H' to output suppression escape sequence
> troff: backtrace: './grnexmpl.me':44: string '10'
> troff: backtrace: file './grnexmpl.me':43
> troff:./grnexmpl.me:43: error: invalid argument 'H' to output suppression escape sequence
> troff: backtrace: './grnexmpl.me':44: string '10'
> troff: backtrace: file './grnexmpl.me':43
>
>   All messages are in the attachment.


Yes, I noticed these too.  They are clearly evidence of a bug,
but as it is (as far as I can tell) unrelated to the problem I
encountered, I did not mention it.

comment #2:

>   The argument 'H' comes (presumably) from this
>
> src/include/html-strings.h:#define HTML_IMAGE_INLINE_BEGIN
> "\\O[HTML-IMAGE-INLINE-BEGIN]"
> src/include/html-strings.h:#define HTML_IMAGE_INLINE_END
> "\\O[HTML-IMAGE-INLINE-END]"


Thank you!  I had not tried to track that down yet.

G. Branden Robinson <gbranden>
Group administrator
Sun 02 Jan 2022 02:31:18 AM UTC, comment #2: 

  The argument 'H' comes (presumably) from this

src/include/html-strings.h:#define HTML_IMAGE_INLINE_BEGIN    
"\\O[HTML-IMAGE-INLINE-BEGIN]"
src/include/html-strings.h:#define HTML_IMAGE_INLINE_END      
"\\O[HTML-IMAGE-INLINE-END]"

Bjarni Ingi Gislason <bjarniig>
Sun 02 Jan 2022 12:48:19 AM UTC, comment #1: 

  I do not get any trouble processing the minimal example.

  I do get errors when processing "doc/grnexmpl.me":

  The pipe is

soelim grnexmpl.me | grn -Thtml | eqn -Tps:html | pre-grohtml troff -b \
  -ww -me -Thtml | post-grohtml

  The strings "10", and "g9" come from "eqn" (eqn grnexmpl.me).

  The "output suppression escape sequence" is "\O" and valid arguments
are the numbers 0-4 and string "5...".

  An exaple of the error messages is:

troff: backtrace: './grnexmpl.me':44: string '10'
troff: backtrace: file './grnexmpl.me':43
troff:./grnexmpl.me:43: error: invalid argument 'H' to output suppression escape sequence
troff: backtrace: './grnexmpl.me':44: string '10'
troff: backtrace: file './grnexmpl.me':43
troff:./grnexmpl.me:43: error: invalid argument 'H' to output suppression escape sequence
troff: backtrace: './grnexmpl.me':44: string '10'
troff: backtrace: file './grnexmpl.me':43

  All messages are in the attachment.


(file #52597)

Bjarni Ingi Gislason <bjarniig>
Thu 30 Dec 2021 09:32:35 PM UTC, original submission:  

The following commit exposed an infinite loop in post-grohtml.


commit c71b4ef4aa46f9264751df37ffac4fdf796c45eb
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Tue Jul 27 00:10:49 2021 +1000

    [grohtml]: Fix Savannah #60971.

    * src/preproc/html/pre-html.cpp (makeFileName): Consistently put a dash
      at the end of `macroset_template` whether the image file name stem is
      user-supplied or the default.  Stop adding the dash before the image
      number in `image_template` instead.  This makes the image file name
      format reliable whether the image needs to be subdivided (eqn) or not
      (tbl).

    Fixes <https://savannah.gnu.org/bugs/?60971>.


However, as the eventual root-cause analysis established, this was not
the fault of the above change.  What the above change (to the
pre-grohtml preprocessor, mind you) did seem to do was cause a
reorganization of storage later such that a read from uninitialized
memory led to a highly undesirable default point size.

Because this sort of thing can be system-dependent, I can't promise that
this is a reproducer for everyone, but it was for me.

Input:


.EQ
gsize 12
delim $$
.EN
.pp
.pp
The faster clocks are $ PN $


You do need the gsize eqn directive, you do need to use delimiters,
you do need the extra paragraph tag, and you do need the inline
eqn input.  (This is an extremely reduced version of our source tree's
grnexmpl.me, which is how I noticed the problem in the first place.)

Reproduce with:


./build/test-groff -b -ww -Thtml -eg -me EXPERIMENTS/gurn.me


As I said, I can't promise that others will be able to reproduce this,
but it was highly reliable on my x86-64 Debian bullseye system.

Here's the fix, if you don't believe me.


$ git diff
diff --git a/src/devices/grohtml/post-html.cpp b/src/devices/grohtml/post-html.cpp
index a1018e19a..5be5e9979 100644
--- a/src/devices/grohtml/post-html.cpp
+++ b/src/devices/grohtml/post-html.cpp
@@ -324,7 +324,7 @@ struct style {
 };

 style::style()
-  : f(NULL)
+  : f(NULL), point_size(-1)
 {
 }


I leave as an exercise for the reader the determination of why the value
-1 is meaningful.

I had thought that argument-less constructors that don't initialize all
the fields of their objects were considered bad C++ style. (?)

G. Branden Robinson <gbranden>
Group administrator

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #52597:  61748.bug added by bjarniig (66KiB - application/octet-stream - Error messages)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by bjarniig (Updated the item)
  • -email is unavailable- added by gbranden (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.

    Only logged-in users can vote.

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-01-02 gbranden StatusIn Progress Fixed
        Open/ClosedOpen Closed
        Planned ReleaseNone 1.23.0
    2022-01-02 bjarniig Attached File- Added 61748.bug, #52597

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code