Possible MSVC compiler bug

Question

Given shared_ptr variable declared in condition clause of for loop and for loop body contains if/continue statement, microsoft compiler (as of version 2015) generates extra destructor call (two total) per loop iteration. This causes destruction of Item objects that should be out of reach of Holder interface users. See example code below

namespace
{
    class Item
    {
    public:
        Item(size_t v)
            : value_(v)
        {
            std::cout << "Item(" << value_ << ")" << std::endl;
        }

        ~Item()
        {
            std::cout << "~Item(" << value_ << ")" << std::endl;
        }

        void print() const
        {
            std::cout << "Item::print(" << value_ << ")" << std::endl;
        }

    private:
        size_t value_;

    };

    typedef std::shared_ptr<const Item> ItemCPtr;

    class Holder
    {
    public:
        Holder(size_t n)
        {
            for (size_t i = 0; i < n; ++i)
                items_.emplace_back(new Item(i));
        }

        ItemCPtr getItem(size_t i) const
        {
            if (i < items_.size())
                return items_[i];
            return ItemCPtr();
        }

    private:
        std::vector<ItemCPtr> items_;
    };
}

TEST(Test, Test)
{
    Holder _holder(5);

    std::cout << "before loop" << std::endl;

    for (size_t i = 0; auto _item = _holder.getItem(i); ++i)
    {
        if (!!(i % 2))
            continue;

        _item->print();
    }

    std::cout << "after loop" << std::endl;

    _holder.getItem(1)->print();
    _holder.getItem(3)->print();
}

This yield output below

||   [ RUN      ] Test.Test
||   Item(0)
||   Item(1)
||   Item(2)
||   Item(3)
||   Item(4)
||   before loop
||   Item::print(0)
||   ~Item(1)
||   Item::print(2)
||   ~Item(3)
||   Item::print(4)
||   after loop
||   Item::print(3722304989)
||   Item::print(3722304989)
||   ~Item(0)
||   ~Item(2)
||   ~Item(4)
||   [       OK ] Test.Test (0 ms)

If I move _item declaration out of for loop this way

    auto _item = ItemCPtr();
    for (size_t i = 0; _item = _holder.getItem(i); ++i)

then I get expected output like this

||   [ RUN      ] Test.Test
||   Item(0)
||   Item(1)
||   Item(2)
||   Item(3)
||   Item(4)
||   before loop
||   Item::print(0)
||   Item::print(2)
||   Item::print(4)
||   after loop
||   Item::print(1)
||   Item::print(3)
||   ~Item(0)
||   ~Item(1)
||   ~Item(2)
||   ~Item(3)
||   ~Item(4)
||   [       OK ] Test.Test (0 ms)

As far as I can understand, getItem should yield a copy of ItemCPtr and no Items could be modified through Holder interface. Yet user can destroy two out of five items inside loop, see ~Item(1) and ~Item(3) destructor output between before loop/after loop marks.

This is trivial example to expose problem. In real world this would cause hard to track memory corruption problem.

Compiler identity:

cmake -G "Visual Studio 14 2015" ..\
-- Selecting Windows SDK version 10.0.14393.0 to target Windows 10.0.19041.
-- The C compiler identification is MSVC 19.0.24210.0
-- The CXX compiler identification is MSVC 19.0.24210.0

OS is 64bit Windows 10

Bug manifests itself even when optimization is disabled /Od, default options.

Answer 1

As far as I can understand, getItem should yield a copy of ItemCPtr and no Items could be modified through Holder interface.

Your understanding is 100% accurate. That is exactly the behavior described for the C++ standard's abstract machine.

Even under the as-if rule, which allows compilers to optimize, the observable behavior (on account of printing to the standard output stream) should be as though there was at least one shared pointer (for each item) that exists until the end of the test's scope. That is clearly not what you see though.

My educated guess would be that MSVC optimizes the copies out entirely. And instead refers to pointers inside the vector directly in the loop body. That on its own is okay.

The bug would likely be that it mishandles the code which would have destroyed the local variable in case of a continue statment. And errounosly applies it to the object in the vector. This is a bug.