Bug 338797 - SEGV crash when umbrello is opening AIXM files
Summary: SEGV crash when umbrello is opening AIXM files
Status: RESOLVED FIXED
Alias: None
Product: umbrello
Classification: Applications
Component: general (show other bugs)
Version: Git
Platform: Compiled Sources Linux
: NOR major
Target Milestone: ---
Assignee: Umbrello Development Group
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-09-04 04:57 UTC by M Hounsell at ACFR
Modified: 2014-09-09 05:32 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In: 4.14.1


Attachments
Debugger Screen - showing local variables (337.33 KB, image/png)
2014-09-04 04:58 UTC, M Hounsell at ACFR
Details
The program before crash - not the diagrams that have opened. (121.79 KB, image/png)
2014-09-04 04:58 UTC, M Hounsell at ACFR
Details

Note You need to log in before you can comment on or make changes to this bug.
Description M Hounsell at ACFR 2014-09-04 04:57:12 UTC
The program causes a SEGV while it is displaying all the diagrams contained in the below model. The program has not yet successfully viewed the model. The model was created using Rational Rose.

Source Model http://www.aixm.aero/gallery/content/public/AIXM51/AIXM-5-1-20100201-uml.zip

See https://bugs.kde.org/show_bug.cgi?id=338536

#0  ClassifierWidget::displayedMembers(this = 0x567b860, this@entry = 0x567b860, ot = UMLObject::ot_Attribute, ot@entry = UMLObject::ot_Attribute) at /home/mhounsell/src/umbrello/umbrello/widgets/classifierwidget.cpp:448
#1  ClassifierWidget::displayedAttributes(this = 0x567b860, this@entry = 0x567b860) at /home/mhounsell/src/umbrello/umbrello/widgets/classifierwidget.cpp:608
#2  ClassifierWidget::paint(this = 0x567b860, painter = 0x7fffffffaa70, option = 0x582e648, widget = 0x42fa040) at /home/mhounsell/src/umbrello/umbrello/widgets/classifierwidget.cpp:750
#3  QGraphicsScenePrivate::draw(this = 0x582e420, this@entry = 0x582e420, item = 0x567b870, item@entry = 0x567b870, painter = 0x7fffffffaa70, painter@entry = 0x7fffffffaa70, viewTransform = 0x7fffffffab20, viewTransform@entry = 0x7fffffffab20, transformPtr = 0x7fffffffa7b0, transformPtr@entry = 0x7fffffffa7b0, exposedRegion = 0x56d09a8, exposedRegion@entry = 0x56d09a8, widget = 0x42fa040, opacity = 1, opacity@entry = 1, effectTransform = 0x0, effectTransform@entry = 0x0, wasDirtyParentSceneTransform = false, wasDirtyParentSceneTransform@entry = false, drawItem = true) at graphicsview/qgraphicsscene.cpp:4964
#4  QGraphicsScenePrivate::drawSubtreeRecursive(this = 0x582e420, this@entry = 0x582e420, item = 0x567b870, painter = 0x7fffffffaa70, painter@entry = 0x7fffffffaa70, viewTransform = 0x7fffffffab20, viewTransform@entry = 0x7fffffffab20, exposedRegion = 0x56d09a8, exposedRegion@entry = 0x56d09a8, widget = 0x42fa040, widget@entry = 0x42fa040, parentOpacity = 1, parentOpacity@entry = 1, effectTransform = 0x0, effectTransform@entry = 0x0) at graphicsview/qgraphicsscene.cpp:4857
#5  QGraphicsScenePrivate::drawItems(this = 0x582e420, painter = 0x7fffffffaa70, painter@entry = 0x7fffffffaa70, viewTransform = 0x7fffffffab20, viewTransform@entry = 0x7fffffffab20, exposedRegion = 0x56d09a8, exposedRegion@entry = 0x56d09a8, widget = 0x42fa040) at graphicsview/qgraphicsscene.cpp:4739
#6  QGraphicsView::paintEvent(this = <optimised out>, event = <optimised out>) at graphicsview/qgraphicsview.cpp:3471
#7  QWidget::event(this = 0x455c420, this@entry = 0x455c420, event = 0x7fffffffb100, event@entry = 0x7fffffffb100) at kernel/qwidget.cpp:8533
#8  QFrame::event(this = 0x455c420, e = 0x7fffffffb100) at widgets/qframe.cpp:557
#9  QGraphicsView::viewportEvent(this = 0x455c420, event = 0x7fffffffb100) at graphicsview/qgraphicsview.cpp:2866
#10  QCoreApplicationPrivate::sendThroughObjectEventFilters(this = 0x9ddae0, this@entry = 0x9ddae0, receiver = 0x42fa040, receiver@entry = 0x42fa040, event = 0x7fffffffb100, event@entry = 0x7fffffffb100) at kernel/qcoreapplication.cpp:1063
#11  QApplicationPrivate::notify_helper(this = 0x9ddae0, this@entry = 0x9ddae0, receiver = 0x42fa040, receiver@entry = 0x42fa040, e = 0x7fffffffb100, e@entry = 0x7fffffffb100) at kernel/qapplication.cpp:4563
#12  QApplication::notify(this = 0x7fffffffdd60, this@entry = 0x7fffffffdd60, receiver = 0x42fa040, receiver@entry = 0x42fa040, e = 0x7fffffffb100, e@entry = 0x7fffffffb100) at kernel/qapplication.cpp:4353
#13  KApplication::notify(this = 0x7fffffffdd60, receiver = 0x42fa040, event = 0x7fffffffb100) at ../../kdeui/kernel/kapplication.cpp:311
#14  QCoreApplication::notifyInternal(this = 0x7fffffffdd60, receiver = 0x42fa040, receiver@entry = 0x42fa040, event = 0x7fffffffb100, event@entry = 0x7fffffffb100) at kernel/qcoreapplication.cpp:953
#15  sendSpontaneousEvent(event = 0x7fffffffb100, receiver = 0x42fa040) at ../../include/QtCore/../../src/corelib/kernel/qcoreapplication.h:234
#16  QWidgetPrivate::drawWidget(this = 0x582de10, this@entry = 0x582de10, pdev = 0x1015120, rgn = , offset = , flags = 4, sharedPainter = 0x0, sharedPainter@entry = 0x0, backingStore = 0xca5090, backingStore@entry = 0xca5090) at kernel/qwidget.cpp:5599
#17  QWidgetBackingStore::sync(this = 0xca5090) at painting/qbackingstore.cpp:1365
#18  QWidgetPrivate::syncBackingStore(this = 0xca01a0, this@entry = 0xca01a0) at kernel/qwidget.cpp:1894
#19  QWidget::event(this = 0xc9fd90, this@entry = 0xc9fd90, event = 0x43d75e0, event@entry = 0x43d75e0) at kernel/qwidget.cpp:8680
#20  QMainWindow::event(this = 0xc9fd90, this@entry = 0xc9fd90, event = 0x43d75e0, event@entry = 0x43d75e0) at widgets/qmainwindow.cpp:1478
#21  KMainWindow::event(this = 0xc9fd90, this@entry = 0xc9fd90, ev = 0x43d75e0, ev@entry = 0x43d75e0) at ../../kdeui/widgets/kmainwindow.cpp:1084
#22  KXmlGuiWindow::event(this = 0xc9fd90, ev = 0x43d75e0) at ../../kdeui/xmlgui/kxmlguiwindow.cpp:126
#23  QApplicationPrivate::notify_helper(this = 0x9ddae0, this@entry = 0x9ddae0, receiver = 0xc9fd90, receiver@entry = 0xc9fd90, e = 0x43d75e0, e@entry = 0x43d75e0) at kernel/qapplication.cpp:4567
#24  QApplication::notify(this = 0x7fffffffdd60, this@entry = 0x7fffffffdd60, receiver = 0xc9fd90, receiver@entry = 0xc9fd90, e = 0x43d75e0, e@entry = 0x43d75e0) at kernel/qapplication.cpp:4353
#25  KApplication::notify(this = 0x7fffffffdd60, receiver = 0xc9fd90, event = 0x43d75e0) at ../../kdeui/kernel/kapplication.cpp:311
/**
 * Return the number of displayed members of the given ObjectType.
 * Takes into consideration m_showPublicOnly but not other settings.
 */
