bugGNU troff - Bugs: bug #61414, [libgroff] emits misleading...

 
 

bug #61414: [libgroff] emits misleading diagnostics when DESC file 'res' is invalid

Submitted by:  G. Branden Robinson <gbranden>
Submitted on:  Tue 02 Nov 2021 07:46:38 AM UTC
 
Category:  Core Severity:  3 - Normal
Item Group:  Warning/Suspicious behaviour Status:  Fixed
Privacy:  Public Assigned to:  gbranden
Open/Closed:  Closed Planned Release:  1.23.0
* Mandatory Fields

Add a New Comment (Rich Markup)
   

Comment Type & Canned Response:
       No canned response available

 

Sat 06 Nov 2021 05:47:01 AM UTC, comment #1: 

commit ee0942bdecdef59b202d84dc2a754d04288c9abd
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Tue Nov 2 18:23:09 2021 +1100

    [libgroff]: Validate DESC, font descriptions more.

    [libgroff]: Do more device and font description file validation, resolve
    an assertion failure arising from a negative declared device resolution,
    and correct a documentation error.

    * src/libs/libgroff/font.cpp (font::load): Include more information
      about invalid input in diagnostic messages.
      - When a kern pair's amount is missing or invalid, report the name of
        the kern pair.
      - When someone tries to declare the first entry in the charset section
        as an alias, report the glyph name.
      - Identify the token for the unnamed character if an attempt is made
        to alias it.
      - When an out-of-range character type is applied to a glyph, name the
        glyph.

      (font::load_desc): Same.
      - Drop redundant zero initialization of `res`.
      - Check all directives that take basic units for positive values,
        adding `res`, `unitwidth`, `paperwidth`, and `paperlength`.  Update
        this diagnostic to demand positive, not nonnegative, values.
      - When the font count is long in a `fonts` directive, report how many
        font names were declared (and thus expected).  (If the count is
        short, the next line is read for a font name, like 'tcommand' in our
        devutf8/DESC.)
      - When interpreting a `papersize` directive, throw an error and return
        false if `res` has not yet been encountered, since it is used in
        subsequent computations.
      - When a paper format cannot be determined, report the original
        declared value from the DESC file.  Use `strdup()` to save it since
        it gets clobbered by the resolving process.  `free()` the saved
        string when we're done, regardless of error condition.

    * doc/groff.texi (Device and Font Files):
    * man/groff_font.5.man (DESC file format): Document additional exception
      to order-indifference of directives: (at least one) `res` must precede
      `papersize`.

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

G. Branden Robinson <gbranden>
Project Administrator
Tue 02 Nov 2021 07:46:38 AM UTC, original submission:  

An explicitly zero device resolution should be rejected as invalid, not missing.  What's worse, a negative one gets used for computation anyway and causes an assertion failure.

(The `papersize` fiddling can be ignored; I was exploring another issue alongside this one.  If you take them out you still get the assertion failures, via troff:main -> init_environments -> environment::environment -> points_to_units -> scale).

$ sed -i.bak 's/res 240/res 0/;/res/a\
papersize 10i,7i
' build/font/devutf8/DESC
$ head -n 2 build/font/devutf8/DESC
res 0
papersize 10i,7i
$ ./build/test-groff -Tutf8
/home/branden/src/GIT/groff/build/groff:/home/branden/src/GIT/groff/build/font/devutf8/DESC: error: device description file missing 'res' directive
/home/branden/src/GIT/groff/build/groff: fatal error: cannot load 'DESC' description file for device 'utf8'

Let's now make the resolution negative and see what happens.

$ sed -i 's/res 0/res -240/' build/font/devutf8/DESC
$ head -n 2 build/font/devutf8/DESC
res -240
papersize 10i,7i
$ ./build/test-groff -Tutf8
troff: ../src/roff/troff/number.cpp:652: scale(): assertion failed: 'x >= 0 && y > 0'
/home/branden/src/GIT/groff/build/groff: error: troff: Aborted (core dumped)

Mondo bad!

groff 1.22.4 behaves similarly.  Here are the negative and zero `res` cases, respectively.

$ groff -Tutf8 -Fbuild/font/
troff: Failed assertion at line 653, file '../../src/roff/troff/number.cpp'.
groff: troff: Signal 6 (core dumped)
$ groff -Tutf8 -Fbuild/font/
groff:build/font/devutf8/DESC:10: missing 'res' command
groff: fatal error: invalid device 'utf8'

G. Branden Robinson <gbranden>
Project Administrator

 

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

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -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 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2021-11-06 gbranden StatusIn Progress => Fixed
        Open/ClosedOpen => Closed
        Planned ReleaseNone => 1.23.0

    Back to the top


    Powered by Savane 3.9