61–67 of 67 posts
Is this a new incarnation of this project? https://github.com/Iyengar111/NanoLog Otherwise it seems that the name is already taken.
This is a legitimate question. Why downvoting?
The code overall is pretty clever, but at the core of all this they completely ignore strict aliasing to dump stuff into a char* buffer indiscriminately... Look at this function: https://github.com/PlatformLab/NanoLog/blob/master/runtime/N... T argument = *reinterpret_cast (*in); ... uint32_t stringBytes = *reinterpret_cast (*in); A total of 6 reinterpret_casts in that file alone. I didn't see any indication that you…
What's your problem with the volatile? Using it to prevent register caching of or a value or prevent optimizing out repeated reads is perfectly fine. That is how it is supposed to be used. The bug was that one thread was reading the gcc thought it wouldn't be update between reads. It is even used properly by grabbing a copy of the volatile value instead of reading it repeatedly. edit: I also don't understand your ali…
The code has a race condition, plain and simple. volatile does not lead to thread synchronization, and that function is (as gcc correctly identified) nonsensical in a single-threaded world. The code expects that a different thread writes to the variable at the same time that this thread might read from it - that's the definition of a race condition, which is straight up UB (volatile or not).
You are trying to reason about compiler optimizations from the perspective of the machine and memory model you know ("prevent register caching of a value" etc.). That's wrong though - compiler optimizations operate on the abstract machine that C++ is defined on. In the abstract machine, this code has a race condition (the absence of which is guaranteed to compilers by the C++ standard) and so optimizations could easily violate whatever mental model you are using to conclude the code is fine. The code will likely break again some time in the future, just like it broke this time for a newer compiler version. Or maybe it compiles to subtly broken code right now, who knows!
> I also don't understand your aliasing issues. A `char* ` can point anything and you are allowed to cast back out of it to the correct type.
int is not the "correct type" because no int object lives at the pointed-to location after the cast. They could just memcpy and everything would be fine, but writing to/reading from type aliased pointers (other than char* and friends, in specific circumstances) is UB.
Here is a good description (in the context of C, not C++) of strict aliasing: https://stackoverflow.com/questions/98650/what-is-the-strict...
I spent a lot of time trying to build a fast logging system in my last couple of jobs. The basic lesson (and I'm only talking about C/C++/C# here) is that you will spend most of your time formatting strings if you do your file I/O asynchronously. Since this system has a preprocessor mode, I assume they learnt the same lesson. The bigger lesson is that it really doesn't matter how many millions of logs you can generat…
Which is why good logging systems try to defer as much formatting logic as possible until the last possible moment (ideally render time).
I wish more logging frameworks would log in some efficient intermediary serialization format like Cap'nProto or Protobuf.
Earlier quoted context omitted.
What's your problem with the volatile? Using it to prevent register caching of or a value or prevent optimizing out repeated reads is perfectly fine. That is how it is supposed to be used. The bug was that one thread was reading the gcc thought it wouldn't be update between reads. It is even used properly by grabbing a copy of the volatile value instead of reading it repeatedly. edit: I also don't understand your ali…
> Using it to prevent register caching of or a value or prevent optimizing out repeated reads is perfectly fine. That is how it is supposed to be used. The code has a race condition, plain and simple. volatile does not lead to thread synchronization, and that function is (as gcc correctly identified) nonsensical in a single-threaded world. The code expects that a different thread writes to the variable at the same ti…
No, that isn't. In one thread it reads a value over and over. Another thread will periodically set it. This isn't a race condition and requires no synchonization. On writer thread only, and you're fine. If the code was doing more, I didn't see it.
I just wrong something very similar yesterday where I had a counter that is read over and over and an async handler updated it. I had to make it volatile to prevent the the spin on it from optimizing out the load.
Yes, in low level programming, you have to know about the hardware, what a register is, what cache it, etc. C++ has a very lose (almost nonexistent) abstract model. This isn't UB.
> int is not the "correct type" because no int object lives at the pointed-to location after the cast
The string had its length prepended to it. There was an int there. Also, those aliasing rules are more about optimization issues, not as much about alignment issues (eg, if you are only writing for intel/amd hardware alignment only really matter for SIMD, and there is no penalty on unaligned access (with a few small exceptions that didn't apply here).
Earlier quoted context omitted.
> Using it to prevent register caching of or a value or prevent optimizing out repeated reads is perfectly fine. That is how it is supposed to be used. The code has a race condition, plain and simple. volatile does not lead to thread synchronization, and that function is (as gcc correctly identified) nonsensical in a single-threaded world. The code expects that a different thread writes to the variable at the same ti…
> The code expects that a different thread writes to the variable at the same time that this thread might read from it - that's the definition of a race condition, No, that isn't. In one thread it reads a value over and over. Another thread will periodically set it. This isn't a race condition and requires no synchonization. On writer thread only, and you're fine. If the code was doing more, I didn't see it. I just w…
Sorry, wrong term by me. I meant data race, not race condition (although many would regard the former as a form of the latter). A data race is two or more threads accessing the same location without synchronization, where at least one of them is modifying the value. A data race results in undefined behavior. Here are the relevant sections from the C++ (draft) standard:
http://eel.is/c++draft/intro.multithread#intro.races-21
http://eel.is/c++draft/intro.multithread#intro.races-2
> I just wro[te] something very similar yesterday where I had a counter that is read over and over and an async handler updated it.
If you wrote this code in C++, you created a data race and your program has undefined behavior.
> I had to make it volatile to prevent the the spin on it from optimizing out the load.
volatile may prevent the load from being optimized out, but it does not prevent the data race (because it does not prevent instruction reordering). Your program still has undefined behavior.
Further reading:
https://www.kernel.org/doc/html/latest/process/volatile-cons...
> C++ has a very lose (almost nonexistent) abstract model
I don't know what you mean with "lo[o]se" or "almost nonexistent" but here are a few specifications of the abstract machine that C++ is defined on:
http://eel.is/c++draft/basic.memobj
http://eel.is/c++draft/basic.exec
> There was an int there.
No. Just because you operate on the memory as if it was an int doesn't create an int there.
> Also, those aliasing rules are more about optimization issues
Correct! And those optimization issues are the primary way in which UB manifests itself in your program.
Earlier quoted context omitted.
> The code expects that a different thread writes to the variable at the same time that this thread might read from it - that's the definition of a race condition, No, that isn't. In one thread it reads a value over and over. Another thread will periodically set it. This isn't a race condition and requires no synchonization. On writer thread only, and you're fine. If the code was doing more, I didn't see it. I just w…
> No, that isn't. In one thread it reads a value over and over. Another thread will periodically set it. This isn't a race condition and requires no synch[r]onization. On[e] writer thread only, and you're fine. Sorry, wrong term by me. I meant data race , not race condition (although many would regard the former as a form of the latter). A data race is two or more threads accessing the same location without synchroni…
Earlier quoted context omitted.
> The code expects that a different thread writes to the variable at the same time that this thread might read from it - that's the definition of a race condition, No, that isn't. In one thread it reads a value over and over. Another thread will periodically set it. This isn't a race condition and requires no synchonization. On writer thread only, and you're fine. If the code was doing more, I didn't see it. I just w…
> No, that isn't. In one thread it reads a value over and over. Another thread will periodically set it. This isn't a race condition and requires no synch[r]onization. On[e] writer thread only, and you're fine. Sorry, wrong term by me. I meant data race , not race condition (although many would regard the former as a form of the latter). A data race is two or more threads accessing the same location without synchroni…
Reading and writing to the same int in memory is atomic for every hardware platform I care about. Even alignment issues don't matter for anything this is going to be run on.
> No. Just because you operate on the memory as if it was an int doesn't create an int there.
Don't be dumb. They wrote an int to the beginning of the buffer and then had to cast back to it, like every piece of networking or file code that had to save a binary number.