Thread: MSVSC++ program works incorrectly when inline function optimization is turned on.

The program works fine in debug mode. It doesn't in release mode with optimizations enabled, more specifically "Inline Function Expansion" enabled. Disabling inline expansion (/0b0) which is on by default and keeping the rest of optimizations at their default value, makes the program behave correctly again.

In my experience, these sort of problems arise when I resort to undefined behavior. So I'm posting a snippet of code that showcases the specific section of the code that is misbehaving plus the required extra information to give clarity to the pertinent section, in hopes someone can spot a glaring mistake of which I am unaware.

Header file with t_item_data definition:

Code:

namespace CraftingUtils
{
#define UINTX_T UInt64

struct                        t_item_data
{
    union                    details
    {
        EnchantmentItem*    enchantment;
        float                temperval;

        explicit            details(float);
        explicit            details (EnchantmentItem*);
        //~details(void);
    };
    typedef                    std::vector<std::pair<details, unsigned> > t_stats;
    t_stats                    stats;
    unsigned                count;
    unsigned                created;
    bool                    processed;

    explicit                t_item_data(void);
    explicit                t_item_data(t_stats*);
    t_item_data(const t_item_data&) = delete;
    t_item_data &operator = (const t_item_data&) = delete;
    //~t_item_data(void);
};
...
...
...

Source file, problematic function plus helper functions:

Code:

static std::pair<t_item_data::details, unsigned> *find_stat(
    t_item_data::t_stats *list, const t_item_data::details *stat)
{
    t_item_data::t_stats::iterator element = (*list).begin();

    for (unsigned i = (*list).size(); i; --i)
    {   if (*(UINTX_T*)&(*element).first == *(UINTX_T*)stat)
            return (&*element);
        ++element;
    }
    return (nullptr);
}

static bool get_item_enchantments(InventoryEntryData* invdata, t_item_data::t_stats *stats)
{
    ExtendDataList::Iterator    list = (*(*invdata).extendDataList).Begin();
    BaseExtraList                *extra_data;
    bool                        retval = false;

    while ((extra_data = list.Get() ) )
    {   ExtraEnchantment* extra_ench =
            DYNAMIC_CAST((*extra_data).GetByType(kExtraData_Enchantment), BSExtraData, ExtraEnchantment);

        if (extra_ench)
        {   std::pair<t_item_data::details, unsigned>    *match;
            t_item_data::details                        enchantment((*extra_ench).enchant);

            if (!(match = find_stat(stats, &enchantment) ) )
                (*stats).emplace_back(enchantment, 1);
            else ++(*match).second;
            retval = true;
        }
        ++list;
    }
    return (retval);
}

static bool get_item_temperstats(InventoryEntryData *invdata, t_item_data::t_stats *stats)
{
    ExtendDataList::Iterator    list = (*(*invdata).extendDataList).Begin();
    BaseExtraList                *extra_data;
    bool                        retval = false;

    while ((extra_data = list.Get() ) )
    {   ExtraHealth *extra_health =
            DYNAMIC_CAST((*extra_data).GetByType(kExtraData_Health), BSExtraData, ExtraHealth);

        if (extra_health)
        {   std::pair<t_item_data::details, unsigned>    *match;
            t_item_data::details                        temperval((*extra_health).health);

            if (!(match = find_stat(stats, &temperval) ) )
                (*stats).emplace_back(temperval, 1);
            else ++(*match).second;
            retval = true;
        }
        ++list;
    }
    return (retval);
}

//__declspec(noinline)
static void detect_item_changes(void)
{
    std::vector<std::pair<t_item_data::details, unsigned> >    statlist;
    std::vector<std::pair<t_item_data::details, unsigned> >    created;
    std::map<TESForm*, t_item_data>::iterator                items = records.begin();
    InventoryEntryData                                        *invdata;
    bool (*get_stats)(InventoryEntryData*, t_item_data::t_stats*) = nullptr;

    switch (service.type)
    {
    case t_service::temper:
        get_stats = &get_item_temperstats;
        break ;
    case t_service::enchant:
        get_stats = &get_item_enchantments;
        break ;
    }
    for (std::map<TESForm*, t_item_data>::const_iterator end = records.cend(); items != end; ++items)
    {   invdata = get_pcinvdata_for_item((*items).first);
        if (!get_stats)
        {   if (invdata && (signed)(*items).second.count < (*invdata).countDelta)
                (*items).second.created = (unsigned)(*invdata).countDelta - (*items).second.count;
            continue;
        }
        if ((*items).second.processed)
            continue;
        if (get_stats(invdata, &statlist) )
        {   const std::pair<t_item_data::details, unsigned>    *cachedstat;
            t_item_data::t_stats::iterator                    newstat = statlist.begin();

            for (unsigned i = statlist.size(); i; --i)
            {   cachedstat = find_stat(&(*items).second.stats, &(*newstat).first); //THis call always fails, i.e find_stat returns a nullptr meaning it didn't find the requested stat in the stats list. But it is there!
                const unsigned new_count = (cachedstat && (*newstat).second > (*cachedstat).second) ?
                                           (*cachedstat).second : 0;

                if (new_count || !cachedstat)
                {   (*items).second.created += (*newstat).second - new_count;
                    created.emplace_back((*newstat).first, 0);
                }
                ++newstat;
            }
            statlist.clear();
        }
        if (created.size() )
        {   (*items).second.stats = created;
            (*items).second.processed = true;
            created.clear();
        }
    }
}

You are probably stepping on memory you shouldn't somewhere.
The different memory layouts of debug and release mode with or without inlining is what is causing the different behavior.
Have you tried running it through a memory analyzer?
DrMemory
Valgrind Home

The best thing would to not use a union in C++ code at all.
They are messy things that are impossible to construct and destruct properly in a C++ world.

> changing UINTX_T to be UInt32 instead of UInt64 solved the issue
It's not solved.
You've buried the problem for now, but it's a zombie waiting to resurrect at some point in the future.


If you're using an older version of C++, then consider Chapter 4. Boost.Any - 1.76.0
If you're up to at least C++17, then std::variant - cppreference.com