Skip to content
  • Categories
  • Recent
  • Tags
  • Popular
  • World
  • Users
  • Groups
Skins
  • Light
  • Cerulean
  • Cosmo
  • Flatly
  • Journal
  • Litera
  • Lumen
  • Lux
  • Materia
  • Minty
  • Morph
  • Pulse
  • Sandstone
  • Simplex
  • Sketchy
  • Spacelab
  • United
  • Yeti
  • Zephyr
  • Dark
  • Cyborg
  • Darkly
  • Quartz
  • Slate
  • Solar
  • Superhero
  • Vapor

  • Default (No Skin)
  • No Skin
Collapse
Code Project
  1. Home
  2. Other Discussions
  3. Clever Code
  4. Optimizer bug [modified]

Optimizer bug [modified]

Scheduled Pinned Locked Moved Clever Code
helpquestioncsharpcomlearning
16 Posts 6 Posters 4 Views 1 Watching
  • Oldest to Newest
  • Newest to Oldest
  • Most Votes
Reply
  • Reply as topic
Log in to reply
This topic has been deleted. Only users with topic management privileges can see it.
  • P peterchen

    of course it is NOT an optimizer bug, but it was subtle enough for me having to look at the assembler code. Topic: Building a flat buffer for some simple data exchange. A few different buffer structures exist. What will be sent is a buffer that starts with a structure of POD's (e.g. CMsgMonitorInfo). variable-sized data is appended to the buffer, and the relevant offset is member of the struct. struct CMsgMonitorInfo { UINT monitorType; UINT offsetName; // offset of the monitor name in the buffer } So, introducing a helper class to build these buffers:

    template <typename T> // T must be a structure of POD's
    class Helper<T>
    {
    size_t m_bufferSize;
    void * m_buffer; // e.g. allocated with malloc. char>
    public:

    // CTor allocates initial buffer
    Helper()
    {
    m_buffer = malloc(sizeof(T));
    m_bufferSize = sizeof(T);
    }

    // member accessor allows direct access to T
    T* operator ->() { return (T *) m_buffer; }

    // grow buffer, append string, return offset of the copied string in the buffer 
    

    UINT Append(LPCTSTR string) { ... }
    }

    Now for the bug:

    Helper<CMsgMonitorInfo> helper;
    helper->monitorType = EMT_GoodOne; // whatever
    helper->offsetName = helper.Append("MyName"); // BUG here

    This is the intended way of using helper. Can you spot the error? See Hints for details (and what is NOT the error)

    We are a big screwed up dysfunctional psychotic happy family - some more screwed up, others more happy, but everybody's psychotic joint venture definition of CP
    My first real C# project | Linkify!|[">FoldWithUs!](http://tinyurl.com/37q6tt<br mode=) | sighist

    modified on Friday, December 14, 2007 8:02:38 AM

    C Offline
    C Offline
    CPallini
    wrote on last edited by
    #7

    Following your hints I can guess that in

    peterchen wrote:

    helper->offsetName = helper.Append("MyName"); // BUG here

    the operator-> is evaluated before helper.Append("MyName") (and reallocation) happens, hence the right offset is assigned to the wrong place. I don't know if it the right answer (though I'm quite confident about), but, mon ami, without hints... I did never guess it. :-D

    If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler. -- Alfonso the Wise, 13th Century King of Castile.

    P 1 Reply Last reply
    0
    • P peterchen

      of course it is NOT an optimizer bug, but it was subtle enough for me having to look at the assembler code. Topic: Building a flat buffer for some simple data exchange. A few different buffer structures exist. What will be sent is a buffer that starts with a structure of POD's (e.g. CMsgMonitorInfo). variable-sized data is appended to the buffer, and the relevant offset is member of the struct. struct CMsgMonitorInfo { UINT monitorType; UINT offsetName; // offset of the monitor name in the buffer } So, introducing a helper class to build these buffers:

      template <typename T> // T must be a structure of POD's
      class Helper<T>
      {
      size_t m_bufferSize;
      void * m_buffer; // e.g. allocated with malloc. char>
      public:

      // CTor allocates initial buffer
      Helper()
      {
      m_buffer = malloc(sizeof(T));
      m_bufferSize = sizeof(T);
      }

      // member accessor allows direct access to T
      T* operator ->() { return (T *) m_buffer; }

      // grow buffer, append string, return offset of the copied string in the buffer 
      

      UINT Append(LPCTSTR string) { ... }
      }

      Now for the bug:

      Helper<CMsgMonitorInfo> helper;
      helper->monitorType = EMT_GoodOne; // whatever
      helper->offsetName = helper.Append("MyName"); // BUG here

      This is the intended way of using helper. Can you spot the error? See Hints for details (and what is NOT the error)

      We are a big screwed up dysfunctional psychotic happy family - some more screwed up, others more happy, but everybody's psychotic joint venture definition of CP
      My first real C# project | Linkify!|[">FoldWithUs!](http://tinyurl.com/37q6tt<br mode=) | sighist

      modified on Friday, December 14, 2007 8:02:38 AM

      P Offline
      P Offline
      Phil J Pearson
      wrote on last edited by
      #8

      Great! I posted this before CPallini's post appeared, but the forum is so slow it took aaaaaaaaagggggggggggeeeeeeeeeeeeeessssssssss for my post to go through!


      I imagine that if you do

      helper->offsetName = helper.Append("MyName");

      the compiler emits code that evaluates the pointer helper-> before helper.Append(...) is called. Then, if the address of the buffer changes helper-> is out of date and the write goes to the wrong place.

      Phil


      The opinions expressed in this post are not necessarily those of the author, especially if you find them impolite, inaccurate or inflammatory.

      modified on Friday, December 14, 2007 1:54:12 PM

      C 1 Reply Last reply
      0
      • P peterchen

        of course it is NOT an optimizer bug, but it was subtle enough for me having to look at the assembler code. Topic: Building a flat buffer for some simple data exchange. A few different buffer structures exist. What will be sent is a buffer that starts with a structure of POD's (e.g. CMsgMonitorInfo). variable-sized data is appended to the buffer, and the relevant offset is member of the struct. struct CMsgMonitorInfo { UINT monitorType; UINT offsetName; // offset of the monitor name in the buffer } So, introducing a helper class to build these buffers:

        template <typename T> // T must be a structure of POD's
        class Helper<T>
        {
        size_t m_bufferSize;
        void * m_buffer; // e.g. allocated with malloc. char>
        public:

        // CTor allocates initial buffer
        Helper()
        {
        m_buffer = malloc(sizeof(T));
        m_bufferSize = sizeof(T);
        }

        // member accessor allows direct access to T
        T* operator ->() { return (T *) m_buffer; }

        // grow buffer, append string, return offset of the copied string in the buffer 
        

        UINT Append(LPCTSTR string) { ... }
        }

        Now for the bug:

        Helper<CMsgMonitorInfo> helper;
        helper->monitorType = EMT_GoodOne; // whatever
        helper->offsetName = helper.Append("MyName"); // BUG here

        This is the intended way of using helper. Can you spot the error? See Hints for details (and what is NOT the error)

        We are a big screwed up dysfunctional psychotic happy family - some more screwed up, others more happy, but everybody's psychotic joint venture definition of CP
        My first real C# project | Linkify!|[">FoldWithUs!](http://tinyurl.com/37q6tt<br mode=) | sighist

        modified on Friday, December 14, 2007 8:02:38 AM

        P Offline
        P Offline
        Phil J Pearson
        wrote on last edited by
        #9

        Incidentally, I think this is a great subtle bug - exactly the kind of thing we should be seeing in this forum!

        Phil


        The opinions expressed in this post are not necessarily those of the author, especially if you find them impolite, inaccurate or inflammatory.

        1 Reply Last reply
        0
        • P peterchen

          of course it is NOT an optimizer bug, but it was subtle enough for me having to look at the assembler code. Topic: Building a flat buffer for some simple data exchange. A few different buffer structures exist. What will be sent is a buffer that starts with a structure of POD's (e.g. CMsgMonitorInfo). variable-sized data is appended to the buffer, and the relevant offset is member of the struct. struct CMsgMonitorInfo { UINT monitorType; UINT offsetName; // offset of the monitor name in the buffer } So, introducing a helper class to build these buffers:

          template <typename T> // T must be a structure of POD's
          class Helper<T>
          {
          size_t m_bufferSize;
          void * m_buffer; // e.g. allocated with malloc. char>
          public:

          // CTor allocates initial buffer
          Helper()
          {
          m_buffer = malloc(sizeof(T));
          m_bufferSize = sizeof(T);
          }

          // member accessor allows direct access to T
          T* operator ->() { return (T *) m_buffer; }

          // grow buffer, append string, return offset of the copied string in the buffer 
          

          UINT Append(LPCTSTR string) { ... }
          }

          Now for the bug:

          Helper<CMsgMonitorInfo> helper;
          helper->monitorType = EMT_GoodOne; // whatever
          helper->offsetName = helper.Append("MyName"); // BUG here

          This is the intended way of using helper. Can you spot the error? See Hints for details (and what is NOT the error)

          We are a big screwed up dysfunctional psychotic happy family - some more screwed up, others more happy, but everybody's psychotic joint venture definition of CP
          My first real C# project | Linkify!|[">FoldWithUs!](http://tinyurl.com/37q6tt<br mode=) | sighist

          modified on Friday, December 14, 2007 8:02:38 AM

          A Offline
          A Offline
          admiralh2
          wrote on last edited by
          #10

          Well, without diving into the reference manual, it looks like you have two evaluations: A left-side evaluation (helper->offsetName) which returns a reference to the "offsetName" dereference of address of m_buffer A right-side evaluation (helper.Append("MyName")) which may change the value of m_buffer. I believe the choice of which to evaluate first is arbitrary (compiler-dependent). That means that the "helper->" operation will return m_buffer either before or after the evaluation of the right-hand side. Since the evaluation of the rhs can change m_buffer, if the lhs is evaluated first, the code will fail.

          Hopelessly pedantic since 1963.

          P L 2 Replies Last reply
          0
          • P Phil J Pearson

            Great! I posted this before CPallini's post appeared, but the forum is so slow it took aaaaaaaaagggggggggggeeeeeeeeeeeeeessssssssss for my post to go through!


            I imagine that if you do

            helper->offsetName = helper.Append("MyName");

            the compiler emits code that evaluates the pointer helper-> before helper.Append(...) is called. Then, if the address of the buffer changes helper-> is out of date and the write goes to the wrong place.

            Phil


            The opinions expressed in this post are not necessarily those of the author, especially if you find them impolite, inaccurate or inflammatory.

            modified on Friday, December 14, 2007 1:54:12 PM

            C Offline
            C Offline
            CPallini
            wrote on last edited by
            #11

            Phil J Pearson wrote:

            Great! I posted this before CPallini's post appeared

            But I posted my answer a lot of time before it appeared (the forum is so slow for me too)! :-D I think it was even before peterchen OP appeared. ;) :)

            If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler. -- Alfonso the Wise, 13th Century King of Castile.

            1 Reply Last reply
            0
            • L leppie

              Ah right, didnt notice that, but it's probably a good reason why operator overloading is bad!

              xacc.ide
              IronScheme a R5RS-compliant Scheme on the DLR
              The rule of three: "The first time you notice something that might repeat, don't generalize it. The second time the situation occurs, develop in a similar fashion -- possibly even copy/paste -- but don't generalize yet. On the third time, look to generalize the approach."

              P Offline
              P Offline
              peterchen
              wrote on last edited by
              #12

              It's not (if you know what you are doing) :) It's just a convenient way to encapsulate access to the buffer (the actual implementation where this pretty thing was drawn from has some extra offset arithmetics and sanity checking). In my und3erstanding, the bug would also be exposed without the overload, by using helper.m_buffer->value (assuming m_buffer was declared as public T *).

              We are a big screwed up dysfunctional psychotic happy family - some more screwed up, others more happy, but everybody's psychotic joint venture definition of CP
              My first real C# project | Linkify!|[">FoldWithUs!](http://tinyurl.com/37q6tt<br mode=) | sighist

              L 1 Reply Last reply
              0
              • C CPallini

                Following your hints I can guess that in

                peterchen wrote:

                helper->offsetName = helper.Append("MyName"); // BUG here

                the operator-> is evaluated before helper.Append("MyName") (and reallocation) happens, hence the right offset is assigned to the wrong place. I don't know if it the right answer (though I'm quite confident about), but, mon ami, without hints... I did never guess it. :-D

                If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler. -- Alfonso the Wise, 13th Century King of Castile.

                P Offline
                P Offline
                peterchen
                wrote on last edited by
                #13

                Exactly what happens. As I understand, the standard allows the compiler to choose the order in which LHS and RHS are evaluated. As said, I had to check the assembly code, and reason my way backwards why the compiler would do such a thing. Since I expected the memory locaiton to change, it took me a while.

                We are a big screwed up dysfunctional psychotic happy family - some more screwed up, others more happy, but everybody's psychotic joint venture definition of CP
                My first real C# project | Linkify!|[">FoldWithUs!](http://tinyurl.com/37q6tt<br mode=) | sighist

                1 Reply Last reply
                0
                • A admiralh2

                  Well, without diving into the reference manual, it looks like you have two evaluations: A left-side evaluation (helper->offsetName) which returns a reference to the "offsetName" dereference of address of m_buffer A right-side evaluation (helper.Append("MyName")) which may change the value of m_buffer. I believe the choice of which to evaluate first is arbitrary (compiler-dependent). That means that the "helper->" operation will return m_buffer either before or after the evaluation of the right-hand side. Since the evaluation of the rhs can change m_buffer, if the lhs is evaluated first, the code will fail.

                  Hopelessly pedantic since 1963.

                  P Offline
                  P Offline
                  peterchen
                  wrote on last edited by
                  #14

                  Yep, you all nailed it down. Actually, evaluation of LHS and RHS is mixed (the actual implementaiton uses m_buffer+ some offset offset, and the generated code looks like this Load helper.m_buffer into register prepare Append parameters call Append mov [register+offset], eax

                  We are a big screwed up dysfunctional psychotic happy family - some more screwed up, others more happy, but everybody's psychotic joint venture definition of CP
                  My first real C# project | Linkify!|[">FoldWithUs!](http://tinyurl.com/37q6tt<br mode=) | sighist

                  1 Reply Last reply
                  0
                  • P peterchen

                    It's not (if you know what you are doing) :) It's just a convenient way to encapsulate access to the buffer (the actual implementation where this pretty thing was drawn from has some extra offset arithmetics and sanity checking). In my und3erstanding, the bug would also be exposed without the overload, by using helper.m_buffer->value (assuming m_buffer was declared as public T *).

                    We are a big screwed up dysfunctional psychotic happy family - some more screwed up, others more happy, but everybody's psychotic joint venture definition of CP
                    My first real C# project | Linkify!|[">FoldWithUs!](http://tinyurl.com/37q6tt<br mode=) | sighist

                    L Offline
                    L Offline
                    leppie
                    wrote on last edited by
                    #15

                    peterchen wrote:

                    if you know what you are doing

                    Thats not me in this case :)

                    xacc.ide
                    IronScheme a R5RS-compliant Scheme on the DLR
                    The rule of three: "The first time you notice something that might repeat, don't generalize it. The second time the situation occurs, develop in a similar fashion -- possibly even copy/paste -- but don't generalize yet. On the third time, look to generalize the approach."

                    1 Reply Last reply
                    0
                    • A admiralh2

                      Well, without diving into the reference manual, it looks like you have two evaluations: A left-side evaluation (helper->offsetName) which returns a reference to the "offsetName" dereference of address of m_buffer A right-side evaluation (helper.Append("MyName")) which may change the value of m_buffer. I believe the choice of which to evaluate first is arbitrary (compiler-dependent). That means that the "helper->" operation will return m_buffer either before or after the evaluation of the right-hand side. Since the evaluation of the rhs can change m_buffer, if the lhs is evaluated first, the code will fail.

                      Hopelessly pedantic since 1963.

                      L Offline
                      L Offline
                      leppie
                      wrote on last edited by
                      #16

                      Ahh, now I get it :)

                      xacc.ide
                      IronScheme a R5RS-compliant Scheme on the DLR
                      The rule of three: "The first time you notice something that might repeat, don't generalize it. The second time the situation occurs, develop in a similar fashion -- possibly even copy/paste -- but don't generalize yet. On the third time, look to generalize the approach."

                      1 Reply Last reply
                      0
                      Reply
                      • Reply as topic
                      Log in to reply
                      • Oldest to Newest
                      • Newest to Oldest
                      • Most Votes


                      • Login

                      • Don't have an account? Register

                      • Login or register to search.
                      • First post
                        Last post
                      0
                      • Categories
                      • Recent
                      • Tags
                      • Popular
                      • World
                      • Users
                      • Groups