Found bugs in version 4.11.3

Need help running G/Q/ZDoom/ECWolf/Zandronum/3DGE/EDuke32/Raze? Did your computer break? Ask here.

Moderator: GZDoom Developers

Forum rules
Contrary to popular belief, we are not all-knowing-all-seeing magical beings!

If you want help you're going to have to provide lots of info. Like what is your hardware, what is your operating system, what version of GZDoom/LZDoom/whatever you're using, what mods you're loading, how you're loading it, what you've already tried for fixing the problem, and anything else that is even remotely relevant to the problem.

We can't magically figure out what it is if you're going to be vague, and if we feel like you're just wasting our time with guessing games we will act like that's what you're really doing and won't help you.
dpJudas
 
 
Posts: 3159
Joined: Sat May 28, 2016 1:01 pm

Re: Found bugs in version 4.11.3

Post by dpJudas »

Chris wrote: Fri Dec 15, 2023 12:16 am I'm not the most fond of his syntax either, though I do have to wonder how much of what I'm interpreting as "ugly" is really just "different".
Of course a lot of it is about what I'm used to and what I've come to enjoy. But if you think about it a bit, it like arguing that I should switch from speaking Danish to English in daily life, because really when it comes down to it that's just a language used to communicate and there's nothing intrinsic about Danish that I couldn't also just say in English. But a Danish speaker don't think that way - they are used to speaking this language, gotten good at speaking it, and enjoy to speak it. Programming languages are no different. I enjoy to read types before variable names, return types before function names, and so on. What Herb Sutter is suggesting is that I abandon everything I'm used to and enjoy and start over. His rationale for it is like telling a German speaker that they should start over because verbs are at the end of a sentence and he met some newcomers that this was confusing when trying to learn German. Even if we take this argument at face value I still need to start over, so why choose Herb's C++ 2.0 then at all? I could then get used to Rust instead. Or maybe C#, D, Go, Typescript, etc.
This is a more general statement about how certain people will try to take the easy way out instead of doing it the right way. It happens in Rust too, where people will just litter the code with unsafe { ... } sections to shut the compiler up about stuff it doesn't deem safe, rather than figuring out the "proper" way to do it. There's no way around that without making the language prohibitively restrictive. A virus only needs to find one point of entry to infect your system, but that doesn't mean one should be easy to find.
You are right about that, but the difference is that now it is no longer a linter (which is really just opinionated warnings if you think about it). Like Graf is also suggesting, I think what C++ really needs is a mode you can configure it into that makes it strict and requiring you to explicitly opt out (perhaps with such an unsafe keyword that Rust got, or how C# did it?). The linter can't fundamentally change the language, so instead it becomes an ocean of opt-in or opt-out lists like the ones you mentioned in the previous post. In a way the linter has become the excuse for the C++ language committee for doing nothing about addressing the fact that Rust can say with a straight face their language is fast and can be secured, while C++ developers have to start mumbling about how if you run linters and stuff then you probably will find the safety critical bugs.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49211
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: Found bugs in version 4.11.3

Post by Graf Zahl »

dpJudas wrote: Fri Dec 15, 2023 10:55 am while C++ developers have to start mumbling about how if you run linters and stuff then you probably will find the safety critical bugs.
And even then it probably will only find some inane things while glancing over the real problems.
User avatar
Chris
Posts: 2967
Joined: Thu Jul 17, 2003 12:07 am
Graphics Processor: ATI/AMD with Vulkan/Metal Support

Re: Found bugs in version 4.11.3

Post by Chris »

Graf Zahl wrote: Fri Dec 15, 2023 10:14 am What I want for safety is a C++ compile mode that can be set to error out on unsafe constructs (like const ->non-const casts) so I can gradually update it.
The only way I can see that working without an all-new syntax is adding something like a strict { ... } section for new/updated functions that enforces appropriate guidelines (and/or the compiler be more strict by default and have a relaxed { ... } section to relax enforcement for old existing functions). Otherwise, it would be a pain, if not impossible, to enable interoperability and gradually update an existing codebase.
Graf Zahl wrote: Fri Dec 15, 2023 10:14 am Ultimately it does not matter if you assign a 64 bit integer to a 32 bit destination with ot without cast. The value gets truncated either way, but in the former case the truncation will even happen if the destination gets extended to 64 bit later and the cast is overlooked because it's virtually impossible to find. So ultimately 'my code compiles without warnings' most of the time just means that considerable effort was invested to hide the problems from the compiler, not to fix them.
A cast expresses intent. If I come across something like uint64_t a; uint32_t b; ... b = a;, I'm going to have no idea if there's a bug there or not (was b supposed to be 64 bit? was a supposed to be 32 bit? did the coder realize the high bits were being lost? is there not supposed to be any high bits to lose at that point?), but if I see b = static_cast<uint32_t>(a);, that let's me know whoever did this knows a isn't 32-bit and b is, and it's meant to convert losing whatever high bits there may be. A cast does matter in that sense, even if the generated bytecode ends up being identical. If b is later changed to a 64-bit type, the existing behavior doesn't change with the cast, while the cast stands out as something to re-check (why cast to a different type than is being assigned to? perhaps change to a a&0xffffffff style mask instead if it's meant to truncate the top bits, but it's the same thing either way). If the destination gets extended to 64 bits, that doesn't necessarily mean the value shouldn't still be truncated. This is basically the kind of bug I ran into that I mentioned before (except it was a "safe" implicit type promotion, rather than a silent truncation); some code was relying on a implicit conversion, some stuff changed that removed the conversion and the behavior silently changed, causing a non-obvious break in the code. If I had an explicit cast where I intended to convert, if I more clearly expressed my intent, it wouldn't have broken.
Graf Zahl wrote: Fri Dec 15, 2023 12:48 pm
dpJudas wrote: Fri Dec 15, 2023 10:55 am while C++ developers have to start mumbling about how if you run linters and stuff then you probably will find the safety critical bugs.
And even then it probably will only find some inane things while glancing over the real problems.
You get the same result if the compiler errors with "don't use const_cast" than if a separate linter errors and stops the build with "don't use const_cast". I'm not saying existing linters are that great, that it wouldn't be better for the compiler itself to handle it, or that we shouldn't push for that, but at least what I've tried this seems decently granular to allow gradual adoption/use to get benefit out of.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49211
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: Found bugs in version 4.11.3

Post by Graf Zahl »

Chris wrote: Fri Dec 15, 2023 5:55 pm
Graf Zahl wrote: Fri Dec 15, 2023 10:14 am What I want for safety is a C++ compile mode that can be set to error out on unsafe constructs (like const ->non-const casts) so I can gradually update it.
The only way I can see that working without an all-new syntax is adding something like a strict { ... } section for new/updated functions that enforces appropriate guidelines (and/or the compiler be more strict by default and have a relaxed { ... } section to relax enforcement for old existing functions). Otherwise, it would be a pain, if not impossible, to enable interoperability and gradually update an existing codebase.
That and the opposite. If strict is opt-in only we gain nothing because code reviewing will still be a mess.
What the C++ community fails to understand that security as an option is not a workable approach, it has to be security by default!


Chris wrote: Fri Dec 15, 2023 5:55 pm A cast expresses intent. If I come across something like uint64_t a; uint32_t b; ... b = a;, I'm going to have no idea if there's a bug there or not (was b supposed to be 64 bit? was a supposed to be 32 bit? did the coder realize the high bits were being lost? is there not supposed to be any high bits to lose at that point?), but if I see b = static_cast<uint32_t>(a);, that let's me know whoever did this knows a isn't 32-bit and b is, and it's meant to convert losing whatever high bits there may be. A cast does matter in that sense, even if the generated bytecode ends up being identical. If b is later changed to a 64-bit type, the existing behavior doesn't change with the cast, while the cast stands out as something to re-check (why cast to a different type than is being assigned to? perhaps change to a a&0xffffffff style mask instead if it's meant to truncate the top bits, but it's the same thing either way). If the destination gets extended to 64 bits, that doesn't necessarily mean the value shouldn't still be truncated. This is basically the kind of bug I ran into that I mentioned before (except it was a "safe" implicit type promotion, rather than a silent truncation); some code was relying on a implicit conversion, some stuff changed that removed the conversion and the behavior silently changed, causing a non-obvious break in the code. If I had an explicit cast where I intended to convert, if I more clearly expressed my intent, it wouldn't have broken.
This completely fails to understand the problem.
Most casts are not there to declare intent but to shut up the compiler.
As things are right now, we get flooded with endless warnings if casts are not added. So they get added liberally all over the code just to quickly silence the warnings the compiler outputs.
Now take a thing like TArray which was programmed long ago in the days where 64 bit was a distant thing in the future, so its size property was made an unsigned int, i.e. 32 bit.
Now, to interoperate with other code using size_t a lot of narrowing casts had to be added to the code to make the compiler shut up.
The current situation is that it is pointless to make the size property an actual size_t because good luck finding all the hundreds of related type casts.
So we are essentially blocked off from fixing an ancient design mistake because we have no means to assess the extent of the problem. And all just to silence compiler warnings that were benign in the first place. Had we instead had an option to annotate the code to say 'This conversion is intended' without specifying the destination type we could do a targeted search for that annotation and get a good overview where fixes are needed.
The fact of programming is that warnings are often just that. They are not programming errors and having to alter otherwise valid code just to get rid of them is a very poor proposition for me.

Chris wrote: Fri Dec 15, 2023 5:55 pm
Graf Zahl wrote: Fri Dec 15, 2023 12:48 pm
dpJudas wrote: Fri Dec 15, 2023 10:55 am while C++ developers have to start mumbling about how if you run linters and stuff then you probably will find the safety critical bugs.
And even then it probably will only find some inane things while glancing over the real problems.
You get the same result if the compiler errors with "don't use const_cast" than if a separate linter errors and stops the build with "don't use const_cast". I'm not saying existing linters are that great, that it wouldn't be better for the compiler itself to handle it, or that we shouldn't push for that, but at least what I've tried this seems decently granular to allow gradual adoption/use to get benefit out of.
But I don't get any warning if I cast away the const with a C style cast! And that's where things get dangerous. All the linters and logic checkers in the world aren't worth anything if one of the most dangerous and most easy to misuse things the language has to offer is not warned about but instead I get pestered about not following the coding conventions of the people who wrote that thing.
I get warnings for all kinds of shit that is ulitmately harmless, what I want is being warned about potential promises made by the implemented code to be accidentally circumvented and this is the #1 issue of all.

Return to “Technical Issues”