Skip to content

Fix #17: segfault due indexing out-of-bounds in multistats

There were a bugs in the functions update_trigger_fars and cohfar_assignfar_transform_ip.

The first bug is in update_trigger_fars. It was found in Luke's add_kagra branch. When running 2 disabled ifos, long runs crashed with segfaults. (See #17 (closed))

update_trigger_fars sets the combined and single fars for each trigger, based on its snr, chisq, and the background stats. However, the array indexing of the background stats didn't consider unused ifos (see #3 for some examples). These stats are arrays with an entry for each included IFO (in order HLV) followed by the combined stats. So if Virgo is excluded, but we loop over all IFOs anyway (trying to read sngl_fars), the loop would see the sngl_far of H, then L, then the combined fars.
Thankfully we're only setting the sngl_far of the trigger from this, and Virgo is the last IFO in our array, so any tests that excluded virgo would simply have copied the combined background stats into virgo's fars.

I've used the pattern described in #3 as a simple refactor and fix.

The second bug is with cohfar_assignfar_transform_ip. Before even running update_trigger_fars we check cur_stats->nevent > MIN_BACKGROUND_NEVENT).
cur_stats is meant to be the combined "1 week" background stats. Prior to O4-dev, it was getting cur_stats using assignfar->nifo, which should index straight into the combined stats. However, I changed it to enabled_ifos, which is a local variable pulled from the trigger itself, and which excludes IFOs with sngl_snr < EPSILON. So nevent may have been pulled from a single ifo's background stats instead. I'm unsure if this is likely to have caused any problems with tests, I'll need to check where we set nevents, it's possible it's identical for all stats.

Merge request reports