Runtime crash in Ahadic on Mac OS 10.9

[Frank Siegert]

This is a followup to the second part of #279. Felix (in CC) has been able to reproduce this, it is still present in 2.1.1. He used lldb to provide more debugging information, from which it becomes relatively clear where it happens. The backtrace is

    frame #4: 0x0000000100280f35 libAhadicTools.0.dylib`AHADIC::Soft_Cluster_Handler::TransformWeight(AHADIC::Cluster*, ATOOLS::Flavour&, bool const&, bool const&) [inlined] std::__1::__map_iterator<std::__1::__tree_iterator<std::__1::__value_type<ATOOLS::Flavour, double>, std::__1::__tree_node<std::__1::__value_type<ATOOLS::Flavour, double>, void*>*, long> >::operator--((null)=<unavailable>) at map:695
    frame #5: 0x0000000100280f35 libAhadicTools.0.dylib`AHADIC::Soft_Cluster_Handler::TransformWeight(this=0x000000010199ba50, cluster=<unavailable>, hadron=0x00007fff5fbfe0f8, lighter=<unavailable>, enforce=0x00007fff5fbfe09e) + 965 at Soft_Cluster_Handler.C:564
    frame #6: 0x000000010027de18 libAhadicTools.0.dylib`AHADIC::Soft_Cluster_Handler::UpdateTransitions(this=0x000000010199ba50, clin=0x0000000101971d28) + 760 at Soft_Cluster_Handler.C:333
    frame #7: 0x000000010027d0bb libAhadicTools.0.dylib`AHADIC::Soft_Cluster_Handler::TreatClusterList(this=0x000000010199ba50, clin=0x0000000101971d28, blob=0x0000000101bb5650) + 203 at Soft_Cluster_Handler.C:72
    frame #8: 0x00000001002d8647 libAhadicFormation.0.dylib`AHADIC::Cluster_Formation_Handler::ClustersToHadrons(this=0x000000010199b950, blob=<unavailable>) + 39 at Cluster_Formation_Handler.C:404
    frame #9: 0x00000001002d6e22 libAhadicFormation.0.dylib`AHADIC::Cluster_Formation_Handler::FormClusters(this=0x000000010199b950, blob=0x0000000101bb5650) + 306 at Cluster_Formation_Handler.C:85
    frame #10: 0x000000010025710f libAhadicMain.0.dylib`AHADIC::Ahadic::Hadronize(this=0x0000000101971d10, blob=0x0000000101bb5650, retry=0) + 111 at Ahadic.C:157

FrankK, the problem is this part in Ahadic's Soft_Cluster_Handler:

  Single_Transition_List * stl(stiter->second);
  Single_Transition_Siter  start(stl->begin()),siter;
  if (lighter) {
    if (hadron!=Flavour(kf_none)) {
      do {
        if (start->first==hadron) {
          siter = start;
          siter--;
          if ((++siter)!=stl->end()) start++;
          else return 0.;
          break;
        }
        else start++;
      } while (start!=stl->end());
    }
  ...
  }

The siter-- is applied also for the first time, were siter = start = stl->begin(). Am I missing something, or how is this supposed to work?

I'm not sure what should happen in the siter == stl->begin() case, and I have no idea what gcc is making out of this. In case it simply ignores the siter-- one could mimick it as follows, but I'm not sure whether that's the physics logic that applies here. FrankK, can you check how these lines can be fixed?

Index: AHADIC++/Tools/Soft_Cluster_Handler.C
===================================================================
--- AHADIC++/Tools/Soft_Cluster_Handler.C	(Revision 24084)
+++ AHADIC++/Tools/Soft_Cluster_Handler.C	(Arbeitskopie)
@@ -561,6 +561,7 @@
       do {
 	if (start->first==hadron) {
 	  siter = start;
+	  if (siter!=stl->begin()) siter--;
 	  if ((++siter)!=stl->end()) start++;
 	  else return 0.;

[jmonk]

Hi Sherpa team,

I'd just like to add my support for this bug to be fixed, as I recently discovered it myself on OS X.9 - I mentioned this to Frank S and Holger yesterday.

Decrementing an iterator beyond the beginning of a container gives undefined behaviour in C++, so even if it works with some compilers, it is not reproducible. In any case, the decrement here looks unnecessary, since it gets incremented again on the very next line! Why can it not simply be replaced with

siter = start;
if(siter != stl->end()) start++;
...

When I try this fix myself, there is no crash anymore, but there is a later similar crash in AMISIC++/Main/MI_Base.C in the destructor:

MI_Base::~MI_Base()
{
  for (String_MI_Base_Map::iterator nbit=s_bases->begin();
       nbit!=s_bases->end();++nbit) {
    if (nbit->first==m_name) {
      s_bases->erase(nbit--); // potentially calls decrement on s_bases->begin()
      break;
    }
  }

Again, it is calling the decrement operator on the beginning of a container. This also seems unnecessary to me, as it breaks out of the loop immediately afterwards and then nbit goes out of scope.

I hope this can be resolved soon, as Sherpa 2 is unusable on OS X at the moment (and the crash-free results on Linux may not be predictable).

cheers,

James

[David.Hall@physics.ox.ac.uk]

Hi, Would it be possible to post a patch for this fix, please?

Thanks, David

[Frank Siegert]

Hi David,

I'm not sure whether Holger is automatically cc'ed after commenting/closing this bug, so I have attached the patch, not only for the Ahadic issue originally reported here, but also for the Amisic one that James mentioned and which had been fixed in Sherpa's trunk a few weeks ago. Both are now also merged as bugfixes into the public rel-2-1-1 SVN branch (but the release tarball is unmodified, so you can get these fixes only from SVN).

Cheers, Frank

[David.Hall@physics.ox.ac.uk]

Hi Frank,

Thanks a lot! I have this implemented in homebrew-hep now

Cheers, David

[David.Hall@physics.ox.ac.uk]

P.S. I should mention that this finally solves the issue in the comments of #279 at my end.