bugGNU Octave - Bugs: bug #60535, malloc bug in unistd-wrappers.c

 
 

bug #60535: malloc bug in unistd-wrappers.c

Submitter:  Gene Harvey <gharveymn>
Submitted:  Thu 06 May 2021 05:39:38 AM UTC
   
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Segfault, Bus Error, etc.
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * Microsoft Windows
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 17 May 2021 04:53:03 PM UTC, comment #12: 

The buildbot mxeoctave builders for the default branch finished without errors.

Closing as fixed.

Markus Mützel <mmuetzel>
Group administrator
Sat 15 May 2021 04:50:31 PM UTC, comment #11: 

Thanks.

I pushed the patch from comment #6 here:
https://hg.savannah.gnu.org/hgweb/octave/rev/2f6f53651d29

Marking as ready for test.

Markus Mützel <mmuetzel>
Group administrator
Fri 14 May 2021 05:22:03 PM UTC, comment #10: 
John W. Eaton <jwe>
Group administrator
Fri 14 May 2021 04:53:01 PM UTC, comment #9: 

I'm updating now and will push when I get a successful build.

John W. Eaton <jwe>
Group administrator
Fri 14 May 2021 03:44:33 PM UTC, comment #8: 

A fix for the windows-spawn module has been applied upstream in gnulib.

Can we update gnulib to commit 0d7232b98c7b5be6ee9ac513a166110989613870 or newer on the default branch?

Markus Mützel <mmuetzel>
Group administrator
Sun 09 May 2021 06:42:56 PM UTC, comment #7: 

Changing release to "dev" for the remainder of this bug.
The immediate issue is likely already solved on the stable branch.

Markus Mützel <mmuetzel>
Group administrator
Fri 07 May 2021 01:59:47 PM UTC, comment #6: 

I pushed the parts of the patch in comment #5 that add support for non-ASCII command line arguments with the GUI on Windows here:
https://hg.savannah.gnu.org/hgweb/octave/rev/efb5b450ab95

I'm attaching an updated patch that we could use when the gnulib module compiles correctly.

(file #51394)

Markus Mützel <mmuetzel>
Group administrator
Thu 06 May 2021 02:59:53 PM UTC, comment #5: 

Oops. I let C++ syntax sneak into the C++ code.
Should be fixed in the updated patch.

This still needs a fix in gnulib though.


(file #51389)

Markus Mützel <mmuetzel>
Group administrator
Thu 06 May 2021 12:32:59 PM UTC, comment #4: 

I tried the attached patch that uses the "windows-spawn" module that was added to gnulib a few months ago.

But compilation for Windows with MXE Octave fails while compiling gnulib with this error:

libtool: compile:  x86_64-w64-mingw32-gcc -DHAVE_CONFIG_H -I. -I/home/osboxes/Documents/Repositories/Octave/mxe-octave/tmp-default-octave/octave-7.0.0/libgnu -I.. -I/home/osboxes/Documents/Repositories/Octave/mxe-octave/usr/x86_64-w64-mingw32/include -fvisibility=hidden -g -O2 -MT windows-spawn.lo -MD -MP -MF .deps/windows-spawn.Tpo -c /home/osboxes/Documents/Repositories/Octave/mxe-octave/tmp-default-octave/octave-7.0.0/libgnu/windows-spawn.c  -DDLL_EXPORT -DPIC -o .libs/windows-spawn.o
/home/osboxes/Documents/Repositories/Octave/mxe-octave/tmp-default-octave/octave-7.0.0/libgnu/windows-spawn.c:415:1: error: conflicting types for 'compose_handles_block'
  415 | compose_handles_block (const struct inheritable_handles *inh_handles,
      | ^~~~~~~~~~~~~~~~~~~~~
In file included from /home/osboxes/Documents/Repositories/Octave/mxe-octave/tmp-default-octave/octave-7.0.0/libgnu/windows-spawn.c:21:
/home/osboxes/Documents/Repositories/Octave/mxe-octave/tmp-default-octave/octave-7.0.0/libgnu/windows-spawn.h:118:12: note: previous declaration of 'compose_handles_block' was here
  118 | extern int compose_handles_block (const struct inheritable_handles *inh_handles,
      |            ^~~~~~~~~~~~~~~~~~~~~
make[5]: *** [Makefile:3299: windows-spawn.lo] Error 1


I sent a report to the gnulib mailing list here:
https://lists.gnu.org/archive/html/bug-gnulib/2021-05/msg00004.html


(file #51388)

Markus Mützel <mmuetzel>
Group administrator
Thu 06 May 2021 09:58:23 AM UTC, comment #3: 

I agree with jwe's commment #2 (in fact I made these exact changes for my tests).
Pushed to stable here:
https://hg.savannah.gnu.org/hgweb/octave/rev/07dc3ad56d74

I'll probably look next at how we could use the gnulib code.
I'll leave this report open for now.

Markus Mützel <mmuetzel>
Group administrator
Thu 06 May 2021 09:15:35 AM UTC, comment #2: 

Instead of


new_argv = malloc ((argc + 1) * sizeof (*new_argv));


I would write


new_argv = (char **) malloc ((argc + 1) * sizeof (char *));


(though I suppose we could do away with the cast on the value returned from malloc since this is C code).

I would not change the other allocation because sizeof (char) is always 1 by definition, though I can also see the argument for using sizeof the way you did.  I'm not sure what is best here, but I don't recall seeing that style used before.

John W. Eaton <jwe>
Group administrator
Thu 06 May 2021 08:02:10 AM UTC, comment #1: 

Good catch and thank you for the patch.

A fix should probably go to stable.

For context: This is in a function that is only used on Windows.

I'm wondering why we aren't using the "execute" module of gnulib:
https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob_plain;f=lib/execute.h

Afaics, they are using almost identical code but included a bunch of other fixes that are missing in our copy.

I'll try if we could use their function instead (probably on the default branch) if I come around to it.

Markus Mützel <mmuetzel>
Group administrator
Thu 06 May 2021 05:39:38 AM UTC, original submission:  

The number of elements should be multiplied by the element size.

This was causing me some weird issues with long command line invocations.

I've attached a patch.

Gene Harvey <gharveymn>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #51394:  bug60535-gnulib-windows-spawn_v3.patch added by mmuetzel (6KiB - application/octet-stream)
file #51389:  bug60535-gnulib-windows-spawn_v2.patch added by mmuetzel (6KiB - application/octet-stream)
file #51388:  bug60535-gnulib-windows-spawn.patch added by mmuetzel (6KiB - application/octet-stream)
file #51387:  unistd-wrappers-bug.patch added by gharveymn (1KiB - 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 jwe (Posted a comment)
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by gharveymn (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 group members can vote.

     

    Follow 11 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2021-05-17 mmuetzel StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2021-05-15 mmuetzel StatusConfirmed Ready For Test
    2021-05-09 mmuetzel Release6.2.0 dev
    2021-05-07 mmuetzel Attached File- Added bug60535-gnulib-windows-spawn_v3.patch, #51394
    2021-05-06 mmuetzel Attached File- Added bug60535-gnulib-windows-spawn_v2.patch, #51389
    2021-05-06 mmuetzel Attached File- Added bug60535-gnulib-windows-spawn.patch, #51388
    2021-05-06 mmuetzel StatusNone Confirmed
        Releasedev 6.2.0
        Operating SystemAny Microsoft Windows
    2021-05-06 gharveymn Attached File- Added unistd-wrappers-bug.patch, #51387

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code