int ClassifierWidget::displayedMembers(UMLObject::ObjectType ot) const
{
    int count = 0;
    UMLClassifier *classifier = this->classifier();
    if (!classifier)
        return count;
    UMLClassifierListItemList list = classifier->getFilteredList(ot);
    foreach (UMLClassifierListItem *m, list) {
      if (!(visualProperty(ShowPublicOnly) && m->visibility() != Uml::Visibility::Public))
            count++;
    }
    return count;
}



Reproducible: Always

Steps to Reproduce:
Get the above model to disk.

1. Open umbrello
2. Open the model
3. WAIT - let the model open each diagram.

Actual Results:  
SEGV

Expected Results:  
All diagrams opened and the program waits for input.

To be attached.
Comment 1 M Hounsell at ACFR 2014-09-04 04:58:08 UTC
Created attachment 88554 [details]
Debugger Screen - showing local variables
Comment 2 M Hounsell at ACFR 2014-09-04 04:58:51 UTC
Created attachment 88555 [details]
The program before crash - not the diagrams that have opened.
Comment 3 M Hounsell at ACFR 2014-09-04 05:01:35 UTC
Git pull and make clean at 20140904T0430Z.
Comment 4 Ralf Habacker 2014-09-04 17:10:51 UTC
(In reply to M Hounsell at ACFR from comment #3)
> Git pull and make clean at 20140904T0430Z.

could not reproduce with master commit  e66a2cbbedf6883f1ddfb68979da7ca0cf2ab071, which fixes an import issue.
Comment 5 Oliver Kellogg 2014-09-04 20:31:24 UTC
(In reply to Ralf Habacker from comment #4)
> could not reproduce with master commit 
> e66a2cbbedf6883f1ddfb68979da7ca0cf2ab071, which fixes an import issue.

It seems your import fix has collaterally fixed this bug.
I tried master@a060b16 (2014-08-30) which was still fine, and I pulled again now; no problem observed on either revision.

(In reply to M Hounsell at ACFR)
> It would also be nice if it did not open _every_ diagram.

That's easy, just disable "Use tabbed diagrams" in Settings -> Configure Umbrello...
Comment 6 M Hounsell at ACFR 2014-09-05 00:59:31 UTC
2014-09-05 00:52:30 Z

Ok, I just did a git pull and a make clean  and it reproduced exactley the same, twice. 
Tried again with "Use tabbed diagrams" disabled but same SEGV.

1997: /home/mhounsell/src/umbrello/ $> git pull
remote: Counting objects: 7, done.
remote: Compressing objects: 100% (4/4), done.
remote: Total 4 (delta 3), reused 0 (delta 0)
Unpacking objects: 100% (4/4), done.
From git://anongit.kde.org/umbrello
   86fdf75..e66a2cb  master     -> origin/master
Updating 86fdf75..e66a2cb
Fast-forward
 umbrello/object_factory.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
1998: /home/mhounsell/src/umbrello/ $> cd build/
1999: /home/mhounsell/.../umbrello/build/ $> make
Comment 7 Oliver Kellogg 2014-09-05 05:15:19 UTC
(In reply to M Hounsell at ACFR from comment #6)
> 2014-09-05 00:52:30 Z
> 
> Ok, I just did a git pull and a make clean  and it reproduced exactley the
> same, twice. 
> Tried again with "Use tabbed diagrams" disabled but same SEGV.
> 

Although I cannot reproduce the crash I had a closer look at the backtrace, and in fact there is a problem:

UMLClassifier *ClassifierWidget::classifier() const
{
    return static_cast<UMLClassifier*>(m_umlObject);
}

Since the recent class-or-package modifications, m_umlObject may also point to UMLPackage - which means that the static_cast to UMLClassifier* is invalid.
I am making the required adjustments and will commit later today.
Comment 8 Ralf Habacker 2014-09-05 17:08:26 UTC
(In reply to Oliver Kellogg from comment #7)
> (In reply to M Hounsell at ACFR from comment #6)
> > 2014-09-05 00:52:30 Z
> > 
> > Ok, I just did a git pull and a make clean  and it reproduced exactley the
> > same, twice. 
> > Tried again with "Use tabbed diagrams" disabled but same SEGV.
> > 
> 
> Although I cannot reproduce the crash I had a closer look at the backtrace,
> and in fact there is a problem:
> 
> UMLClassifier *ClassifierWidget::classifier() const
> {
>     return static_cast<UMLClassifier*>(m_umlObject);
> }
> 
> Since the recent class-or-package modifications, m_umlObject may also point
> to UMLPackage - which means that the static_cast to UMLClassifier* is
> invalid.

class-or-package uml objects are created of type UMLClassifier. A later retype to a package only sets the WidgetBase class base type member to UMLPackage, the object itself is still an UMLClassifier. A cast to UMLClassifier therefore should not fail. See CppTree2Uml::parseNamespace for more details. 




> I am making the required adjustments and will commit later today.
Comment 9 Oliver Kellogg 2014-09-05 18:11:16 UTC
Git commit 9a9f89f89a12305bd976ac158605d2f142954fc0 by Oliver Kellogg.
Committed on 05/09/2014 at 18:11.
Pushed by okellogg into branch 'master'.

Address http://bugs.kde.org/show_bug.cgi?id=338797#c7,
static_cast<UMLClassifier*>(m_umlObject) in ClassifierWidget::classifier()
may be invalid:

umbrello/widgets/classifierwidget.cpp
- In function classifier(), replace static_cast by dynamic_cast.
- In the entire class implementation, ensure return value from classifier()
  is non NULL before dereferencing.

Since I could not reproduce the crash, I leave this bug REOPENED until the
fix is verified.

M  +50   -27   umbrello/widgets/classifierwidget.cpp

http://commits.kde.org/umbrello/9a9f89f89a12305bd976ac158605d2f142954fc0
Comment 10 Oliver Kellogg 2014-09-05 18:47:17 UTC
(In reply to Ralf Habacker from comment #8)
> (In reply to Oliver Kellogg from comment #7)
> > 
> > Since the recent class-or-package modifications, m_umlObject may also point
> > to UMLPackage - which means that the static_cast to UMLClassifier* is
> > invalid.
> 
> class-or-package uml objects are created of type UMLClassifier. A later
> retype to a package only sets the WidgetBase class base type member to
> UMLPackage, the object itself is still an UMLClassifier. A cast to
> UMLClassifier therefore should not fail. See CppTree2Uml::parseNamespace for
> more details. 

Ah, I forgot to do a refresh before committing and did not see your comment, sorry.

Are you saying that the new constructor,
  ClassifierWidget(UMLScene * scene, UMLPackage *o)
is only ever called with UMLClassifier* as the second argument?
I guess I was barking up the wrong tree then.
Comment 11 Ralf Habacker 2014-09-05 19:33:05 UTC
(In reply to Oliver Kellogg from comment #10)
> (In reply to Ralf Habacker from comment #8)
> > (In reply to Oliver Kellogg from comment #7)
> > > 
> > > Since the recent class-or-package modifications, m_umlObject may also point
> > > to UMLPackage - which means that the static_cast to UMLClassifier* is
> > > invalid.
> > 
> > class-or-package uml objects are created of type UMLClassifier. A later
> > retype to a package only sets the WidgetBase class base type member to
> > UMLPackage, the object itself is still an UMLClassifier. A cast to
> > UMLClassifier therefore should not fail. See CppTree2Uml::parseNamespace for
> > more details. 
> 
> Ah, I forgot to do a refresh before committing and did not see your comment,
> sorry.
> 
> Are you saying that the new constructor,
>   ClassifierWidget(UMLScene * scene, UMLPackage *o)
> is only ever called with UMLClassifier* as the second argument?
yes, the constructor was added at an early state of the class-or-package support, when I initial tried to retype to a real uml package object,  but stopped this approach because of too many issues. I should have removed it before merging the branch into master
> I guess I was barking up the wrong tree then.
May be not: What is left over to handle UmlPackage with ClassifierWidget complete ? 
Then PackageWidget could be dropped and umbrello could have a feature to retype any Package to a Classifier and vice versa on the fly.
Comment 12 M Hounsell at ACFR 2014-09-08 02:50:39 UTC
I did a Git pull this morning and there was an update to UML Classifier.
I built and ran the software with the safe test files - first under the debugger and then standalone.
In both instances the files loaded until done and I was able to look at the Route diagrams. 
It looks like the SEGV has been fixed.
Thus loading the AIXM UML and diagrams seems to be working.
Thanks.
Comment 13 Oliver Kellogg 2014-09-08 04:36:27 UTC
(In reply to Ralf Habacker from comment #11)
> (In reply to Oliver Kellogg from comment #10)

> [...]
> > Are you saying that the new constructor,
> >   ClassifierWidget(UMLScene * scene, UMLPackage *o)
> > is only ever called with UMLClassifier* as the second argument?
> yes, the constructor was added at an early state of the class-or-package
> support, when I initial tried to retype to a real uml package object,  but
> stopped this approach because of too many issues. I should have removed it
> before merging the branch into master

Are you still working on this?
I found that the new constructor is called at least at these locations:
Widget_Factory::createWidget() (umbrello/widgets/widget_factory.cpp line 90)
Widget_Factory::makeWidgetFromXMI() (umbrello/widgets/widget_factory.cpp line 258)

(In reply to M Hounsell at ACFR from comment #12)
> [...]
> It looks like the SEGV has been fixed.
> Thus loading the AIXM UML and diagrams seems to be working.

Many thanks for checking.
Comment 14 Oliver Kellogg 2014-09-09 05:30:43 UTC
Git commit d98c3db9f23b89e8743115db8a09bcc046070e03 by Oliver Kellogg.
Committed on 05/09/2014 at 18:11.
Pushed by okellogg into branch 'KDE/4.14'.

Address http://bugs.kde.org/show_bug.cgi?id=338797#c7,
static_cast<UMLClassifier*>(m_umlObject) in ClassifierWidget::classifier()
may be invalid:

umbrello/widgets/classifierwidget.cpp
- In function classifier(), replace static_cast by dynamic_cast.
- In the entire class implementation, ensure return value from classifier()
  is non NULL before dereferencing.

Since I could not reproduce the crash, I leave this bug REOPENED until the
fix is verified.
FIXED-IN: 4.14.1

M  +50   -27   umbrello/widgets/classifierwidget.cpp

http://commits.kde.org/umbrello/d98c3db9f23b89e8743115db8a09bcc046070e03