harsh
harsh

Reputation: 969

Segfault with inheritence and casting

I am using an existing C++ which looks something like this

class Geometry {
public:
    enum {BOX, MESH} type;
    std::string name;
};

class Mesh : public Geometry {
public:
    std::string filename;
};

std::shared_ptr<Geometry> getGeometry() {
    std::shared_ptr<Geometry> geom = std::make_shared<Geometry>();
    geom->name = "geometry";

    Mesh *mesh = new Mesh();
    mesh->name = "name";
    mesh->filename = "filename";
    mesh->type = Geometry::MESH;
    geom.reset(mesh);

    return geom;
}

I need to extend the functionalities provided by these classes, so I have derived these two classes

#include <memory>
#include <iostream>

class Geometry {
public:
    enum {BOX, MESH} type;
    std::string name;
};

class Mesh : public Geometry {
public:
    std::string filename;
};

std::shared_ptr<Geometry> getGeometry() {
    std::shared_ptr<Geometry> geom = std::make_shared<Geometry>();
    geom->name = "geometry";

    Mesh *mesh = new Mesh();
    mesh->name = "name";
    mesh->filename = "filename";
    mesh->type = Geometry::MESH;
    geom.reset(mesh);

    return geom;
}

class GeometryDerived : public Geometry {
public:
    void consumeGeometry() {
        Geometry geom = static_cast<Geometry>(*this);
        std::cout << geom.name << std::endl;
        if(geom.type == Geometry::MESH) {
            Mesh *mesh = static_cast<Mesh*>(&geom);
            std::cout << mesh->filename << std::endl;
            // mesh->get_output();
        }
    }
};

class MeshDerived : public Mesh {
public:
    std::string get_output() {
        return this->filename;        
    }
};



int main(int argc, char** argv)
{
    std::shared_ptr<Geometry> geom = getGeometry(); // this object is returned by the library

    std::shared_ptr<GeometryDerived> geomDerived = std::static_pointer_cast<GeometryDerived>(geom);
    geomDerived->consumeGeometry();

    return 0;
}

I get a segfault in line (in my actual code, its garbage string)

std::cout << mesh->filename << std::endl;

though other ints and doubles are printed correctly. Even geom.name gets printed correctly. What is going on with strings here? What is the correct way to cast the objects here? Casting the object twice looks ugly. Below is the GDB output

GNU gdb (Ubuntu 9.2-0ubuntu1~20.04) 9.2
Copyright (C) 2020 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from ./cpp-example...
(gdb) run
Starting program: /home/user/cpp-example
name

Program received signal SIGSEGV, Segmentation fault.
__memmove_avx_unaligned_erms () at ../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:383
383 ../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S: No such file or directory.
(gdb) bt
#0  __memmove_avx_unaligned_erms () at ../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:383
#1  0x00007ffff7c247b2 in _IO_new_file_xsputn (n=140737488346512, data=0xdb8bbc853a3b3400, f=<optimized out>) at fileops.c:1236
#2  _IO_new_file_xsputn (f=0x7ffff7d7e6a0 <_IO_2_1_stdout_>, data=0xdb8bbc853a3b3400, n=140737488346512) at fileops.c:1197
#3  0x00007ffff7c18541 in __GI__IO_fwrite (buf=0xdb8bbc853a3b3400, size=1, count=140737488346512, fp=0x7ffff7d7e6a0 <_IO_2_1_stdout_>) at libioP.h:948
#4  0x00007ffff7ed2824 in std::basic_ostream<char, std::char_traits<char> >& std::__ostream_insert<char, std::char_traits<char> >(std::basic_ostream<char, std::char_traits<char> >&, char const*, long) ()
   from /lib/x86_64-linux-gnu/libstdc++.so.6
#5  0x0000555555556858 in GeometryDerived::consumeGeometry (this=0x55555556def0) at ../downcasting.cpp:23
#6  0x0000555555556513 in main (argc=1, argv=0x7fffffffdec8) at ../downcasting.cpp:53

Edit 1: The following changes works fine, but this is not what I want

int main(int argc, char** argv)
{
    std::shared_ptr<Geometry> geom = getGeometry();

    if(geom->type == Geometry::MESH) {
        std::shared_ptr<MeshDerived> meshDerived = std::static_pointer_cast<MeshDerived>(geom);
        std::cout << meshDerived->filename << std::endl;
    }

    // std::shared_ptr<GeometryDerived> geomDerived = std::static_pointer_cast<GeometryDerived>(geom);
    // geomDerived->consumeGeometry();

    return 0;
}

Upvotes: 1

Views: 157

Answers (1)

Andrey Semashev
Andrey Semashev

Reputation: 10624

There are multiple issues with your code:

  • In GeometryDerived::consumeGeometry, Geometry geom = static_cast<Geometry>(*this); creates a new object of type Geometry by slicing the current object. Later in that function, you do Mesh *mesh = static_cast<Mesh*>(&geom);, which returns an invalid pointer (as geom is not a Mesh). Accessing the data through that pointer is UB. (It will likely work for data members that are declared in Geometry on most implementations simply due to memory layout that is used by most compilers; that doesn't make the behavior well defined.)

  • static_pointer_cast does not check that the pointed object is actually of the requested type. The cast will succeed even if the object is of a different type, but the returned pointer will be invalid. The getGeometry function returns a pointer to a Mesh object, which is unrelated to GeometryDerived, so the cast returns an invalid pointer.


A comment to the Edit 1 part. No, this line is also wrong:

std::shared_ptr<MeshDerived> meshDerived = std::static_pointer_cast<MeshDerived>(geom);

The reason is the same as I noted above: getGeometry returns a pointer to Mesh, not MeshDerived. The code will probably work on most compilers because MeshDerived has the same memory layout as Mesh, but this is not a valid C++ nonetheless.

It looks like you want to "add methods" to an existing class in runtime. This is not possible in C++. Either you have to define the methods you need in the primary class (e.g. Mesh) or use a visitor pattern, i.e. define the new functionality in a separate class that will operate on Mesh etc. through its public members.

Upvotes: 4

Related Questions