[HELP] [bug #55475] Segmentation fault in libgroff:relocatep

[G. Branden Robinson]

[redirecting from bug-groff@]

I deeply hate hacking on C when I have no reproducible test case I can
use and no access to a system that exercises the code paths I'm
troubleshooting.  But this bug has been sitting for a long time and it
seems no one wants to touch it.

Details below.

`make distcheck` passed for each of these commits on my Debian-based
x86-64 system but that doesn't tell us much since the problem only ever
manifested on other configurations.

I could use confirmation of two things:

(1) What happens when Guix reverts the workaround patch discussed in
    <https://debbugs.gnu.org/cgi/bugreport.cgi?bug=30785>, letting this
    code get exercised.  I predict an assertion failure.  If I'm right,
    then the bug has been RCAed and we can decide how to make
    relocatep() cope with the issue.

(2) Whether these changes break things for Windows users.

In fact, we could really use ongoing assistance from someone with a
Windows environment to test builds on.

Regards,
Branden

At 2021-01-05T23:59:10-0500, G. Branden Robinson wrote:
> Update of bug #55475 (project groff):
> 
>                   Status:               Confirmed => Need Info              
>              Assigned to:                    None => gbranden               
> 
>     _______________________________________________________
> 
> Follow-up Comment #5:
> 
> Well, no one has stepped up to this, and I can't reproduce it, but I also
> can't stand the thought of groff 1.23.0 going out with this bug, so with
> considerable discomfort I took a stab at it.
> 
> I've pushed the following 3 commits to try and get at the issue.  A tar
> archive of the tree can be obtained from:
> 
> https://git.savannah.gnu.org/cgit/groff.git/commit/?id=2fabd352f4ccdb382acffb7705a129977a2768d3
> 
> Or I can easily prepare a distribution archive from any of these three points
> if that would help.
> 
> I need someone's help to confirm whether the issue has been resolved, or to
> observe the assertion failure!
> 
> 
> commit 2fabd352f4ccdb382acffb7705a129977a2768d3 (HEAD -> master,
> origin/master, origin/HEAD)
> Author: G. Branden Robinson <g.branden.robinson@gmail.com>
> Date:   Wed Jan 6 15:48:39 2021 +1100
> 
>     src/libs/libgroff/relocate.cpp: Shift #ifdef.
>     
>     * src/libs/libgroff/relocate.cpp (set_current_prefix) [!_WIN32]: Move
>       logic attempting to set `curr_prefix` by calling searchpathext() from
>       here...
>       [WIN32]: ...to here.  The PATHEXT environment variable has semantics
>       only under Windows, not POSIX systems, so the placement of this code
>       seemed erroneous.
> 
> commit c2f0e424e4a2bdcd287c8be9957daf93a581673a
> Author: G. Branden Robinson <g.branden.robinson@gmail.com>
> Date:   Wed Jan 6 15:43:57 2021 +1100
> 
>     src/libs/libgroff/relocate.cpp: Fix memory leak.
>     
>     * src/libs/libgroff/relocate.cpp (set_current_prefix) [_WIN32]: Allocate
>       memory from heap for `curr_prefix` only on Windows; on other systems,
>       this file's searchpath() is used to populate `curr_prefix`, and that
>       function (except on Windows) performs its own allocation.  Fixes
>       memory leak noted by Ingo Schwarze.
> 
> commit 89c98409d32d01867e6f7cb7ab61efaf7b1da67e
> Author: G. Branden Robinson <g.branden.robinson@gmail.com>
> Date:   Wed Jan 6 15:34:50 2021 +1100
> 
>     [libgroff]: (relocatep) Add assertion.
>     
>     * src/libs/libgroff/relocate.cpp (relocatep): Add assertion to identify
>       logic error if `curr_prefix` is unexpectedly a null pointer.  See
>       <https://savannah.gnu.org/bugs/?55475>.
> 
> 
>     _______________________________________________________
> 
> Reply to this item at:
> 
>   <https://savannah.gnu.org/bugs/?55475>
> 
> _______________________________________________
>   Message sent via Savannah
>   https://savannah.gnu.org/
>

signature.asc
Description: PGP signature