[ 
https://issues.apache.org/jira/browse/XERCESC-2088?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16683515#comment-16683515
 ] 

Philip Armstrong commented on XERCESC-2088:
-------------------------------------------

Let’s try again. Apologies for the comment / list spam.
{code:java}
Subject: [PATCH] Change dynamic_casts to virtual methods to avoid the need for
 rtti.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This requires that derived classes that inherit both from DOMNode and one of

  HasDOMNodeImpl
  HasDOMParentImpl
  HasDOMChildImpl

*must* implement hasDOMNodeImpl() and it’s neighbour virtual methods to return 
"this"
instead of nullptr as the methods on DOMNode do. Programmer beware!

diff --git a/src/xercesc/dom/DOMNode.hpp b/src/xercesc/dom/DOMNode.hpp
index 49d45f228..c6a155872 100644
--- a/src/xercesc/dom/DOMNode.hpp
+++ b/src/xercesc/dom/DOMNode.hpp
@@ -31,6 +31,9 @@ class DOMDocument;
 class DOMNamedNodeMap;
 class DOMNodeList;
 class DOMUserDataHandler;
+class HasDOMNodeImpl;
+class HasDOMParentImpl;
+class HasDOMChildImpl;
 
 /**
  * The <code>DOMNode</code> interface is the primary datatype for the entire
@@ -333,6 +336,16 @@ public:
     // -----------------------------------------------------------------------
     //  Node methods
     // -----------------------------------------------------------------------
+    /**
+     * casting methods
+     */
+    virtual HasDOMNodeImpl* hasDOMNodeImpl() { return nullptr; }
+    virtual const HasDOMNodeImpl* constHasDOMNodeImpl() const { return 
nullptr; }
+    virtual HasDOMParentImpl* hasDOMParentImpl() { return nullptr; }
+    virtual const HasDOMParentImpl* constHasDOMParentImpl() const { return 
nullptr; }
+    virtual HasDOMChildImpl* hasDOMChildImpl() { return nullptr; }
+    virtual const HasDOMChildImpl* constHasDOMChildImpl() const { return 
nullptr; }
+
     /**
      * Returns a duplicate of this node.
      *
diff --git a/src/xercesc/dom/impl/DOMAttrImpl.hpp 
b/src/xercesc/dom/impl/DOMAttrImpl.hpp
index 8c6a5ae7b..62bb4937d 100644
--- a/src/xercesc/dom/impl/DOMAttrImpl.hpp
+++ b/src/xercesc/dom/impl/DOMAttrImpl.hpp
@@ -47,8 +47,12 @@ class DOMElementImpl;
 class DOMTypeInfoImpl;
 
 class CDOM_EXPORT DOMAttrImpl: public DOMAttr, public HasDOMNodeImpl, public 
HasDOMParentImpl {
-
 public:
+    HasDOMNodeImpl* hasDOMNodeImpl() { return this; }
+    const HasDOMNodeImpl* constHasDOMNodeImpl() const { return this; }
+    HasDOMParentImpl* hasDOMParentImpl() { return this; }
+    const HasDOMParentImpl* constHasDOMParentImpl() const { return this; }
+
     DOMNodeImpl        fNode;
     DOMParentNode      fParent;
     const XMLCh       *fName;
diff --git a/src/xercesc/dom/impl/DOMCDATASectionImpl.hpp 
b/src/xercesc/dom/impl/DOMCDATASectionImpl.hpp
index d141deb27..cbc29c0cc 100644
--- a/src/xercesc/dom/impl/DOMCDATASectionImpl.hpp
+++ b/src/xercesc/dom/impl/DOMCDATASectionImpl.hpp
@@ -44,6 +44,12 @@ XERCES_CPP_NAMESPACE_BEGIN
 
 
 class CDOM_EXPORT DOMCDATASectionImpl: public DOMCDATASection, public 
HasDOMNodeImpl, public HasDOMChildImpl {
+public:
+    HasDOMNodeImpl* hasDOMNodeImpl() { return this; }
+    const HasDOMNodeImpl* constHasDOMNodeImpl() const { return this; }
+    HasDOMChildImpl* hasDOMChildImpl() { return this; }
+    const HasDOMChildImpl* constHasDOMChildImpl() const { return this; }
+
 protected:
     DOMNodeImpl           fNode;
     DOMChildNode          fChild;
diff --git a/src/xercesc/dom/impl/DOMCasts.hpp 
b/src/xercesc/dom/impl/DOMCasts.hpp
index 7d99dae29..b77ec1eb5 100644
--- a/src/xercesc/dom/impl/DOMCasts.hpp
+++ b/src/xercesc/dom/impl/DOMCasts.hpp
@@ -56,7 +56,8 @@ XERCES_CPP_NAMESPACE_BEGIN
 
 static inline const DOMNodeImpl *castToNodeImpl(const DOMNode *p)
 {
-    const HasDOMNodeImpl* pE = dynamic_cast<const HasDOMNodeImpl*>(p);
+    //const HasDOMNodeImpl* pE = dynamic_cast<const HasDOMNodeImpl*>(p);
+    const HasDOMNodeImpl* pE = p->constHasDOMNodeImpl();
     if (!pE || !pE->getNodeImpl()) {
         throw DOMException(DOMException::INVALID_STATE_ERR, 0, 
XMLPlatformUtils::fgMemoryManager);
     }
@@ -65,7 +66,8 @@ static inline const DOMNodeImpl *castToNodeImpl(const DOMNode 
*p)
 
 static inline DOMNodeImpl *castToNodeImpl(DOMNode *p)
 {
-    HasDOMNodeImpl *pE = dynamic_cast<HasDOMNodeImpl*>(p);
+    //HasDOMNodeImpl *pE = dynamic_cast<HasDOMNodeImpl*>(p);
+    HasDOMNodeImpl* pE = p->hasDOMNodeImpl();
     if (!pE || !pE->getNodeImpl()) {
         throw DOMException(DOMException::INVALID_STATE_ERR, 0, 
XMLPlatformUtils::fgMemoryManager);
     }
@@ -73,7 +75,8 @@ static inline DOMNodeImpl *castToNodeImpl(DOMNode *p)
 }
 
 static inline const DOMParentNode *castToParentImpl(const DOMNode *p) {
-    const HasDOMParentImpl *pE = dynamic_cast<const HasDOMParentImpl*>(p);
+    //const HasDOMParentImpl *pE = dynamic_cast<const HasDOMParentImpl*>(p);
+    const HasDOMParentImpl *pE = p->constHasDOMParentImpl();
     if (!pE || !pE->getParentNodeImpl()) {
         throw DOMException(DOMException::INVALID_STATE_ERR, 0, 
XMLPlatformUtils::fgMemoryManager);
     }
@@ -81,7 +84,8 @@ static inline const DOMParentNode *castToParentImpl(const 
DOMNode *p) {
 }
 
 static inline DOMParentNode *castToParentImpl(DOMNode *p) {
-    HasDOMParentImpl *pE = dynamic_cast<HasDOMParentImpl*>(p);
+    //HasDOMParentImpl *pE = dynamic_cast<HasDOMParentImpl*>(p);
+    HasDOMParentImpl *pE = p->hasDOMParentImpl();
     if (!pE || !pE->getParentNodeImpl()) {
         throw DOMException(DOMException::INVALID_STATE_ERR, 0, 
XMLPlatformUtils::fgMemoryManager);
     }
@@ -89,7 +93,8 @@ static inline DOMParentNode *castToParentImpl(DOMNode *p) {
 }
 
 static inline const DOMChildNode *castToChildImpl(const DOMNode *p) {
-    const HasDOMChildImpl *pE = dynamic_cast<const HasDOMChildImpl*>(p);
+    //const HasDOMChildImpl *pE = dynamic_cast<const HasDOMChildImpl*>(p);
+    const HasDOMChildImpl* pE = p->constHasDOMChildImpl();
     if (!pE || !pE->getChildNodeImpl()) {
         throw DOMException(DOMException::INVALID_STATE_ERR, 0, 
XMLPlatformUtils::fgMemoryManager);
     }
@@ -97,7 +102,8 @@ static inline const DOMChildNode *castToChildImpl(const 
DOMNode *p) {
 }
 
 static inline DOMChildNode *castToChildImpl(DOMNode *p) {
-    HasDOMChildImpl *pE = dynamic_cast<HasDOMChildImpl*>(p);
+    //HasDOMChildImpl *pE = dynamic_cast<HasDOMChildImpl*>(p);
+    HasDOMChildImpl* pE = p->hasDOMChildImpl();
     if (!pE || !pE->getChildNodeImpl()) {
         throw DOMException(DOMException::INVALID_STATE_ERR, 0, 
XMLPlatformUtils::fgMemoryManager);
     }
diff --git a/src/xercesc/dom/impl/DOMCommentImpl.hpp 
b/src/xercesc/dom/impl/DOMCommentImpl.hpp
index 45c6eedcc..466b05165 100644
--- a/src/xercesc/dom/impl/DOMCommentImpl.hpp
+++ b/src/xercesc/dom/impl/DOMCommentImpl.hpp
@@ -45,6 +45,11 @@ XERCES_CPP_NAMESPACE_BEGIN
 
 class CDOM_EXPORT DOMCommentImpl: public DOMComment, public HasDOMNodeImpl, 
public HasDOMChildImpl {
 public:
+    HasDOMNodeImpl* hasDOMNodeImpl() { return this; }
+    const HasDOMNodeImpl* constHasDOMNodeImpl() const { return this; }
+    HasDOMChildImpl* hasDOMChildImpl() { return this; }
+    const HasDOMChildImpl* constHasDOMChildImpl() const { return this; }
+
     DOMNodeImpl            fNode;
     DOMChildNode           fChild;
     DOMCharacterDataImpl   fCharacterData;
diff --git a/src/xercesc/dom/impl/DOMDocumentFragmentImpl.hpp 
b/src/xercesc/dom/impl/DOMDocumentFragmentImpl.hpp
index dfdd626ef..3bb11d080 100644
--- a/src/xercesc/dom/impl/DOMDocumentFragmentImpl.hpp
+++ b/src/xercesc/dom/impl/DOMDocumentFragmentImpl.hpp
@@ -42,6 +42,12 @@ XERCES_CPP_NAMESPACE_BEGIN
 
 class CDOM_EXPORT DOMDocumentFragmentImpl: public DOMDocumentFragment,
         public HasDOMNodeImpl, public HasDOMParentImpl {
+public:
+    HasDOMNodeImpl* hasDOMNodeImpl() { return this; }
+    const HasDOMNodeImpl* constHasDOMNodeImpl() const { return this; }
+    HasDOMParentImpl* hasDOMParentImpl() { return this; }
+    const HasDOMParentImpl* constHasDOMParentImpl() const { return this; }
+
 protected:
     DOMNodeImpl     fNode;
     DOMParentNode   fParent;
diff --git a/src/xercesc/dom/impl/DOMDocumentImpl.hpp 
b/src/xercesc/dom/impl/DOMDocumentImpl.hpp
index c748e0ad0..1187897fb 100644
--- a/src/xercesc/dom/impl/DOMDocumentImpl.hpp
+++ b/src/xercesc/dom/impl/DOMDocumentImpl.hpp
@@ -83,6 +83,12 @@ typedef RefStackOf<DOMNode>               DOMNodePtr;
 class CDOM_EXPORT DOMDocumentImpl: public XMemory, public DOMMemoryManager, 
public DOMDocument,
         public HasDOMNodeImpl, public HasDOMParentImpl {
 public:
+
+    HasDOMNodeImpl* hasDOMNodeImpl() { return this; }
+    const HasDOMNodeImpl* constHasDOMNodeImpl() const { return this; }
+    HasDOMParentImpl* hasDOMParentImpl() { return this; }
+    const HasDOMParentImpl* constHasDOMParentImpl() const { return this; }
+
     // -----------------------------------------------------------------------
     //  data
     // -----------------------------------------------------------------------
diff --git a/src/xercesc/dom/impl/DOMDocumentTypeImpl.hpp 
b/src/xercesc/dom/impl/DOMDocumentTypeImpl.hpp
index 3c3ba4976..c2c76b728 100644
--- a/src/xercesc/dom/impl/DOMDocumentTypeImpl.hpp
+++ b/src/xercesc/dom/impl/DOMDocumentTypeImpl.hpp
@@ -47,6 +47,14 @@ class DOMNamedNodeMapImpl;
 
 class CDOM_EXPORT DOMDocumentTypeImpl: public DOMDocumentType,
         public HasDOMNodeImpl, public HasDOMParentImpl, public HasDOMChildImpl 
{
+public:
+    HasDOMNodeImpl* hasDOMNodeImpl() { return this; }
+    const HasDOMNodeImpl* constHasDOMNodeImpl() const { return this; }
+    HasDOMParentImpl* hasDOMParentImpl() { return this; }
+    const HasDOMParentImpl* constHasDOMParentImpl() const { return this; }
+    HasDOMChildImpl* hasDOMChildImpl() { return this; }
+    const HasDOMChildImpl* constHasDOMChildImpl() const { return this; }
+
 protected:
     DOMNodeImpl          fNode;
     DOMParentNode        fParent;
diff --git a/src/xercesc/dom/impl/DOMElementImpl.hpp 
b/src/xercesc/dom/impl/DOMElementImpl.hpp
index e4eb193ec..568a98915 100644
--- a/src/xercesc/dom/impl/DOMElementImpl.hpp
+++ b/src/xercesc/dom/impl/DOMElementImpl.hpp
@@ -56,6 +56,13 @@ class DOMDocument;
 class CDOM_EXPORT DOMElementImpl: public DOMElement,
         public HasDOMNodeImpl, public HasDOMParentImpl, public HasDOMChildImpl 
{
 public:
+    HasDOMNodeImpl* hasDOMNodeImpl() { return this; }
+    const HasDOMNodeImpl* constHasDOMNodeImpl() const { return this; }
+    HasDOMParentImpl* hasDOMParentImpl() { return this; }
+    const HasDOMParentImpl* constHasDOMParentImpl() const { return this; }
+    HasDOMChildImpl* hasDOMChildImpl() { return this; }
+    const HasDOMChildImpl* constHasDOMChildImpl() const { return this; }
+
     DOMNodeImpl       fNode;
     DOMParentNode     fParent;
     DOMChildNode      fChild;
diff --git a/src/xercesc/dom/impl/DOMEntityImpl.hpp 
b/src/xercesc/dom/impl/DOMEntityImpl.hpp
index 0c50ecbc4..1ec251eaa 100644
--- a/src/xercesc/dom/impl/DOMEntityImpl.hpp
+++ b/src/xercesc/dom/impl/DOMEntityImpl.hpp
@@ -43,6 +43,12 @@ XERCES_CPP_NAMESPACE_BEGIN
 class    DOMEntityReference;
 
 class CDOM_EXPORT DOMEntityImpl: public DOMEntity, public HasDOMNodeImpl, 
public HasDOMParentImpl {
+public:
+    HasDOMNodeImpl* hasDOMNodeImpl() { return this; }
+    const HasDOMNodeImpl* constHasDOMNodeImpl() const { return this; }
+    HasDOMParentImpl* hasDOMParentImpl() { return this; }
+    const HasDOMParentImpl* constHasDOMParentImpl() const { return this; }
+
 protected:
     DOMNodeImpl      fNode;
     DOMParentNode    fParent;
diff --git a/src/xercesc/dom/impl/DOMEntityReferenceImpl.hpp 
b/src/xercesc/dom/impl/DOMEntityReferenceImpl.hpp
index 4afc436c6..35cff9a1b 100644
--- a/src/xercesc/dom/impl/DOMEntityReferenceImpl.hpp
+++ b/src/xercesc/dom/impl/DOMEntityReferenceImpl.hpp
@@ -44,6 +44,14 @@ XERCES_CPP_NAMESPACE_BEGIN
 class CDOM_EXPORT DOMEntityReferenceImpl: public DOMEntityReference,
         public HasDOMNodeImpl, public HasDOMParentImpl, public HasDOMChildImpl
 {
+public:
+    HasDOMNodeImpl* hasDOMNodeImpl() { return this; }
+    const HasDOMNodeImpl* constHasDOMNodeImpl() const { return this; }
+    HasDOMParentImpl* hasDOMParentImpl() { return this; }
+    const HasDOMParentImpl* constHasDOMParentImpl() const { return this; }
+    HasDOMChildImpl* hasDOMChildImpl() { return this; }
+    const HasDOMChildImpl* constHasDOMChildImpl() const { return this; }
+
 protected:
     DOMNodeImpl      fNode;
     DOMParentNode    fParent;
diff --git a/src/xercesc/dom/impl/DOMNotationImpl.hpp 
b/src/xercesc/dom/impl/DOMNotationImpl.hpp
index 23ff62177..535ff3f8c 100644
--- a/src/xercesc/dom/impl/DOMNotationImpl.hpp
+++ b/src/xercesc/dom/impl/DOMNotationImpl.hpp
@@ -44,6 +44,9 @@ class DOMDocument;
 
 class CDOM_EXPORT DOMNotationImpl: public DOMNotation, public HasDOMNodeImpl {
 public:
+    HasDOMNodeImpl* hasDOMNodeImpl() { return this; }
+    const HasDOMNodeImpl* constHasDOMNodeImpl() const { return this; }
+
     DOMNodeImpl      fNode;
 
     const XMLCh * fName;
diff --git a/src/xercesc/dom/impl/DOMParentNode.cpp 
b/src/xercesc/dom/impl/DOMParentNode.cpp
index 73920f9e2..d73d18ba2 100644
--- a/src/xercesc/dom/impl/DOMParentNode.cpp
+++ b/src/xercesc/dom/impl/DOMParentNode.cpp
@@ -67,7 +67,8 @@ const DOMNode* DOMParentNode::getContainingNode() const {
 }
 
 const DOMNodeImpl* DOMParentNode::getContainingNodeImpl() const {
-    const HasDOMNodeImpl* p = dynamic_cast<const 
HasDOMNodeImpl*>(getContainingNode());
+    //const HasDOMNodeImpl* p = dynamic_cast<const 
HasDOMNodeImpl*>(getContainingNode());
+    const HasDOMNodeImpl* p = getContainingNode()->constHasDOMNodeImpl();
     if (!p || !p->getNodeImpl()) {
         throw DOMException(DOMException::INVALID_STATE_ERR, 0, 
GetDOMNodeMemoryManager);
     }
diff --git a/src/xercesc/dom/impl/DOMProcessingInstructionImpl.hpp 
b/src/xercesc/dom/impl/DOMProcessingInstructionImpl.hpp
index c336b6746..8c080bea0 100644
--- a/src/xercesc/dom/impl/DOMProcessingInstructionImpl.hpp
+++ b/src/xercesc/dom/impl/DOMProcessingInstructionImpl.hpp
@@ -47,6 +47,12 @@ class    DocumentImpl;
 
 class CDOM_EXPORT DOMProcessingInstructionImpl: public 
DOMProcessingInstruction,
         public HasDOMNodeImpl, public HasDOMChildImpl {
+public:
+    HasDOMNodeImpl* hasDOMNodeImpl() { return this; }
+    const HasDOMNodeImpl* constHasDOMNodeImpl() const { return this; }
+    HasDOMChildImpl* hasDOMChildImpl() { return this; }
+    const HasDOMChildImpl* constHasDOMChildImpl() const { return this; }
+
 protected:
     DOMNodeImpl   fNode;
     DOMChildNode  fChild;
diff --git a/src/xercesc/dom/impl/DOMTextImpl.hpp 
b/src/xercesc/dom/impl/DOMTextImpl.hpp
index ecd3b980b..0061baaf2 100644
--- a/src/xercesc/dom/impl/DOMTextImpl.hpp
+++ b/src/xercesc/dom/impl/DOMTextImpl.hpp
@@ -45,6 +45,11 @@ XERCES_CPP_NAMESPACE_BEGIN
 
 class CDOM_EXPORT DOMTextImpl: public DOMText, public HasDOMNodeImpl, public 
HasDOMChildImpl {
 public:
+    HasDOMNodeImpl* hasDOMNodeImpl() { return this; }
+    const HasDOMNodeImpl* constHasDOMNodeImpl() const { return this; }
+    HasDOMChildImpl* hasDOMChildImpl() { return this; }
+    const HasDOMChildImpl* constHasDOMChildImpl() const { return this; }
+
     DOMNodeImpl             fNode;
     DOMChildNode            fChild;
     DOMCharacterDataImpl    fCharacterData;
--
2.19.1

{code}

> Bad casting from DOMTextImpl to DOMElementImpl
> ----------------------------------------------
>
>                 Key: XERCESC-2088
>                 URL: https://issues.apache.org/jira/browse/XERCESC-2088
>             Project: Xerces-C++
>          Issue Type: Bug
>          Components: DOM
>    Affects Versions: 3.1.1, 3.1.2, 3.1.3, 3.1.4
>         Environment: ubuntu 16.04 LTS, Intel(R) Core(TM) i7-6700 CPU @ 
> 3.40GHz, 16GB
>            Reporter: Yuseok Jeon
>            Assignee: Scott Cantor
>            Priority: Major
>             Fix For: 3.2.0
>
>         Attachments: Actual_result.txt, DOMNodeBase.hpp, casting.patch, 
> relationship_tree.jpeg
>
>
> Hi all, 
> Our recently developed type confusion detection tool reports a type_confusion 
> error in the "xercesc/dom/imple/DOMCasts.hpp" 
> xercesc/dom/imple/DOMCasts.hpp, line 146
> static inline DOMNodeImpl *castToNodeImpl(const DOMNode *p)
> {
>     DOMElementImpl *pE = (DOMElementImpl *)p;
>     return &(pE->fNode);
> }
> p is pointing to the object allocated as DOMTextImpl, and it is casted into 
> DOMElementImpl. However, since DOMElementImpl is not a subobject of 
> DOMTextImpl, it is violating C++ standard rules 5.2.9/11 (down casting is 
> undefined if the object that the pointer to be casted points to is not a 
> suboject of down casting type) and causes undefined behaviors.
> There are similar type-confusion cases as below links. 
>  - (libstdc++) https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60734
>  - (Firefox) https://bugzilla.mozilla.org/show_bug.cgi?id=1074280
> I attached a actual type confusion report and object relationship 
> information. 



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: c-dev-unsubscr...@xerces.apache.org
For additional commands, e-mail: c-dev-h...@xerces.apache.org

Reply via email to