Optimizer bug [modified]
-
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." -
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 bufferUINT Append(LPCTSTR string) { ... }
}Now for the bug:
Helper<CMsgMonitorInfo> helper;
helper->monitorType = EMT_GoodOne; // whatever
helper->offsetName = helper.Append("MyName"); // BUG hereThis 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=) | sighistmodified on Friday, December 14, 2007 8:02:38 AM
Following your hints I can guess that in
peterchen wrote:
helper->offsetName = helper.Append("MyName"); // BUG here
the
operator->is evaluated beforehelper.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. :-DIf 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.
-
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 bufferUINT Append(LPCTSTR string) { ... }
}Now for the bug:
Helper<CMsgMonitorInfo> helper;
helper->monitorType = EMT_GoodOne; // whatever
helper->offsetName = helper.Append("MyName"); // BUG hereThis 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=) | sighistmodified on Friday, December 14, 2007 8:02:38 AM
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->beforehelper.Append(...)is called. Then, if the address of the buffer changeshelper->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
-
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 bufferUINT Append(LPCTSTR string) { ... }
}Now for the bug:
Helper<CMsgMonitorInfo> helper;
helper->monitorType = EMT_GoodOne; // whatever
helper->offsetName = helper.Append("MyName"); // BUG hereThis 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=) | sighistmodified on Friday, December 14, 2007 8:02:38 AM
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.
-
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 bufferUINT Append(LPCTSTR string) { ... }
}Now for the bug:
Helper<CMsgMonitorInfo> helper;
helper->monitorType = EMT_GoodOne; // whatever
helper->offsetName = helper.Append("MyName"); // BUG hereThis 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=) | sighistmodified on Friday, December 14, 2007 8:02:38 AM
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.
-
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->beforehelper.Append(...)is called. Then, if the address of the buffer changeshelper->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
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.
-
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."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 -
Following your hints I can guess that in
peterchen wrote:
helper->offsetName = helper.Append("MyName"); // BUG here
the
operator->is evaluated beforehelper.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. :-DIf 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.
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 -
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.
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 -
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=) | sighistpeterchen 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." -
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.
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."
