www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 3609] New: isNumeric causes a stack overlfow with mutable arrays

reply d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3609

           Summary: isNumeric causes a stack overlfow with mutable arrays
           Product: D
           Version: 2.036
          Platform: Other
        OS/Version: Windows
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Phobos
        AssignedTo: nobody puremagic.com
        ReportedBy: sandford jhu.edu



Test case:

import std.string;

void main(string[] args) {
    isNumeric("1".dup);
}

This results in a stack overflow on DMD 2.037.
Note that immutable arrays work correctly. (i.e. isNumeric("1".idup); is okay)

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Dec 11 2009
next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3609


David Simcha <dsimcha yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dsimcha yahoo.com



I ran into this a few days, too and was meaning to file it.  isNumeric is
**completely** busted on D2, and it amazes me that noone noticed it until now. 
The problem is that isNumeric(string, bool) is overloaded against
isNumeric(...).  In D1, string is an alias for char[].  In D2, it's an alias
for immutable(char)[].  If the type that's input is not a string, it's sent to
isNumeric(...).  If it's a mutable char[] array in D2, the following is
executed:

    if (_arguments[0] == typeid(char[]))
        return isNumeric(va_arg!(char[])(_argptr));

Obviously, the intent here is to call the overload, but in D2 the overload
expects an immutable(char)[], leading to isNumeric(...) getting called again. 
Violia, infinite recursion and stack overflows.

I doubt anyone actually uses isNumeric(...), because its functionality should
really be handled at compile time w/ templates.  I doubt there are too many use
cases for having it be handled at runtime.  Therefore, I recommend dumping it
and templating isNumeric(string) to handle char[], wchar[], dchar[], and the
const and immutable flavors of these.  This would also fix bug 3610.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Dec 11 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3609


Andrei Alexandrescu <andrei metalanguage.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |andrei metalanguage.com



21:24:55 PST ---

 I ran into this a few days, too and was meaning to file it.  isNumeric is
 **completely** busted on D2, and it amazes me that noone noticed it until now. 
 The problem is that isNumeric(string, bool) is overloaded against
 isNumeric(...).  In D1, string is an alias for char[].  In D2, it's an alias
 for immutable(char)[].  If the type that's input is not a string, it's sent to
 isNumeric(...).  If it's a mutable char[] array in D2, the following is
 executed:
 
     if (_arguments[0] == typeid(char[]))
         return isNumeric(va_arg!(char[])(_argptr));
 
 Obviously, the intent here is to call the overload, but in D2 the overload
 expects an immutable(char)[], leading to isNumeric(...) getting called again. 
 Violia, infinite recursion and stack overflows.
 
 I doubt anyone actually uses isNumeric(...), because its functionality should
 really be handled at compile time w/ templates.  I doubt there are too many use
 cases for having it be handled at runtime.  Therefore, I recommend dumping it
 and templating isNumeric(string) to handle char[], wchar[], dchar[], and the
 const and immutable flavors of these.  This would also fix bug 3610.
IMHO it's kind of silly to define isNumeric. It does about the same amount of work as to, but doesn't give you the actual number. I suggest we just yank it. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Dec 11 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3609





 IMHO it's kind of silly to define isNumeric. It does about the same amount of
 work as to, but doesn't give you the actual number. I suggest we just yank it.
The only problem I see with this is that then you're using exception handling as flow control if your goal was just to validate something. Then again, this might not be a big deal because: 1. Why would you be validating something as being numeric if you have no intention of converting it to a number? 2. One reason why using exceptions as flow control is considered bad is performance. Not too many people try to convert strings to numbers inside performance critical code. 3. The other reason why exceptions as flow control is considered bad is that it's unstructured flow control, kind of like goto. However, in this case I don't think that applies. real foo; try { foo = to!real(someString); } catch(ConvError) { // yada yada yada } Seems pretty clear and readable to me. IMHO taking it out is probably a good move but I think we should think twice and allow others to comment b/c it does have the downside of encouraging using exceptions as flow control. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Dec 11 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3609


Andrei Alexandrescu <andrei metalanguage.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|nobody puremagic.com        |andrei metalanguage.com


-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jan 08 2011
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3609


Andrej Mitrovic <andrej.mitrovich gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
                 CC|                            |andrej.mitrovich gmail.com
         Resolution|                            |FIXED



13:41:41 PDT ---
I can see from the year 2009 checkout that isNumeric took a string, but now it
takes a const(char)[] s, and this seems to fix the issue. I'm not having stack
overflows here. I'll mark this as fixed.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
May 26 2011