- Feature Articles
- CodeSOD
- Error'd
-
Forums
-
Other Articles
- Random Article
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
There is another bug in this, depending what the memory was being used for after it was allocated. The string 'abc' would require 'wcslen(abc)+1' characters if null-terminated.
Admin
sprintf(xyz, "%s", abc) and you fly away from memory together with your program
Admin
TRWTF is using new wchar_t[] in the first place. Can't think of a single good reason to prefer such a C-ish idiom over std::wstring.
Admin
Those who abuse unicode are condemned to ø§¢£
Admin
Actually, if the last character of the string is being corrupted, it should be pretty straightforward to set a data breakpoint there, and catch whatever is modifying it after it's been set. Then, of course, you'd find that an assignment to a completely legit and unrelated variable is modifying your array, and that's something else.
Admin
Wait. What happens when you assign a wchar to wchar*? Does it store pointer to stack where the wchar lives, or takes the value of wchar and makes that a pointer?
Addendum 2019-02-12 07:21: Never mind it answers my question in the article.
Admin
Undefined behaviour is undefined. No surprise there.
In fact, the main surprise with UB is that weirdness isn't more common, although I guess if it was, we'd see more noobs crying on StackOverflow about buggy compilers when in fact it's their fault.
Admin
takes a value and makes a pointer
Admin
Maybe. The last time I dealt with a memory corruption thing like this:
I eventually compiled the program with debug info and loaded it into Turbo Debugger (not to run it, mind, but just to look at memory), and discovered that the string being corrupted was exactly
sizeof(interrupt_table_t)bytes away from a variable that was modified every second by an interrupt handler. And that led me to find a piece of code that modified DS (0x40=>0=>0x40) to address a memory-mapped real-time clock(1), and didn't disable interrupts, and that the interrupt handlers didn't restore DS to the right value before calling the "worker" code.The "window of opportunity" was three instructions long, and my colleague who was also investigating the bug didn't believe that that could be the bug because it couldn't possibly trigger the problem often enough(2). I insisted that he wouldn't be able to find the real bug until he fixed the thing I had found (and he - reluctantly - agreed), so he fixed it, and the bug went away, never to return.
(1) No, of course it wasn't necessary to modify DS, but the (external) developer who had cobbled this together had decided to do so. That was, of course, the guy who celebrated buying a new car by leaving us the kind gift of a critically important variable called
golf_gti.(2) He was wrong, of course. It generally took about 20 minutes for the interrupts to line up with the clock-reading code.
Admin
My favorite bug from my C days...
char* s = malloc(strlen(GetText(SOME_ID)+1));
Get the length of GetText(SOME_ID) and add one for the NUL terminator? Right? Its been working all this time and the only change is making some other string shorter!
Admin
One good reason: stdlib stuff can do funny things when used across DLLs due to how it handles memory, thus the general rule at one of my former shops where we used a very DLL-heavy architecture was that you could (and should) use strings and STL containers all day long within a single DLL, but interfaces between DLLs had to be raw pointers.
One very very bad reason, from another place I worked: the co-founder/CTO had implemented his own library of strings and containers back when the STL was still new and untrusted, and thanks to inertia (and not a little pride) this homebrew library was the very foundation of the system that this company was building when I joined. Of course the library was full of gotchas and unexpected behavior. When I suggested that we switch to something more standardized (i.e. STL) to make it easier to onboard new folks and reduce the potential for mistakes, his response was "good programmers don't make those kinds of mistakes". I can't say I was completely surprised when the company folded within a year...
Admin
Also... why wchar's and not UTF-8?
Admin
Screw it. Hope the users don't notice the watchdog resets.
Admin
Unfortunately, the corrupted memory was a string literal used as a header when printed. When corrupted, it was immediately apparent as soon as you printed stuff, so there was no way to avoid fixing the bug.
Admin
"good programmers don't make those kinds of mistakes".
It can be highly satisfying to reply to that sort of comment with "Well here's a mistake that didn't make itself, buster, and it's got your name plastered all over it."
Admin
Unless the incident happened a very long time ago, this should have been caught by any number of automated analysis/testing tools. Both static analysis tools (e.g. Coverity) and dynamic analysis tools (e.g. Valgrind) would have immediately caught a bug like this.
TRWTF is developing a non-trivial software project without any such procedure in the workflow.
Admin
I admit I did smirk a bit when a crash that brought down the production server for a week traced back to a memory leak in his map implementation.
Admin
Watchdog resets? Ha! I once worked on a 6809-based system with a watchdog that was cleared by writing to a memory address. Except that I found that it also was cleared by a READ to that address, because Motorola CPUs of that era had a test instruction which put the CPU into a "read all addresses sequentially forever" mode which could only be exited by a reset signal. Of course there's no way the CPU will ever find a $14 or $15 opcode to execute, and it's undocumented too, so only the most grizzled veteran programmers will even know about it. (There's also an undocumented 6809 instruction which does a SWI through the FIRQ vector while pushing the full register set.) It also locked up my in-circuit emulator because it shared the same CPU for both sides, making it so much more fun to track down.
The best part was when I discovered this, we had recently developed second-generation hardware where the WR input had been removed from the PAL that generated the watchdog clear address decode. Presumably the hardware guy (likely the same contract guy who made the initial mistake a few years before) noticed that it wasn't being used by that PAL, so might as well remove it. I never found out what they did about that.
Admin
Oh, I forgot to mention that this was for unattended equipment. Did you know that Roswell NM is half an hour away from an airport? Or at least it was 20 years ago. At least the equipment wasn't in orbit waiting for someone to push a reset button. And there were also no CD-ROM trays nearby.
Admin
"TRWTF is using new wchar_t[] in the first place."
It's pretty obviously a naive minimal effort port, where they just replace all char with wchar_t (including the bugs!) without thinking about it or taking advantage of C++'s advantages at all.
Admin
It's not naive to keep your refactoring (or, really, any branch) as focused as possible, especially when it's one that touches a large portion of the codebase. Simply being faster to release means there's less pain merging with other branches, and reviewers can often spot typos when the changes are all relatively uniform.
And it doesn't preclude later refactoring that changes the actual datatypes. And at that point, you'd be reasonably certain new bugs from that weren't due to the wchar_t transition.
Admin
I think this WTF was neatly tied up.
Admin
It's not exactly "naive" to implement a minimal port, as you say.
But it's actually frickin cretinous to port something from <language here><year of standard here> to <ditto><different> without actually considering the language and the standard.
In this particular case, not solving the technical debt with a trivial use of a well-tested and entirely appropriate language feature is ... TRWTF.
Admin
All of this demonstrates what happens when you turn off warnings in a compiler, and forget about them. Of course warnings that are ignored off handedly (we always get those) are just as bad.
At a company long ago, a project (not mine) was cobbled together, and they had lots of errors that they were constantly working around, and when somebody decided to FIX them, lots of other bugs were cured in the process. Who would have suspected!!
Live and learn!
Admin
"Good programmers don't make those kind of mistakes" "Yeah, but maybe we could also not make those kind of mistakes."
Admin
20 years ago, Roswell NM had two airports within city limits. One has since closed.
Admin
TRWTF is using wchar_t instead of UTF-8. wchar_t has an implementation-defined size, which makes it nearly useless for cross-platform code: it's 2 bytes on Windows but typically 4 bytes on POSIX-like systems. If you use UTF-8 internally and convert to UTF-16 when needed for e.g. Windows APIs, it will work identically across all platforms.
Admin
Admin
easy fix! new wchar_t(wcslen(abc)+1);
Admin
Interesting, I had an old grizzled veteran aero engineer tell me that their ethos back then was "If it's on the same die, it won't fly." eg. you should trust the internal WD exactly as far as you can throw it; they had to use external WD circuitry, and also log how often the WD was activated.
Admin
TRWTF #1 is using wide characters at all. UTF-8 has shown to be much more versatile than a duplication of all the standard string functions, I/O functions, etc.
TRWTF #2 is using new[strlen(...)]. This is presumably followed by a copy in, which basically means that strdup() would have been easier, or wcsdup if you insist on hamstringing yourself with wide characters.
Admin
They should have just done what Twitter did and limit messages to 140 characters. Problem solved!
Admin
Even more fun: it's actually going to be one character less than you need; there's a NUL terminator that'll not fit…
Admin
"tracing through 7,000 lines of label printing code to figure out where things got thrown off was more of a chore"
Apparently, Rita is also not aware that tools like Valgrind exist. Running under Valgrind (or DrMemory or similar on Windows) would immediately display the exact location of any instances of this kind of memory corruption.
Admin
No, with this kind of code base that would immediately have you swamped in errors and warnings (well, warnings mostly, the compiler apparently is fine with it, given a small amount of fine - disabling/ignoring warnings and such).
Admin
No, with this kind of code base that would immediately have you swamped in errors and warnings (well, warnings mostly, the compiler apparently is fine with it, given a small amount of fine - disabling/ignoring warnings and such).