bugGNU Octave - Bugs: bug #47690, Octave 64-bit for Windows can't...

 
 

bug #47690: Octave 64-bit for Windows can't parse long hex integers

Submitter:  Tatsuro MATSUOKA <tmacchant>
Submitted:  Thu 14 Apr 2016 12:55:12 AM UTC
   
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  Tatsuro MATSUOA Open/Closed:  * Closed
Release:  * 4.0.1 Operating System:  * Microsoft Windows
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 04 May 2016 02:51:43 PM UTC, comment #12: 

Yeah, I guess my thought was any test we add for 64-bit numbers is going to exceed the range of what we can do right now, so this is a quick and dirty inaccurate test, we can fix later once we get bug #45945 fixed.

Mike Miller <mtmiller>
Group Member
Tue 26 Apr 2016 05:59:42 PM UTC, comment #11: 

Maybe one could (additionally?) test for

assert(uint64 (0xFFFF_FFFF_FFFF_FFFF), uint64 (18446744073709551615));


which will make more sense once bug #45945 should be solved.

Markus Mützel <mmuetzel>
Group administrator
Thu 21 Apr 2016 07:40:23 PM UTC, comment #10: 

I went with the closest test I could think of that is still accurate and still somewhat simple to write.

I wanted to keep the value to 64 bits, but we know it evaluates to a double value, so that test should be accurate within the known constraints of the Octave interpreter.

Maybe 0xFFFF_FFFF_FFFF_0000 would be a more accurate test?


assert (0xFFFF_FFFF_FFFF_0000, 2^64 - 2^16 + 1)


Mike Miller <mtmiller>
Group Member
Thu 21 Apr 2016 07:33:30 AM UTC, comment #9: 

Thank you for pushing the patch.

Wrt the BIST tests. They do not fail due to double precission but:

0xFFFF_FFFF_FFFF_FFFF = 18446744073709551615
2^64 = 18446744073709551616


So in fact, they are one off. Is it worth asserting the "more correct" result there?


Markus Mützel <mmuetzel>
Group administrator
Wed 20 Apr 2016 10:33:35 PM UTC, comment #8: 

I pushed your updated patch, and followed up with some simple tests in parser.tst:

http://hg.savannah.gnu.org/hgweb/octave/rev/986dbd769bb1
http://hg.savannah.gnu.org/hgweb/octave/rev/a3a412dee704

Works for me on Debian 32-bit and 64-bit. Thanks for your contribution!

Mike Miller <mtmiller>
Group Member
Wed 20 Apr 2016 09:04:03 AM UTC, comment #7: 

Thank you, Mike, for your feedback.

I agree with you that the changed handling of large decimal integer input is wrong. Since all input is cast to double anyway (in Octave and in Matlab), I should have stripped off that part of the patch from the beginning.

The fact that integer input seems to not loose precission in Matlab when used for some functions is the other bug #45945. The uintmax_t overflow has to be handled there.

I did not find the patch with your changes so I'll attach a new one that does not touch the parsing of decimal numbers and that uses uintmax_t for the binary cast.

I will file a new bug for the related issue with the scanf family as you suggested.

