[bug #61643] grops: core dump when running and compiled with -Wsanitize=undefined

[G. Branden Robinson]

Update of bug #61643 (project groff):

                Severity:              3 - Normal => 2 - Minor              
              Item Group:      Build/Installation => Lint                   
                  Status:                    None => In Progress            
             Assigned to:                    None => gbranden               
         Planned Release:                    None => 1.23.0                 

    _______________________________________________________

Follow-up Comment #4:

[comment #1 comment #1:]
>   This bug should be fixed before the next release.

I don't agree with this implication of urgency.  This looks to me like a false
positive from AddressSanitizer.

Consider the data structures that are being walked here.  They are both
statically initialized within functions and never modified.

https://git.savannah.gnu.org/cgit/groff.git/tree/src/devices/grops/ps.cpp#n1528

https://git.savannah.gnu.org/cgit/groff.git/tree/src/devices/grops/psrm.cpp#n971

As an experiment, I added `assert()`s to both functions.


@@ -1557,7 +1557,9 @@ void ps_printer::special(char *arg, const environment
*env, char type)
     error("empty X command ignored");
     return;
   }
-  for (unsigned int i = 0; i < sizeof(proc_table)/sizeof(proc_table[0]);
i++)
+  for (unsigned int i = 0; i < sizeof(proc_table)/sizeof(proc_table[0]); i++)
{
+    assert(proc_table[i].name != 0 /* nullptr */);
+    assert(proc_table[i].proc != 0 /* nullptr */);
     if (strncmp(command, proc_table[i].name, p - command) == 0) {
       flush_sbuf();
       if (sbuf_color != *env->col)
@@ -1565,6 +1567,7 @@ void ps_printer::special(char *arg, const environment
*env, char type)
       (this->*(proc_table[i].proc))(p, env);
       return;
     }
+  }
   error("X command '%1' not recognised", command);
 }

@@ -1023,12 +1023,15 @@ void resource_manager::process_file(int rank, FILE
*fp,
        if (buf[1] == '%') {
          const char *ptr;
          int i;
-         for (i = 0; i < NCOMMENTS; i++)
+         for (i = 0; i < NCOMMENTS; i++) {
+           assert(comment_table[i].name != 0 /* nullptr */);
+           assert(comment_table[i].proc != 0 /* nullptr */);
            if ((ptr = matches_comment(buf, comment_table[i].name))) {
              copy_this_line
                = (this->*(comment_table[i].proc))(ptr, rank, fp, outfp);
              break;
            }
+         }
          if (i >= NCOMMENTS && in_header) {
            if ((ptr = matches_comment(buf, "EndComments")))
              in_header = 0;


Neither rebuilding the groff tree nor running your reproducer script provoked
an assertion failure.

However, recalling above that these structs are "never modified", I think a
better fix is simply to declare them `const`.  So I'll do that.

That reduces the severity of this ticket to 'minor' and moves it to item group
'Lint'.

If some C/C++ genius wants to explain to me how AddressSanitizer isn't
actually false-flagging these structure member accesses, I'm all ears.


    _______________________________________________________

Reply to this item at:

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

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