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

 
 

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

Submitted by:  Gene Harvey <gharveymn>
Submitted on:  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

Add a New Comment (Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( 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>
Project Member
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>
Project Member
Fri 14 May 2021 05:22:03 PM UTC, comment #10: 
John W. Eaton <jwe>
Project 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>
Project 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>
Project Member
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>
Project Member
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>
Project Member
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>
Project Member
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>
Project Member
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>
Project Member
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>
Project 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>
Project Member
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)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only project 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.7