bugGNU roff - Bugs: bug #58153, input_stack::backtrace()...

 
 

bug #58153: input_stack::backtrace() over-suppresses output

Submitter:  G. Branden Robinson <gbranden>
Submitted:  Fri 10 Apr 2020 08:10:01 AM UTC
   
 
Category:  Core Severity:  3 - Normal
Item Group:  Incorrect behaviour Status:  Fixed
Privacy:  Public Assigned to:  gbranden
Open/Closed:  Closed Planned Release:  1.23.0
* Mandatory Fields

Add a New Comment Rich Markup
   

Mon 10 Oct 2022 12:41:32 PM UTC, comment #2: 


commit 6051dae0ed025521ec5a32f9c5e8ebffa7feaf36
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Sat Apr 11 11:08:55 2020 +1000

    Empower backtracing in error/warning conditions.

    Enable backtracing across process/file boundaries when errors or
    non-ignored warnings are encountered.

    Experimentation reveals that .so, .mso, and .pso requests act as
    barriers to backtracing except when explicitly requested with the
    .backtrace request.  Judging by the git history, this behavior dates
    back to June 1991 or earlier.  This did not appear to be the intention
    according to a comment, which was only to suppress the output of
    backtrace output for the line corresponding to the top level itself.
    Unfortunately, that was not its only effect.

    This change does result in one additional line of output when -b is
    given and an error or (non-ignored) warning happens at the top level.
    However, I regard this as unobjectionable because (1) a backtrace was in
    fact explicitly requested; and (2) it seems a poor tradeoff to suppress
    most of the backtrace in all complicated and frustrating cases for the
    sake of one fewer line of backtrace output in a trivial one.

    Now, backtracing behaves the same no matter what triggers it.

    Fixes Savannah #58153.

    * src/roff/troff/input.cpp (file_iterator::backtrace): Call
      get_location() for its side effect of rewriting a filename of "-" to
      "<standard input>", for consistency with other diagnostic messages.
      (In this class, this member function always returns 1, so ignore the
      return value.)

      (input_stack::backtrace): Replace member function body with that
      of input_stack::backtrace_all().

      (input_stack::backtrace_all): Delete.

      (backtrace_request): Update the only call site of the above.

    * src/roff/groff/tests/regression_savannah_58153.sh:
    * src/roff/groff/groff.am: Add regression test.

    * src/roff/groff/tests/string_case_xform_errors.sh: Update regression
      test to not be confounded by additional line of backtrace output.
      Remove bashism along the way.

    * doc/groff.texi:
    * man/groff.7.man:
    * man/groff_diff.7.man: Update documentation.


G. Branden Robinson <gbranden>
Group administrator
Sat 11 Apr 2020 01:30:09 AM UTC, comment #1: 

Fixed in 6051dae0ed025521ec5a32f9c5e8ebffa7feaf36.

G. Branden Robinson <gbranden>
Group administrator
Fri 10 Apr 2020 08:10:01 AM UTC, original submission:  

The above function in src/roff/troff/input.cpp has the following condition in a for loop with an accompanying comment:


// only backtrace down to (not including) the topmost file
...
p && !p->get_location(0, &f, &n);


However, for reasons I admit I haven't been able to quite figure out, this conditional fails under more cases than intended.  Specifically, it seems to fail (ending the loop) when any file_iterator is encountered, halting the backtrace across .so, .nx, or .pso boundaries.  This doesn't happen for the .backtrace request--only for backtraces triggered by warnings or errors in input combined with the groff -b flag.

Since read files and not just stdin are involved, I'm attaching a reproducing case.  Here's what I get with the groff 1.22.4 in my distribution:


$ /usr/bin/groff -b -ww -U /tmp/outer >/dev/null
troff: printf '\s[10]\n';printf '\s[-20]\n';printf 'foobar':2: warning: \s escape results in non-positive point size; set to 1


And here's groff git HEAD with a patch I'm preparing:


$ ./test-groff -U /tmp/outer >/dev/null
troff: backtrace: pipe 'printf '\s[10]\n';printf '\s[-20]\n';printf 'foobar'':2
troff: backtrace: file '/tmp/inner':3
troff: backtrace: pipe 'echo .so /tmp/inner':1
troff: backtrace: file '/tmp/outer':1
troff: printf '\s[10]\n';printf '\s[-20]\n';printf 'foobar':2: warning: point-size escape results in non-positive size -10000u; set to 1u


This patch does result in one additional line of output when -b is given and an error or (non-ignored) warning happens at the top level.  However, I regard this as unobjectionable because (1) a backtrace was in fact explicitly requested; and (2) it seems a poor tradeoff to suppress the backtrace in all complicated and frustrating cases for the sake of less backtrace output in a trivial one.

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 #48806:  58153_repro.tar added by gbranden (10KiB - application/x-tar - Here are some test cases.)
file #48799:  outer added by gbranden (25B - application/octet-stream)
file #48800:  inner added by gbranden (66B - application/octet-stream)

 

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 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2020-04-11 gbranden StatusIn Progress Fixed
        Open/ClosedOpen Closed
        Planned ReleaseNone 1.23.0
    2020-04-11 gbranden Attached File- Added 58153_repro.tar, #48806
    2020-04-10 gbranden Attached File- Added outer, #48799
        Attached File- Added inner, #48800

    Back to the top

    Powered by Savane 3.13-bb6a.
    Corresponding source code