0

I have a little project (~2k lines of code), which compiles with both clang and gcc. gcc gives the following error (-O0 optimization level):

/home/nikita/projects/curse_dim/bellman/include/bellman/bellman_operators/qfunc.hpp:10:7: runtime error: member call on address 0x7fff96470b80 which does not point to an object of type 'ICloneable'
0x7fff96470b80: note: object has invalid vptr
 fc fd fe ff  00 00 00 00 00 00 00 00  c0 16 00 00 20 61 00 00  00 18 00 00 20 61 00 00  00 18 00 00
              ^~~~~~~~~~~~~~~~~~~~~~~
              invalid vptr

It is triggered by a simple piece of code:

class DiscreteQFuncEst final : public EnableClone<DiscreteQFuncEst, InheritFrom<IQFuncEstimate>> {
public:
    DiscreteQFuncEst() = default;
    ...
protected:
    std::vector<FloatT> values_;
    size_t num_actions_;
    std::optional<ParticleCluster> particle_cluster_;
};

#include <bellman/bellman_operators/uniform_operator.hpp>

int main() {
    DiscreteQFuncEst est;
    static_assert(std::is_convertible_v<DiscreteQFuncEst*, ICloneable*>);
    bool should_be_true = dynamic_cast<ICloneable*>(&est) != 0; // <- UBSan is mad here
    bool has_failed = !should_be_true;
    return has_failed;
}

clang++ also gives "member call...", but on a different piece of code.

I wasn't able to reproduce it in a short example. EnableClone is almost standalone -- I've tried creating a minimal example using full cloneable.hpp, but wasn't able to reproduce UBSan error this way.

I suspected ODR to be the case, so I added -Wodr to compile flags, but it gave me nothing.

I've tried using gdb to examine vptrs, and I'm not completely sure, but they seem to be OK:

(gdb) p est
$1 = {<EnableClone<DiscreteQFuncEst, InheritFrom<IQFuncEstimate> >> = {<_CloneableImpl<DiscreteQFuncEst, InheritFrom<IQFuncEstimate>, false, true>> = {<InheritFrom<IQFuncEstimate>> = {<IQFuncEstimate> = {<EnableCloneInterface<IQFuncEstimate, void>> = {<_CloneableImpl<IQFuncEstimate, void, true, false>> = {<ICloneable> = {
                _vptr.ICloneable = 0x555555b20b18 <vtable for DiscreteQFuncEst+40>}, <No data fields>}, <No data fields>}, <No data fields>}, <No data fields>}, <No data fields>}, <No data fields>}, values_ = std::vector of length 0, capacity 0, num_actions_ = 0, 
  particle_cluster_ = std::optional<ParticleCluster> [no contained value]}
(gdb) p est.~ICloneable 
$2 = {void (ICloneable * const, int)} 0x555555914644 <ICloneable::~ICloneable()>
(gdb) p ICloneable::~ICloneable
$3 = {void (ICloneable * const)} 0x555555914644 <ICloneable::~ICloneable()>

EnableClone implementation: https://github.com/npetrenko/curse_dim/blob/4c80eccf637bf351ef708c04c432a5c18e41dcfd/bellman/include/bellman/cloneable.hpp

If I add a destructor with side effects to ICloneable

virtual ~ICloneable() {
    std::cerr << "Destructor works!\n";
}

then DiscreteQFuncEst prints the expected result on destruction in the example where gcc's UBSan complains (-O0, and ICloneable is a virtual base, so I hope that virtual ptr traversal hasn't been optimized out).

I've found several bug reports reporting false positive results, e.g.: https://bugs.llvm.org/show_bug.cgi?id=39191

Unlike the reports, though, I don't mess with visibility, and all libraries that I write are static-linked.

Is this a false-positive? If not, how can I possibly debug this? Or can I somehow ensure myself that it is indeed UB?

Thanks!

Steps to reproduce:

git clone --recursive https://github.com/npetrenko/curse_dim/ && cd curse_dim && git checkout ub_reproduce

and look into README.md

Update: I managed to strip it to almost zero. I don't know why I didn't succeed before. It might be more convenient to checkout the repo, I'm just copying the code from there.

Class that creates trouble:

#pragma once

#include <type_traits>

class ICloneable {
public:
    virtual ~ICloneable() = default;
};

template <class T>
struct InheritFrom : public T {
    using T::T;
};

template <class Derived, class AnotherBase, bool derived_is_abstract,
          bool base_is_cloneable = std::is_base_of_v<ICloneable, AnotherBase>>
class _CloneableImpl;

