Ticket #1245 (closed bug: duplicate)

Opened 5 years ago

Last modified 4 years ago

src/pmc/integer.pmc: Check overflow for -maxint in absolute()

Reported by: jkeenan Owned by: Paul C. Anagnostopoulos
Priority: normal Milestone:
Component: core Version: 1.7.0
Severity: medium Keywords:
Cc: whiteknight cotto Notfound Language:
Patch status: Platform:

Description

This ticket moves to the Trac system discussion of an issue previously found in RT at  RT #46635.

Here is the relevant section of src/pmc/integer.pmc:

1260     VTABLE PMC *absolute(PMC *dest) {
1261         const INTVAL a = abs(SELF.get_integer());
1262 
1263         /* RT #46635 overflow for -maxint */
1264         dest = pmc_new(INTERP, VTABLE_type(INTERP, SELF));
1265 
1266         VTABLE_set_integer_native(INTERP, dest, a);
1267         return dest;
1268 
1269     }

In RT, Paul Cochrane commented: "I think [this] means that before setting the integer value we need to check here for overflow of -maxint (or at least numbers which go over the maximum integer value allowed on the current platform, or something like that)."

Discussion among whiteknight, cotto and NotFound followed. The last comment from NotFound was:

"The current implementation is wrong. abs takes and returns an int, but parrot INTVAL is not guaranteed to be int.

"Even worse, 'Trying to take the absolute value of the most negative integer is not defined' (from the linux man page, and the C standard if I remember well). So is not granted that we can take decisions based on the result of abs, must be done before.

"To obtain a safe implementation, abs must be avoided."

Change History

Changed 4 years ago by Paul C. Anagnostopoulos

  • status changed from new to assigned
  • owner set to Paul C. Anagnostopoulos

Changed 4 years ago by Paul C. Anagnostopoulos

It appears that the negate operation has the same problem.

Changed 4 years ago by Paul C. Anagnostopoulos

Imagine how happy I'll be to jump into this once I can find Windows binaries for GMP v4.1.4 or later.

Changed 4 years ago by Paul C. Anagnostopoulos

  • status changed from assigned to closed
  • resolution set to duplicate

This is the same problem as reported in  http://trac.parrot.org/parrot/ticket/1616, so I'll switch the discussion to that ticket and close this one.

Note: See TracTickets for help on using tickets.