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

Submitted by:  Tatsuro MATSUOKA <tmacchant>
Submitted on:  Thu 14 Apr 2016 12:55:12 AM UTC  
 
Category: InterpreterSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Incorrect Result
Status: FixedAssigned to: None
Originator Name: Tatsuro MATSUOAOpen/Closed: Closed
Release: 4.0.1Operating 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 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>
Project Administrator
Tue 26 Apr 2016 05:59:42 PM UTC, comment #11:

Maybe one could (additionally?) test for

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

Markus Mützel <mmuetzel>
Project Member
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?

Mike Miller <mtmiller>
Project Administrator
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:

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

Markus Mützel <mmuetzel>
Project Member
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>
Project Administrator
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>
Project Member
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,

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

Mike Miller <mtmiller>
Project Administrator
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>
Project Administrator
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>
Project Member
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:

With this patch on Windows:

(file #36958)

Markus Mützel <mmuetzel>
Project Member
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>
Project 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:

On Windows:

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 File(s):
   
   
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
  • -unavailable- added by mtmiller (Posted a comment)
  • -unavailable- added by mmuetzel (Updated the item)
  • -unavailable- added by jwe (Posted a comment)
  • -unavailable- added by pot (Posted a comment)
  • -unavailable- added by tmacchant (Submitted the item)
  • -unavailable- added by tmacchant
  • -unavailable- added by tmacchant
  •  

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

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 6 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 20 Apr 2016 10:33:35 PM UTCmtmillerStatusNone=>Fixed
      Open/ClosedOpen=>Closed
    Wed 20 Apr 2016 09:04:03 AM UTCmmuetzelAttached File-=>Added bug47690_v2.patch, #36970
    Mon 18 Apr 2016 04:08:41 PM UTCmmuetzelAttached File-=>Added bug47690.patch, #36958
    Thu 14 Apr 2016 12:55:12 AM UTCtmacchantCarbon-Copy-=>Added francesco potortì
      Carbon-Copy-=>Added mike miller

    Back to the top


    Powered by Savane 3.1-cleanup1