sherpa is hosted by Hepforge, IPPP Durham
close Warning: Can't synchronize with repository "(default)" (/hepforge/svn/sherpa does not appear to be a Subversion repository.). Look in the Trac log for more information.
Modify

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#300 closed defect (fixed)

Runtime crash in Ahadic on Mac OS 10.9

Reported by: Frank Siegert Owned by: Frank Krauss
Priority: major Milestone: rel-2.2.0
Component: Unknown Version: 2.1.1
Keywords: Cc: David.Hall@physics.ox.ac.uk, Felix.Socher@mailbox.tu-dresden.de

Description

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.;

Attachments (1)

iterator.patch (1.2 KB) - added by Frank Siegert 5 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 5 years ago by 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

comment:2 Changed 5 years ago by holsch

Resolution: fixed
Status: newclosed

comment:3 Changed 5 years ago by David.Hall@physics.ox.ac.uk

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

Thanks, David

comment:4 Changed 5 years ago by 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

Changed 5 years ago by Frank Siegert

Attachment: iterator.patch added

comment:5 Changed 5 years ago by David.Hall@physics.ox.ac.uk

Hi Frank,

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

Cheers, David

comment:6 Changed 5 years ago by 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.

Modify Ticket

Change Properties
Action
as closed The owner will remain Frank Krauss.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.