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

  1. #1
    Registered User
    Join Date
    Sep 2020
    Posts
    15

    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();
            }
        }
    }
    Last edited by Exosomes; 07-03-2021 at 05:09 PM. Reason: Code format

  2. #2
    Registered User
    Join Date
    Dec 2017
    Posts
    1,136
    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 argument against democracy is a five minute conversation with the average voter. - Churchill

  3. #3
    Registered User
    Join Date
    Sep 2020
    Posts
    15
    Quote Originally Posted by john.c View Post
    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
    Valgrind is one of my favorite tools ever. I use it all the time for my C projects in Linux. I wasn't aware that it worked with C++ much less in Windows. I will certainly try it, thanks!

    After much pain and judiciously placed debug statements(If I placed too many and the thing worked fine!) I found the root of the problem:
    Code:
        ...
        union                    details
        {
             EnchantmentItem*    enchantment;
             float                           temperval;
         }
    
        #define UINTX_T UInt64
        ...
        ...
        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);
        }
    changing UINTX_T to be UInt32 instead of UInt64 solved the issue, which made me realize that the extra bits in the union, after the 32 bit float contained garbage, making the comparison fail.

    I created a defaultconstructor to zero out the union and the problem was solved.
    Code:
    t_item_data::details::details(void)
    {    *(UINTX_T*)this = 0;
    }
    t_item_data::details::details(float f) : t_item_data::details()
    {    temperval = f;
    }
    So you were right, I was indeed accessing memory I shouldn't, in this case uninitialized memory.

    Thanks.

  4. #4
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    38,714
    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
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Replies: 2
    Last Post: 05-10-2015, 03:40 PM
  2. Program running slower after optimization?
    By kkk in forum C Programming
    Replies: 2
    Last Post: 01-21-2012, 12:13 AM
  3. Program optimization
    By a.mlw.walker in forum C Programming
    Replies: 1
    Last Post: 03-27-2009, 06:12 AM
  4. Inline function
    By Bargi in forum C++ Programming
    Replies: 9
    Last Post: 07-18-2008, 08:02 AM
  5. Virtual function optimization
    By C+/- in forum C++ Programming
    Replies: 1
    Last Post: 11-15-2007, 10:04 AM

Tags for this Thread