(file #36970)

Markus Mützel <mmuetzel>
Group administrator
Tue 19 Apr 2016 07:48:10 PM UTC, comment #6: 

This patch looks good for the most part, I tweaked a couple minor style things, and used uintmax_t for the cast in the binary number case. I've tested on Debian with a 32-bit build and it does correct the hex input case that is also seen on Windows.

But one big thing that this patch changes is the handling of integer values that are larger than 2^64. Previous to this patch,


>> x = 100000000000000000000
x =    1.00000000000000e+20


Now with this patch applied it limits integer inputs to 2^64.

I think this patch should not affect decimal number input based on this, but I'm not sure.

What does Matlab do with


x = 100000000000000000000
class(x)


Mike Miller <mtmiller>
Group Member
Tue 19 Apr 2016 06:51:37 PM UTC, comment #5: 

Let's put the scanf issue aside as unrelated for the moment, since you have a fix for this but not that, I think it should be reported and handled separately. But just to note, Octave explicitly says in the section "Input Conversion Syntax" that the type modifier is ignored by the scanf family of functions. So basically I think the right direction for that error would be to always read the widest possible value, and add more characters to be silently ignored (including "ll" and "j")

Mike Miller <mtmiller>
Group Member
Mon 18 Apr 2016 06:08:23 PM UTC, comment #4: 

For the similar issue with "sscanf" described in #comment 1, I am unsure what to do.

One way to go would be to allow all C99 modifiers that can be used with scanf (see e.g. on [1]). That would lead to native results on all platforms. However, results would differ for some integer types on different platforms:
"%lx" would result in a 32bit unsigned integer on 32bit Linux and 32/64bit Windows, but in 64bit unsigned integer on 64bit Linux.

Another way would be to "fix" each modifier to a certain length, e.g.:
- "l" --> always 64 bits (Is "long long int" always 64bit on all platforms?)
- "h" --> always 16 bits
- no modifier --> always 32 bits
That would give consistent results on different platforms, but would need some conversion of modifiers for certain (or for the sake of simplicity for all?) platforms (convert "l" to "ll").

The same goes for unsigned integers.

[1] http://www.cplusplus.com/reference/cstdio/scanf/

Markus Mützel <mmuetzel>
Group administrator
Mon 18 Apr 2016 04:08:41 PM UTC, comment #3: 

This patch is part of the patch I originally posted at bug #45945. This one here should be way saver to accept than anything I could come up with foor the other bug.
The laurels should go to Mike Miller who pointed on the right spots there (and to the initial hint of jwe here).

Without this patch on Windows:

>> format hex
>> uint64 (0xffffffffffffffff)
ans =  0 0 0 0ffffffff


With this patch on Windows:

>> format hex
>> uint64 (0xffffffffffffffff)
ans = ffffffffffffffff
>> format long; 0x1cbe991a08
ans =  123456789000



(file #36958)

Markus Mützel <mmuetzel>
Group administrator
Thu 14 Apr 2016 04:39:57 PM UTC, comment #2: 

The problem is in oct-parse.in.yy, where a scanf call is used to convert the text representation ("0x..." hex) to a number.  It uses long int, but on Windows that is just 32 bits wide.

It also ultimately converts the result to a double and I'm not sure that's the right thing to do in all cases (see also bug #45945).

John W. Eaton <jwe>
Group administrator
Thu 14 Apr 2016 01:17:47 PM UTC, comment #1: 

I can add that hex2dec works as expected, while sscanf does not: when reading a hex string longer than 8 chars it returns [](0x1), while it works with strings shorter than 9 chars.

Francesco Potortì <pot>
Thu 14 Apr 2016 12:55:12 AM UTC, original submission:  

Francesco Potortì reported on the Octave help list:

http://octave.1599824.n4.nabble.com/Octave-64-bit-for-Windows-can-t-parse-long-hex-integers-td4676253.html

Here I have copied and pasted from Francesco Potortì's post
On Debian:


>> printf("%x\n", 123456789000)
1cbe991a08
>> format long; 0x1cbe991a08
ans =  123456789000


On Windows:


>> printf("%x\n", 123456789000)
1cbe991a08
>> format long; 0x1cbe991a08
ans =  4294967295


In practice, when parsing a hex literal bigger than 2^32-1, octave reads
it as 0xffffffff.

Tatsuro MATSUOKA <tmacchant>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #36970:  bug47690_v2.patch added by mmuetzel (1KiB - text/x-diff)
file #36958:  bug47690.patch added by mmuetzel (2KiB - text/x-diff)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mmuetzel (Updated the item)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by pot (Posted a comment)
  • -email is unavailable- added by tmacchant (Submitted the item)
  • -email is unavailable- added by tmacchant
  •  

    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 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-02-22 mtmiller Carbon-CopyRemoved 80942 -
    2019-02-22 mtmiller Carbon-CopyRemoved mike miller <mtmiller@octave.org> -
    2016-04-20 mtmiller StatusNone Fixed
        Open/ClosedOpen Closed
    2016-04-20 mmuetzel Attached File- Added bug47690_v2.patch, #36970
    2016-04-18 mmuetzel Attached File- Added bug47690.patch, #36958
    2016-04-14 tmacchant Carbon-Copy- Added francesco potortì <potorti@isti.cnr.it>
        Carbon-Copy- Added mike miller <mtmiller@octave.org>

    Back to the top

    Powered by Savane 3.13-7c56.
    Corresponding source code