"Perl_newSVpvf("%lld")" is broken

[hv@crypt.org]

hv@crypt.org wrote:
:David Landgren <david@landgren.net> wrote:
::This Fortnight on perl5-porters - 28 September-12 October 2008
:[...]
::"Perl_newSVpvf("%lld")" is broken
::
::   One other item that fell out of the Y2038+ code was that, on 32-bit
::   platforms, "printf("%lld\n", 2**32)" gives a result of -1. Michael and
::   admitted to being terrified by the code and hoped someone else could
::   dig down through it and figure out where the problem lay.
::
::     quad wrangle
::     http://xrl.us/oudxw
:
:The problem is combo-ifdefs. First we get this:
:
:        case 'l':
:#if defined(HAS_QUAD) || defined(HAS_LONG_DOUBLE)
:            if (*(q + 1) == 'l') {      /* lld, llf */
:                intsize = 'q';
:                q += 2; 
:                break;
:             }
:#endif
:
:and then we get this (for the %d case):
:                default:        iv = tiv; break;
:#ifdef HAS_QUAD
:                case 'q':       iv = (Quad_t)tiv; break;
:#endif
:
:So when HAS_LONG_DOUBLE but not HAS_QUAD, we set intsize='q', but then
:silently go to the default (%d) case at the point we consume it.

Patch below (over blead@34775) should do the job. I'm not sure how to test
it, since results will be conditional on HAS_QUAD, but existing tests are
not broken by the patch.

:(It is also arguable that "%lld" isn't the most useful output, but if that
:is changed thought should be given to making the analogous change for %llf.)

I didn't change that.

Hugo
--- sv.c.old	2008-11-07 22:37:34.000000000 +0000
+++ sv.c	2008-11-08 12:52:17.000000000 +0000
@@ -9557,8 +9557,11 @@
 		case 'l':	iv = va_arg(*args, long); break;
 		case 'V':	iv = va_arg(*args, IV); break;
 		default:	iv = va_arg(*args, int); break;
+		case 'q':
 #ifdef HAS_QUAD
-		case 'q':	iv = va_arg(*args, Quad_t); break;
+				iv = va_arg(*args, Quad_t); break;
+#else
+				goto unknown;
 #endif
 		}
 	    }
@@ -9569,8 +9572,11 @@
 		case 'l':	iv = (long)tiv; break;
 		case 'V':
 		default:	iv = tiv; break;
+		case 'q':
 #ifdef HAS_QUAD
-		case 'q':	iv = (Quad_t)tiv; break;
+				iv = (Quad_t)tiv; break;
+#else
+				goto unknown;
 #endif
 		}
 	    }
@@ -9642,8 +9648,11 @@
 		case 'l':  uv = va_arg(*args, unsigned long); break;
 		case 'V':  uv = va_arg(*args, UV); break;
 		default:   uv = va_arg(*args, unsigned); break;
+		case 'q':
 #ifdef HAS_QUAD
-		case 'q':  uv = va_arg(*args, Uquad_t); break;
+			   uv = va_arg(*args, Uquad_t); break;
+#else
+			   goto unknown;
 #endif
 		}
 	    }
@@ -9654,8 +9663,11 @@
 		case 'l':	uv = (unsigned long)tuv; break;
 		case 'V':
 		default:	uv = tuv; break;
+		case 'q':
 #ifdef HAS_QUAD
-		case 'q':	uv = (Uquad_t)tuv; break;
+				uv = (Uquad_t)tuv; break;
+#else
+				goto unknown;
 #endif
 		}
 	    }
@@ -9941,8 +9953,11 @@
 		default:	*(va_arg(*args, int*)) = i; break;
 		case 'l':	*(va_arg(*args, long*)) = i; break;
 		case 'V':	*(va_arg(*args, IV*)) = i; break;
+		case 'q':
 #ifdef HAS_QUAD
-		case 'q':	*(va_arg(*args, Quad_t*)) = i; break;
+				*(va_arg(*args, Quad_t*)) = i; break;
+#else
+				goto unknown;
 #endif
 		}
 	    }

---

Follow-Ups from:

Marcus Holland-Moritz <mhx-perl@gmx.net>

---

[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index][Thread Index][Top&Search][Original]

---