bugGNU roff - Bugs: bug #66653, [patch][troff].asciify odd...

 
 

bug #66653: [patch][troff].asciify odd behaviour / adding pdf features to ms

Submitter:  Deri James <deri>
Submitted:  Fri 10 Jan 2025 05:13:24 PM UTC
   
 
Category:  Core Severity:  3 - Normal
Item Group:  Warning/Suspicious behaviour Status:  Need Info
Privacy:  Public Assigned to:  deri
Open/Closed:  Open Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 19 Jan 2025 05:58:20 PM UTC, comment #8: 

The attached patch fixes the first problem (truncation of the result of asciifying) and may be a fix for the elusive crash (it certainly stops the crashing with this reproducer). In addition I have brought the result of asciify into line with Branden's new \X handling, i.e. any glyph which has not got an ascii code is represented in its \[uXXXX] form.

Technical Note

In order to extract "text" back from nodes in a diversion (or box) asciify should only be interested in glyph_nodes and word_space_nodes, since these are the nodes created from text in the first place. Unfortunately when trying to asciify other types of node, intead of text chars being added, a '\000' is added instead (which results in truncation when treated as a string. This is why the ms versions of .I caused truncation since the the italic correction also includes a dummy_node, which is one of nodes which adds '\000', the other I discovered was tag_node, there may well be others. The \& is another escape which can add a dummy_node so "G.\& Branden Robinson" (as a co-author) got truncated to just "G."

This patch prevents these two node types interfering with asciify.

One strange thing I observed in the truncated macros is that the cl structure which contains members ptr, head, tail and len, was in a case where the diversion started with a tag_node (devtag TL) so the string appears empty (starts with '\000') and ptr, head and tail all point to the same address but len held a positive number equal to the number of characters added to the macro by asciify. This may be expected behaviour, len could be the number of bytes of allocated memory, but I just wondered what would happen if this was subsequently .chop(ped). So this might be the reason for the assert crash dump at the end of the run, but I would not bank on it, it is a slippery fellow.

The patch can probably be improved, I'd appreciate Branden's insight.

My "proof of concept" work on adding pdf features to the ms macros is coming along, the attached pdfs are examples. Both djms.pdf and djpic.pdf are run with the current ms source documents (with just the addition of ".RP no" adding a separate title page so the TOC can be auto relocated to page after the title) against my current version of pdfms.tmac (still got a few wrinkles to sort out so it is not attached - yet). When finished I am hoping Branden wants to incorporate this work into s.tmac, probably after 1.24, with suitable .if '\*[.T]'pdf' protections. The pdfmark2.pdf is using the latest pdfmark.ms before it was removed from git and requires "pdfmom --roff -ms" and my new pdfms.tmac to work. Things to notice are author and title correct if you run pdfinfo and a clickable TOC. All of these needed the patch applied to work properly.

(file #56781, file #56782, file #56783, file #56784)

Deri James <deri>
Group Member
Tue 14 Jan 2025 05:58:41 PM UTC, comment #7: 

comment #5:

> You may remember a "wish" to allow Dingbats glyphs to be accessed
> using \[uXXXX] notation rather than just \N'nnn'.


Right.  Bug #63018, committed in August.  I really need to run a current build.

Dave <barx>
Group Member
Mon 13 Jan 2025 06:29:45 PM UTC, comment #6: 

comment #5:

> To see the second issue you have to run it with ms.ms,


D'oh!  I'm illiterate.  That is indeed the filename in the command you pasted in comment #2.  So much for me "trying to mimic your test as closely as possible"!

Dave <barx>
Group Member
Mon 13 Jan 2025 12:31:13 PM UTC, comment #5: 

The warning:-

troff:it.trf:7: warning: special character 'u260E' not defined

Occurs if I run 1.23.0. You may remember a "wish" to allow Dingbats glyphs to be accessed using \[uXXXX] notation rather than just \N'nnn'. I believe that is now in current master.

The document it.trf does not trigger the second problem (core dump) unfortunately, however, it does illustrate the first problem when the ms versions of B and BI are used the overview entry is truncated to a single word. To see the second issue you have to run it with ms.ms, it's not even triggered by pic.ms, but at least we have a "reproducer" again now.

Deri James <deri>
Group Member
Mon 13 Jan 2025 09:47:10 AM UTC, comment #4: 

comment #2:

> The two problems can only be seen by making an amendment to pdfms.tmac.


I'm still unable to provoke the assertion failure, whether I make both changes to pdfms.tmac, or only the second one.

$ cat pdfms.tmac
.nr PDFHREF.VIEW.LEADING  10.0p
.tm DJ loaded
.de I
\fI\\$1\fP
..
.de BI
\f(BI\\$1\fP
..
.am @NH
.  nr nh*run 1
.  box @NHpdf
.  dt 1v @NH-end
..
.de shw
.  pdfbookmark \\n[nh*hl] "\\$*"
..
.de @NH-end
.  fl
.  box
.  nr nh*run 0
.  chop @NHpdf
\&\\*[@NHpdf]
.  asciify @NHpdf
.  pdfbookmark \\n[nh*hl] "\\*[@NHpdf]"
.\" .shw \\*[@NHpdf]
..
$ groff -Tpdf -Kutf8 -ms -M. -mpdfms -pet -ww it.trf | wc
DJ loaded
troff:it.trf:7: warning: special character 'u260E' not defined
    171    2017   16989
$ groff-latest -Tpdf -Kutf8 -ms -M. -mpdfms -pet -ww it.trf | wc
DJ loaded
troff:it.trf:7: warning: special character 'u260E' not defined
     82     231    2167
$ sed 3,8d pdfms.tmac > pdfms2.tmac
$ groff -Tpdf -Kutf8 -ms -M. -mpdfms2 -pet -ww it.trf | wc
DJ loaded
troff:it.trf:7: warning: special character 'u260E' not defined
    171    2014   16983
$ groff-latest -Tpdf -Kutf8 -ms -M. -mpdfms2 -pet -ww it.trf | wc
DJ loaded
troff:it.trf:7: warning: special character 'u260E' not defined
     80     227    2164

My it.trf is unchanged from the one attached to this ticket.

The -pet options seem unnecessary for this input file (and indeed, I get the same results with or without them), but as you supplied them, I did too to try to mimic your test as closely as possible.

Dave <barx>
Group Member
Mon 13 Jan 2025 09:02:22 AM UTC, comment #3: 

comment #2:

> testing with any other ms documents you have would be appreciated


I have none, sorry, or I'd try it on them.

> The warning you get running it.trf is because it is a UTF-8
> document, so it needs preconv as well.


Ah, yes, I should have picked up on that.

However, invoking preconv merely changes it to a different warning.  (My August groff build generates a substantially smaller PDF than groff 1.23, but both give the same warning.)

$ groff -Tpdf -Kutf8 -ms -M. -mpdfms it.trf | wc
DJ loaded
troff:it.trf:7: warning: special character 'u260E' not defined
    171    2017   16989
$ groff-latest -Tpdf -Kutf8 -ms -M. -mpdfms it.trf | wc
DJ loaded
troff:it.trf:7: warning: special character 'u260E' not defined
     82     231    2167


> I can't remember the previous bug number which may be the same
> issue for the core dump.


With the full assertion text now at hand, I surmise you refer to bug #65322.

Dave <barx>
Group Member
Sun 12 Jan 2025 02:15:45 PM UTC, comment #2: 

Sorry, I was not very clear. The pdfms.tmac I sent works fine (at least with ms.ms and pic.ms, testing with any other ms documents you have would be appreciated). The two problems can only be seen by making an amendment to pdfms.tmac. The first problem can be revealed by commenting out the definitions of the I and BI near the top of the file. You will see the two sub entries to "Legacy Features" are truncated to a single word. The second problem can be seen by changing:-

.\" .  pdfbookmark \\n[nh*hl] "\\*[@NHpdf]"
.shw \\*[@NHpdf]

To:-

.  pdfbookmark \\n[nh*hl] "\\*[@NHpdf]"
.\" .shw \\*[@NHpdf]

Near the bottom of the file. You should then see:-

[derij@pip build (master)]$ test-groff -Tpdf -ms -M. -mpdfms -pet -ww ../doc/ms.ms | okular -
DJ loaded
troff: ../src/roff/troff/input.cpp:642: static int input_stack::finish_get(node**): Assertion `level == 0' failed.
/home/derij/groff-git/groff/build/groff: error: troff: Aborted (core dumped)

You get the same result if you run 1.23.0.

The warning you get running it.trf is because it is a UTF-8 document, so it needs preconv as well. I can't remember the previous bug number which may be the same issue for the core dump. I think it was a mom document which got postponed when it no longer caused the error after I removed the call to asciify from pdf.tmac.

Deri James <deri>
Group Member
Sat 11 Jan 2025 10:13:10 PM UTC, comment #1: 

original submission:

> I have also attached a minimum example (it.trf) which
> reproduces the coredump if you use pdfms.tmac with the direct
> call to pdfbookmark rather than the indirection though shw.


I can't reproduce this using the two attached files and a groff I built about 5 months ago from then-latest git source.  Is this something that requires bleeding-edge groff, or am I doing something wrong?  In particular, I'm not sure whether the above-quoted sentence means I should modify something in pdfms.tmac, or whether the version of the file as attached here should produce the core dump.

$ ls -gG pdfms.tmac it.trf
-rw------- 1  79 Jan 10 12:59 it.trf
-rw------- 1 341 Jan 10 13:01 pdfms.tmac
$ groff-latest --version | head -1
GNU groff version 1.23.0.1624-4d251-dirty
$ groff-latest -Tpdf -ms -M. -mpdfms it.trf | wc
DJ loaded
troff:it.trf:11: error: cannot translate character code 226 to special character '^a' in device-independent output
     80     226    2223

I get the same results using groff 1.23 (except without the warning, unless I set GROFF_ENABLE_TRANSPARENCY_WARNINGS), so if you're seeing the core dump on 1.23, then we're using different methodologies.

Dave <barx>
Group Member
Fri 10 Jan 2025 05:13:24 PM UTC, original submission:  

First I will explain what I am trying to accomplish, before describing the issue.

With the demise of pdfmark.pdf and mspdf.tmac I realised that creating a replacement would be a lot easier if we did a similar job Branden did for man but this time for ms. Branden is correct that Keith's original pdf work is more unixy than roffish, using commands on a single line and various flags, and his mspdf.tmac provides new commands which are wrappers around existing ms commands with added pdf extensions. So instead of:-

.NH 1
Introduction

It supports:-

.NH 1
.XN Introduction

Apart from having to learn new commands it is also less flexible. Ms allows:-

.NH 1
Introduction to
.I groff

Which would have to be converted to inline font changes to work with .XN. It also means that existing ms documents can't magically start using pdf features without considerable editing. The gold standard "solution" would be if the output was pdf then pdf features are automatically included, the same as Branden did with man.

As a proof of concept I attempted to make .NH produce a document outline as well as headings. I used ms.ms as a test document, using the command "test-groff -Tpdf -ms -M. -mpdfms -pet -ww ../doc/ms.ms > msdj.pdf", the result is attached. Seems to like pic.ms too.

Now onto the problem with .asciify. Because I have to use a diversion to capture the line(s) after the .NH the diversion contains nodes, fine when the diversion is output as a heading but needs to be converted back to text to be used for the bookmark. Traditionally this was done by calling .asciify which converted glyph nodes and word space nodes back to text:-

.{type: glyph_node, character: "F", diversion level: 1},
{type: glyph_node, character: "i", diversion level: 1},
{type: glyph_node, character: "r", diversion level: 1},
{type: glyph_node, character: "s", diversion level: 1},
{type: glyph_node, character: "t", diversion level: 1},
{type: word_space_node, diversion level: 1},

But, oddly, fails to convert:-

{type: glyph_node, character: "\u260E", diversion level: 1},

Back to \[u260E], possibly the code was written before the advent of preconv.

Now, the issues this code uncovered.

1. Near the top of the pdfms.tmac there are redefinitions of .B and .BI which replace the ones which are generated when s.tmac is loaded. If you comment out the definitions and produce the pdf there are no errors/warnings but some overview entries are truncated. You can see this in the two subsidiary entries to "Legacy Features" which both get truncated to one word by asciify if the s.tmac version of .I is used, but not if using the redefined version. One difference is that the s.tmac versions introduce italic correction escapes which may be what is upsetting asciify.

2. The shw macro just calls pdfbookmark, so largely redundant, why not call pdfbookmark directly instead of shw. If you replace shw with a direct call to pdfbookmark, groff coredumps with an assert failure. I believe this is the same as a bug number we already have, but the reproducer started working after removal of asciify from pdf.tmac. So I have also attached a minimum example (it.trf) which reproduces the coredump if you use pdfms.tmac with the direct call to pdfbookmark rather than the indirection though shw.

This can all be put on the back burner until after the current release I'd just appreciate an affirmation that we agree this is the best way forward in replacing mspdf.tmac. I started on .NH because I think if we have a workable solution for that I think it is likely all pdf features can be slotted in without a separate api, which is what Keith used.

Any good ideas, or better approaches, would be appreciated.


Deri James <deri>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #56781:  asciify.patch added by deri (856B - text/x-patch)
file #56782:  djpic.pdf added by deri (103KiB - application/pdf)
file #56783:  djms.pdf added by deri (81KiB - application/pdf)
file #56784:  pdfmark2.pdf added by deri (69KiB - application/pdf)
file #56763:  pdfms.tmac added by deri (341B - application/octet-stream)
file #56764:  it.trf added by deri (79B - application/octet-stream)
file #56765:  msdj.pdf added by deri (75KiB - application/pdf)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by barx (Posted a comment)
  • -email is unavailable- added by deri (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 10 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2025-01-19 deri Attached File- Added asciify.patch, #56781
        Attached File- Added djpic.pdf, #56782
        Attached File- Added djms.pdf, #56783
        Attached File- Added pdfmark2.pdf, #56784
        StatusNone Need Info
        Assigned toNone deri
        Summary.asciify odd behaviour / adding pdf features to ms [patch][troff].asciify odd behaviour / adding pdf features to ms
    2025-01-10 deri Attached File- Added pdfms.tmac, #56763
        Attached File- Added it.trf, #56764
        Attached File- Added msdj.pdf, #56765

    Back to the top

    Powered by Savane 3.14-7003.
    Corresponding source code