From 1189cfd62a2d30f5e61c00183a387c3d767ed698 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Sat, 20 Aug 2016 20:32:10 -0400 Subject: [PATCH] Identify the few unsafe narrowing conversions from sampleCount... ... I believe this list of four places is exhaustive. There are many, many more safe narrowings that I examined. This resulted from changing the definition of sampleCount in my builds so that narrowing conversions failed to compile without some fixes, and I examined and fixed every place. The rest of that work is not yet shared. --- src/effects/SBSMSEffect.cpp | 14 +++++++++----- src/effects/nyquist/Nyquist.cpp | 24 +++++++++++++++++++----- src/effects/vamp/VampEffect.cpp | 7 ++++++- src/widgets/NumericTextCtrl.cpp | 4 +++- 4 files changed, 37 insertions(+), 12 deletions(-) diff --git a/src/effects/SBSMSEffect.cpp b/src/effects/SBSMSEffect.cpp index 596a4b678..eb34d2d56 100644 --- a/src/effects/SBSMSEffect.cpp +++ b/src/effects/SBSMSEffect.cpp @@ -334,11 +334,15 @@ bool EffectSBSMS::Process() (long)((float)(processPresamples)*(srTrack/srProcess))); rb.offset = start - trackPresamples; rb.end = trackEnd; - rb.iface = std::make_unique(rb.resampler.get(), - &rateSlide,&pitchSlide, - bPitchReferenceInput, - samplesToProcess,processPresamples, - rb.quality.get()); + rb.iface = std::make_unique + (rb.resampler.get(), &rateSlide, &pitchSlide, + bPitchReferenceInput, + // UNSAFE_SAMPLE_COUNT_TRUNCATION + // The argument type is only long! + static_cast ( static_cast ( + samplesToProcess ) ), + processPresamples, + rb.quality.get()); } Resampler resampler(outResampleCB,&rb,outSlideType); diff --git a/src/effects/nyquist/Nyquist.cpp b/src/effects/nyquist/Nyquist.cpp index 0d5e4722e..bc30862fc 100644 --- a/src/effects/nyquist/Nyquist.cpp +++ b/src/effects/nyquist/Nyquist.cpp @@ -86,7 +86,7 @@ enum }; // Protect Nyquist from selections greater than 2^31 samples (bug 439) -#define NYQ_MAX_LEN ((sampleCount) 2147483647) +#define NYQ_MAX_LEN (std::numeric_limits::max()) #define UNINITIALIZED_CONTROL ((double)99999999.99) @@ -630,8 +630,12 @@ bool NyquistEffect::Process() mCurLen = (sampleCount)(end - mCurStart[0]); if (mCurLen > NYQ_MAX_LEN) { - wxMessageBox(_("Selection too long for Nyquist code.\nMaximum allowed selection is 2147483647 samples\n(about 13.5 hours at 44100 Hz sample rate)."), - _("Nyquist Error"), wxOK | wxCENTRE); + float hours = (float)NYQ_MAX_LEN / (44100 * 60 * 60); + const auto message = wxString::Format( +_("Selection too long for Nyquist code.\nMaximum allowed selection is %ld samples\n(about %.1f hours at 44100 Hz sample rate)."), + (long)NYQ_MAX_LEN, hours + ); + wxMessageBox(message, _("Nyquist Error"), wxOK | wxCENTRE); if (!mProjectChanged) em.SetSkipStateFlag(true); return false; @@ -959,11 +963,21 @@ bool NyquistEffect::ProcessOne() nyx_set_audio_params(mCurTrack[0]->GetRate(), 0); } else { - nyx_set_audio_params(mCurTrack[0]->GetRate(), mCurLen); + // UNSAFE_SAMPLE_COUNT_TRUNCATION + // Danger! Truncation of long long to long! + // Don't say we didn't warn you! + + // Note mCurLen was elsewhere limited to mMaxLen, which is normally + // the greatest long value, and yet even mMaxLen may be experimentally + // increased with a nyquist comment directive. + // See the parsing of "maxlen" + + auto curLen = (long)(static_cast(mCurLen)); + nyx_set_audio_params(mCurTrack[0]->GetRate(), curLen); nyx_set_input_audio(StaticGetCallback, (void *)this, mCurNumChannels, - mCurLen, mCurTrack[0]->GetRate()); + curLen, mCurTrack[0]->GetRate()); } // Restore the Nyquist sixteenth note symbol for Generate plugins. diff --git a/src/effects/vamp/VampEffect.cpp b/src/effects/vamp/VampEffect.cpp index 22e156386..837b8ecba 100644 --- a/src/effects/vamp/VampEffect.cpp +++ b/src/effects/vamp/VampEffect.cpp @@ -522,7 +522,12 @@ bool VampEffect::Process() } } - Vamp::RealTime timestamp = Vamp::RealTime::frame2RealTime(ls, (int)(mRate + 0.5)); + // UNSAFE_SAMPLE_COUNT_TRUNCATION + // Truncation in case of very long tracks! + Vamp::RealTime timestamp = Vamp::RealTime::frame2RealTime( + (long) static_cast( ls ), + (int)(mRate + 0.5) + ); Vamp::Plugin::FeatureSet features = mPlugin->process(data, timestamp); AddFeatures(ltrack, features); diff --git a/src/widgets/NumericTextCtrl.cpp b/src/widgets/NumericTextCtrl.cpp index 62ec01f41..d7eacfc2c 100644 --- a/src/widgets/NumericTextCtrl.cpp +++ b/src/widgets/NumericTextCtrl.cpp @@ -848,7 +848,9 @@ void NumericConverter::ValueToControls(double rawValue, bool nearest /* = true * } else { if (t_int >= 0) { - value = (t_int / mFields[i].base); + // UNSAFE_SAMPLE_COUNT_TRUNCATION + // truncation danger! + value = (static_cast( t_int ) / mFields[i].base); if (mFields[i].range > 0) value = value % mFields[i].range; }