kicad-developers team mailing list archive

[Andrew Zonenberg <azonenberg@xxxxxxxxxxxxxxx>]

The attached patch causes the model to load properly on my machine. It
adds a check to disable loading color information if no color nodes were
found.

On Fri, 2014-08-01 at 15:29 -0400, Andrew Zonenberg wrote:
> When loading this model:
> http://colossus.cs.rpi.edu/~azonenberg/downloads/molex-1051561008.x3d
> 
> I get a crash in the X3D model parser.
> 
> It appears to be due to a null dereference accessing color[0] since
> color has a size of zero and the internal array is null. I'm not sure if
> the model is well-formed or not (I converted the Molex model from I
> think STEP or IGES format via Freecad and Blender, then exported to X3D)
> but the loader shouldn't crash regardless of the input.
> 
> Program received signal SIGSEGV, Segmentation fault.
> 0x00007fffe6a423bb in X3D_MODEL_PARSER::readIndexedFaceSet
> (this=0x63ea870, aFaceNode=0x64a1970, aTransformProps=...)
> at /nfs/home/azonenberg/Documents/local/programming/3rdparty/kicad.bzr/3d-viewer/x3dmodelparser.cpp:527
> 527	    GetNodeProperties( color[0], color_properties );
> 
> (gdb) print color
> $1 = {<std::_Vector_base<wxXmlNode*, std::allocator<wxXmlNode*> >> =
> {_M_impl = {<std::allocator<wxXmlNode*>> =
> {<__gnu_cxx::new_allocator<wxXmlNode*>> = {<No data fields>}, <No data
> fields>}, _M_start = 0x0, _M_finish = 0x0, 
>       _M_end_of_storage = 0x0}}, <No data fields>}
> (gdb) print color_properties
> $2 = {_M_t = {_M_impl =
> {<std::allocator<std::_Rb_tree_node<std::pair<wxString const, wxString>
> > >> = {<__gnu_cxx::new_allocator<std::_Rb_tree_node<std::pair<wxString
> const, wxString> > >> = {<No data fields>}, <No data fields>}, 
>       _M_key_compare = {<std::binary_function<wxString, wxString, bool>>
> = {<No data fields>}, <No data fields>}, _M_header = {_M_color =
> std::_S_red, _M_parent = 0x0, _M_left = 0x7fffffffb3a8, _M_right =
> 0x7fffffffb3a8}, 
>       _M_node_count = 0}}}
> 
> (gdb) bt
> #0  0x00007fffe6a423bb in X3D_MODEL_PARSER::readIndexedFaceSet
> (this=0x63ea870, aFaceNode=0x64a1970, aTransformProps=...)
> at /nfs/home/azonenberg/Documents/local/programming/3rdparty/kicad.bzr/3d-viewer/x3dmodelparser.cpp:527
> #1  0x00007fffe6a404d5 in X3D_MODEL_PARSER::readTransform
> (this=0x63ea870, aTransformNode=0x63d9ec0)
> at /nfs/home/azonenberg/Documents/local/programming/3rdparty/kicad.bzr/3d-viewer/x3dmodelparser.cpp:222
> #2  0x00007fffe6a3fd26 in X3D_MODEL_PARSER::Load (this=0x63ea870,
> aFilename=...)
> at /nfs/home/azonenberg/Documents/local/programming/3rdparty/kicad.bzr/3d-viewer/x3dmodelparser.cpp:109
> #3  0x00007fffe6a34251 in S3D_MASTER::ReadData (this=0x521fc60)
> at /nfs/home/azonenberg/Documents/local/programming/3rdparty/kicad.bzr/3d-viewer/3d_read_mesh.cpp:122
> #4  0x00007fffe6a4f4e2 in MODULE::ReadAndInsert3DComponentShape
> (this=0x721f0c0, glcanvas=0x63fb660, aAllowNonTransparentObjects=true,
> aAllowTransparentObjects=false, aSideToLoad=false)
> 
> at /nfs/home/azonenberg/Documents/local/programming/3rdparty/kicad.bzr/3d-viewer/3d_draw.cpp:1703
> #5  0x00007fffe6a4e719 in EDA_3D_CANVAS::BuildFootprintShape3DList
> (this=0x63fb660, aOpaqueList=7, aTransparentList=8, aSideToLoad=false)
> at /nfs/home/azonenberg/Documents/local/programming/3rdparty/kicad.bzr/3d-viewer/3d_draw.cpp:1471
> #6  0x00007fffe6a4e5e3 in EDA_3D_CANVAS::CreateDrawGL_List
> (this=0x63fb660)
> at /nfs/home/azonenberg/Documents/local/programming/3rdparty/kicad.bzr/3d-viewer/3d_draw.cpp:1436
> #7  0x00007fffe6a4ad6c in EDA_3D_CANVAS::GenerateFakeShadowsTextures
> (this=0x63fb660)
> at /nfs/home/azonenberg/Documents/local/programming/3rdparty/kicad.bzr/3d-viewer/3d_draw.cpp:302
> #8  0x00007fffe6a4b1be in EDA_3D_CANVAS::Redraw (this=0x63fb660)
> at /nfs/home/azonenberg/Documents/local/programming/3rdparty/kicad.bzr/3d-viewer/3d_draw.cpp:371
> #9  0x00007fffe6a48dba in EDA_3D_CANVAS::OnPaint (this=0x63fb660,
> event=...)
> at /nfs/home/azonenberg/Documents/local/programming/3rdparty/kicad.bzr/3d-viewer/3d_canvas.cpp:490
> #10 0x00007ffff69273f6 in
> wxEvtHandler::ProcessEventIfMatches(wxEventTableEntryBase const&,
> wxEvtHandler*, wxEvent&) ()
> from /usr/lib/x86_64-linux-gnu/libwx_baseu-2.8.so.0
> 
> (remainder of backtrace trimmed, can supply if necessary)
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp

-- 
Andrew Zonenberg
PhD student, security group
Computer Science Department
Rensselaer Polytechnic Institute
http://colossus.cs.rpi.edu/~azonenberg/

=== modified file '3d-viewer/x3dmodelparser.cpp'
--- 3d-viewer/x3dmodelparser.cpp	2014-07-31 07:01:30 +0000
+++ 3d-viewer/x3dmodelparser.cpp	2014-08-01 19:34:32 +0000
@@ -521,47 +521,50 @@
     std::vector< double > color_points;
     NODE_LIST color;
     GetChildsByName( aFaceNode, wxT( "Color" ), color);
-
-    PROPERTY_MAP color_properties;
-    // IndexedFaceSet has one Coordinate child node
-    GetNodeProperties( color[0], color_properties );
-
-    // Save points to vector as doubles
-    wxStringTokenizer colorpoint_tokens( color_properties[ wxT("color") ] );
-    double color_point = 0.0;
-
-    while( colorpoint_tokens.HasMoreTokens() )
-    {
-        if( colorpoint_tokens.GetNextToken().ToDouble( &color_point ) )
-        {
-            color_points.push_back( color_point );
-        }
-        else
-        {
-            wxLogError( wxT( "Error converting to double" ) );
-        }
-    }
-
-    if( color_points.size() % 3 != 0 )
-    {
-        DBG( printf( "Number of points is incorrect" ) );
-        return;
-    }
-
-    /* Create 3D face color from 3 color points
-     */
-    for( unsigned id = 0; id < color_points.size() / 3; id++ )
-    {
-        m_model->m_MaterialIndex.push_back( id );
-
-        int color_triplet_indx = id * 3;
-        glm::vec3 colorface( color_points[ color_triplet_indx + 0 ],
-                             color_points[ color_triplet_indx + 1 ],
-                             color_points[ color_triplet_indx + 2 ] );
-
-        m_model->m_Materials->m_DiffuseColor.push_back( colorface );
-    }
-
+    
+    // Some models lack color information, need to handle this safely
+    if(!color.empty())
+    {
+        PROPERTY_MAP color_properties;
+        // IndexedFaceSet has one Coordinate child node
+        GetNodeProperties( color[0], color_properties );
+
+        // Save points to vector as doubles
+        wxStringTokenizer colorpoint_tokens( color_properties[ wxT("color") ] );
+        double color_point = 0.0;
+
+        while( colorpoint_tokens.HasMoreTokens() )
+        {
+            if( colorpoint_tokens.GetNextToken().ToDouble( &color_point ) )
+            {
+                color_points.push_back( color_point );
+            }
+            else
+            {
+                wxLogError( wxT( "Error converting to double" ) );
+            }
+        }
+
+        if( color_points.size() % 3 != 0 )
+        {
+            DBG( printf( "Number of points is incorrect" ) );
+            return;
+        }
+
+        /* Create 3D face color from 3 color points
+         */
+        for( unsigned id = 0; id < color_points.size() / 3; id++ )
+        {
+            m_model->m_MaterialIndex.push_back( id );
+
+            int color_triplet_indx = id * 3;
+            glm::vec3 colorface( color_points[ color_triplet_indx + 0 ],
+                                 color_points[ color_triplet_indx + 1 ],
+                                 color_points[ color_triplet_indx + 2 ] );
+
+            m_model->m_Materials->m_DiffuseColor.push_back( colorface );
+        }
+    }
 
     /* -- Read coordinate indexes -- */
     PROPERTY_MAP faceset_properties;

Attachment: signature.asc
Description: This is a digitally signed message part