#13731: Fix libsingular memory management
------------------------------------------------------------------+---------
Reporter: nbruin |
Owner: rlm
Type: defect |
Status: new
Priority: major |
Milestone: sage-5.6
Component: memleak |
Resolution:
Keywords: | Work
issues:
Report Upstream: Reported upstream. Developers acknowledge bug. |
Reviewers:
Authors: | Merged
in:
Dependencies: |
Stopgaps:
------------------------------------------------------------------+---------
Comment (by SimonKing):
Replying to [comment:37 nbruin]:
> Replying to [comment:36 SimonKing]:
> > Hence: Could you create diff files for your fixes in the Singular
source code (I am not talking about the malloc thing but about the bug
fixes), so that one can put them into the `patches/` directory of the
Singular spkg?
>
> For the bug that tripped `MALLOC_CHECK_`: sure. That's just a 2-line
change which I'm fairly confident is safe to make.
>
> {{{#!diff
> - int *F=(int *)omAlloc0((rN+1)*sizeof(int));
> + int *F=(int *)omAlloc0(ExpSize);
> - int *G=(int *)omAlloc0((rN+1)*sizeof(int));
> + int *G=(int *)omAlloc0(ExpSize);
> }}}
>
> For the second bug: I have no idea how to fix that. Just doing
> {{{#!diff
> - int *order = (int *) omAlloc(2* sizeof(int))
> + int *order = (int *) omAlloc(3* sizeof(int))
> }}}
> may work but is a bit questionable.
Can you suggest that as a fix on the singular ticket? Or shall I
communicate it?
> Also I have no experience in making patch files for the singular source.
That's easy.
* Create a diff with context (is that called "unified"? I am not sure).
* Give it a good name and perhaps a commit message, and put it into the
`patches/` folder of the Singular spkg. Just look what you currently find
in that folder. The point is: The patches placed into the `patches/`
folder are automatically applied before building Singular.
* ''Do '''not''' change the files in `src/`''! They are supposed to be
identical with the official Singular 3-1-5 source files.
* Rename the spkg into `singular-3-1-5.p2.spkg`.
* Update `SPKG.txt`, describing the new patch.
* Both the `patches/` folder and `SPKG.txt` are under version control,
hence, you also need to `hg commit`.
* Pack the spkg, post it somewhere, and ask me or someone else for a
review.
The same patch could be put into your `malloc-Singular/patches/`, so that
we can test whether the problem is really solved. And if it is, then the
patch in the usual omalloc Singular should be fine to go.
> It may be a bit more robust to take whatever change Singular makes to
their development version and backport that.
Well, let's see what the Singular team thinks of your suggestion...
If they react quickly, then we could take the patch from Singular trac. If
they are too slow, we take your patch from Sage trac.
> Also: we're not done yet. There's still a branch on what valgrind thinks
is an uninitialized value and there's the `List<CanonicalForm>::isEmpty()`
issue which ventures into templated code, so I'm at a loss for that.
Sure. Could be that we will open further Singular trac tickets, and could
be that there will be two or three more patches in `patches/`...
> Purpose of this ticket: I guess it's vague. Feel free to specify. I was
actually hoping that after seeing that singular-malloc can expose issues
that are otherwise hard to find, Singular might be interesting in adopting
it and including it as a honest configure option (resolving the problem
with the executable too). Then we can just put the proper doc in the spkg.
I see that the `patches/` directory contains a sub-directory
`patches/conditional/` containing a patch `sage_debug.patch` and a patch
for cygwin. The changes that you've done in the Singular source code to
use malloc could be put into `patches/conditional/`. Probably spkg-install
needs to be changed to take the new option into account.
> If they don't do that, we could still do so but as a patch to singular.
I think it will be a patch to the Singular spkg ''in any case''. Waiting
for Singular 3-1-6 (which will hopefully contain a fix for the memory
problem) is probably no option.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/13731#comment:38>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