// three identical implementations, only the inheritance is different

#define Implement(IsAbstract)                                                                     \
    /* "no base is defined" case*/                                                                \
    template <class Derived>                                                                      \
    class _CloneableImpl<Derived, void, IsAbstract, false> : public virtual ICloneable {          \
    };                                                                                            \
                                                                                                  \
    /* Base is defined, and already provides ICloneable*/                                         \
    template <class Derived, class AnotherBase>                                                   \
    class _CloneableImpl<Derived, AnotherBase, IsAbstract, true> : public AnotherBase {           \
    };                                                                                            \
                                                                                                  \
    /* Base is defined, but has no ICloneable*/                                                   \
    template <class Derived, class AnotherBase>                                                   \
    class _CloneableImpl<Derived, AnotherBase, IsAbstract, false> : public AnotherBase,           \
                                                                    public virtual ICloneable {   \
    };

Implement(false) 
Implement(true)

#undef Implement

template <class Derived, class AnotherBase = void>
class EnableClone : public _CloneableImpl<Derived, AnotherBase, false> {
};

template <class Derived, class AnotherBase = void>
class EnableCloneInterface : public _CloneableImpl<Derived, AnotherBase, true> {
};

Example of UBSan error trigger code:

#include "cloneable.hpp"

class IQFuncEstimate : public EnableCloneInterface<IQFuncEstimate> {
public:
    virtual ~IQFuncEstimate() = default;
};

class DiscreteQFuncEst final : public EnableClone<DiscreteQFuncEst, InheritFrom<IQFuncEstimate>> {
};

int main() {
    DiscreteQFuncEst est;
    static_assert(std::is_convertible_v<DiscreteQFuncEst*, ICloneable*>);
    bool should_be_true = dynamic_cast<ICloneable*>(&est) != 0;
    bool has_failed = !should_be_true;
    return has_failed;
}

Compile with:

g++ -I../include -Wall -Wextra -Wpedantic -std=c++17 -fsanitize=address,undefined -fno-sanitize-recover=all ../ub_example/ub_example.cpp

(replace g++ with clang++ to make UBSan happy)

g++ (GCC) 9.1.0

clang version 8.0.1

Nikita Petrenko
  • 838
  • 1
  • 6
  • 10
  • 3
    "clang++ also gives "member call..."" Please don't truncate error messages like that. You are denying us all the potentially useful parts of the message. – Jesper Juhl Aug 02 '19 at 16:34
  • Can you please post how to replicate the behavior you are getting? I have https://github.com/npetrenko/curse_dim , how to compile, to run, what arguments to give, what input? Ideally would be if you would include `trigger_ub.sh` script that does the work for others. What compiler, compiler options and versions are you using? I can guess that most probably you have UB in unrelated part of the code. Your project is quite big. Please limit it, create a minimal reproducible example. 2K lines of code in a single file - I'll say you could even post it here in your question. – KamilCuk Aug 02 '19 at 16:40
  • `std::is_base_of_v` can work in cases where the conversion is not actually able to be preformed (private inheritance for instance). What happens if you check `std::is_convertible_v`? – NathanOliver Aug 02 '19 at 16:42
  • @KamilCuk I've added steps to reproduce to the question. OK, I'll additionally create a separate branch and describe everything needed there – Nikita Petrenko Aug 02 '19 at 16:50
  • External steps do not make a valid SO question, sorry. You're going to have to keep trying to narrow it down. – Lightness Races in Orbit Aug 02 '19 at 16:50
  • https://github.com/npetrenko/curse_dim/tree/ub_reproduce – Nikita Petrenko Aug 02 '19 at 17:03
  • @NathanOliver it is indeed convertible – Nikita Petrenko Aug 02 '19 at 17:08
  • @KamilCuk "I can guess that most probably you have UB in unrelated part of the code." I don't think this is the case here. I've updated my question. https://github.com/npetrenko/curse_dim/blob/ub_reproduce/bellman/ub_example/ub_example.cpp#L6 – Nikita Petrenko Aug 02 '19 at 19:25
  • Still in ` DiscreteQFuncEst est;` is a lot going on in the constructor. You have to make a minimal reproducible example. Remove parts of your code until it stops crashing. Then, the smallest example, post it here. : / – KamilCuk Aug 02 '19 at 19:50
  • 1
    @KamilCuk managed to strip almost everything. Now I believe it is a false-positive. Sadly, clang also fails, but for a different example. So I believe clang also gives false-positive – Nikita Petrenko Aug 02 '19 at 22:02

0 Answers0