From bf26bcdb3185fcd0632d7e4d4706a3a39fcd6602 Mon Sep 17 00:00:00 2001
From: David Abdurachmanov <David.Abdurachmanov@cern.ch>
Date: Sun, 25 Oct 2015 10:04:05 +0100
Subject: [PATCH] Use std::fmin instead of std::min in HcalTimeSlew::delay

SIGSEGV (Segfault) was detected in `HcalDeterministicFit::getLandauFrac`
with ICC 16.0.0 20150815.

Segfault happens in RecoLocalCalo/HcalRecAlgos/src/HcalDeterministicFit.cc:42

    42        sum= landauFrac[int(ceil(tStart+tsWidth))];

The function is called here in
RecoLocalCalo/HcalRecAlgos/interface/HcalDeterministicFit.h:112

    112       getLandauFrac(-tsShift4,-tsShift4+tsWidth,i4);

But `tsShift4` ends up being **NAN**:

    112       getLandauFrac(-tsShift4,-tsShift4+tsWidth,i4);
    (gdb) p tsShift4
    $3 = nan(0x400000)
    (gdb) p tsWidth
    $4 = 25
    (gdb) p i4
    $5 = 0

`HcalTimeSlew::delay` use a `log` function and a negative argument in
passed.

    (gdb) p fTimeSlew
    $9 = HcalTimeSlew::InputPars
    (gdb) p fTimeSlewBias
    $10 = HcalTimeSlew::Medium
    (gdb) p fpar0
    $11 = 12.299899999999999
    (gdb) p fpar1
    $12 = -2.1914199999999999
    (gdb) p fpar2 // par2
    $13 = 0
    (gdb) p inputCharge[4] // fC
    $14 = (__gnu_cxx::__alloc_traits<std::allocator<double> >::value_type &)
    @0x7fff38cf7e20: -0.08074517548084259

CalibCalorimetry/HcalAlgos/src/HcalTimeSlew.cc:23

    return std::min(cap, par0 + par1*log(fC+par2));

Thus we end up with `return std::min(6.0, NAN);`

`std::log` has direct description of how it works if implementation
supports IEEE:

    If the argument is negative, NaN is returned and FE_INVALID is
    raised.

What `std::min` should return in case one of arguments is `NAN` is not directly
defined in C++ standard. Standard requires `std::min` to return the
first argumnet (6.0 in our case) if arguments are equivalent. All
comparison with `NAN` will return `false`. Becasue of this `std::min`
implementation with GCC and Clang will return the first argument. `NAN`
is returned in ICC case.

According to IEEE 754:

    min(x,NaN) = min(NaN,x) = x

C++11 also introduced `std::fmin` from C standard, which provides a
direct instructions how `NAN` is handled:

    .. between a NaN and a numeric value, the numeric value is chosen)

`std::min` is generic and does not direclty talk about IEEE
floating-points.

`std::fmin` works fine in correctly between GCC, Clang and ICC.

ICC issue will be reported for Intel for further discussions.

Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=47706

Signed-off-by: David Abdurachmanov <David.Abdurachmanov@cern.ch>
---
 CalibCalorimetry/HcalAlgos/src/HcalTimeSlew.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/CalibCalorimetry/HcalAlgos/src/HcalTimeSlew.cc b/CalibCalorimetry/HcalAlgos/src/HcalTimeSlew.cc
index cafb4fff055b5..f1668dc8c6e9e 100644
--- a/CalibCalorimetry/HcalAlgos/src/HcalTimeSlew.cc
+++ b/CalibCalorimetry/HcalAlgos/src/HcalTimeSlew.cc
@@ -20,10 +20,10 @@ double HcalTimeSlew::delay(double fC, ParaSource source, BiasSetting bias, doubl
     return HcalTimeSlew::delay(fC, bias);
   }
   else if (source==InputPars) {
-    return std::min(cap, par0 + par1*log(fC+par2));
+    return std::fmin(cap, par0 + par1*log(fC+par2));
   }
   else if (source==Data || source==MC){
-    return std::min(cap,tspar0[source-1]+tspar1[source-1]*log(fC+tspar2[source-1]));
+    return std::fmin(cap,tspar0[source-1]+tspar1[source-1]*log(fC+tspar2[source-1]));
   }
   return 0;
 }