[bug #61748] [grohtml] infinite loop

[G. Branden Robinson]

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

                 Summary: [grohtml] infinite loop
                 Project: GNU troff
            Submitted by: gbranden
            Submitted on: Thu 30 Dec 2021 09:32:35 PM UTC
                Category: Device grohtml
                Severity: 4 - Important
              Item Group: Crash/Not responding
                  Status: In Progress
                 Privacy: Public
             Assigned to: gbranden
             Open/Closed: Open
         Discussion Lock: Any
         Planned Release: None

    _______________________________________________________

Details:

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. (?)





    _______________________________________________________

Reply to this item at:

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

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/