Bad code? As in *really* bad?



  • Bad code? As in really bad? As in so goddamn awful that the compiler shits itself and tries to escape?

    Occasionally, I sort-of-maintain a homegrown C++ plugin to a widely used CAD software, with a quite large set of illogical discrepancies and bugs that have accumulated over the last decade or so. The plugin, that is. The CAD thingy is sort of decent.

    Their latest version mandates VS2015, with the usual potential of subtly breaking changes, such as overrides that still compiles but no longer overrides, and familiar stuff. Or so I thought, yesterday.

    Anyway, first let's see if it compiles, . No. Internal error so-and-so. Hmm, that's unusual, at least for the more recent versions. Since it was rather late, I gave up there and went home.

    This morning, VS no longer usable on the that box. Had to call IT and have them do their stuff. But that piece of code is pure evil.

    I need a drink.



  • If you're using VS2015 you may as well be using the override keyword on functions to find those pesky no-longer-overrides functions.


  • Impossible Mission - B



  • I used to get the occasional ICE when doing "fun" stuff in C++, though that was with older versions than VS2015. Also got one with clang once, fun times.

    Just got to isolate the problem and get around it. VS2015 is halfway decent, so it's likely there's something wrong in your code that the compiler gets tripped up on (instead of erroring out on).



  • @flashback said in Bad code? As in *really* bad?:

    Had to call IT and have them do their stuff

    ugh... Developer that can't control his own box? That's a cherry on the shit cake...



  • @WernerCD said in Bad code? As in *really* bad?:

    ugh... Developer that can't control his own box? That's a cherry on the shit cake...

    I see that you've never tried to install/uninstall Visual Studio 🍹



  • @WernerCD said in Bad code? As in *really* bad?:

    ugh... Developer that can't control his own box? That's a cherry on the shit cake...

    That's nothing. Sometimes infosec will reply that I can't have something despite it being necessary to do my job and we lose customers.



  • @WernerCD said in Bad code? As in *really* bad?:

    @flashback said in Bad code? As in *really* bad?:

    Had to call IT and have them do their stuff

    ugh... Developer that can't control his own box? That's a cherry on the shit cake...

    On one of my work computers, I can't even change the mouse settings.



  • @CarrieVS
    Well, if you'd quit biting the hand that brings you mice, maybe you'd be in better shape... 🍹





  • @TimeBandit said in Bad code? As in *really* bad?:

    @WernerCD said in Bad code? As in *really* bad?:

    ugh... Developer that can't control his own box? That's a cherry on the shit cake...

    I see that you've never tried to install/uninstall Visual Studio 🍹

    Generally speaking... I'd rather format/install than uninstall/install Visual Studio lol... there are WAAAAAY to many dependencies...

    @CarrieVS said in Bad code? As in *really* bad?:

    @WernerCD said in Bad code? As in *really* bad?:

    @flashback said in Bad code? As in *really* bad?:

    Had to call IT and have them do their stuff

    ugh... Developer that can't control his own box? That's a cherry on the shit cake...

    On one of my work computers, I can't even change the mouse settings.

    I'd refuse to work in such an environment... I'm paid good money and if I can't be trusted to take care of my own computer then I'm apparently not good enough to be on your payroll. Just my 2c.



  • @flashback said in Bad code? As in *really* bad?:

    This morning, VS no longer usable on the that box.

    I work in VS2015 with ResharperC++ installed. And lately I have to restart the computer every couple of days, because Visual Stupido gradually slows itself down to a crawl when building and takes the rest of the computer with it. No idea what causes it, but it's been observed by other people too.



  • @wharrgarbl said in Bad code? As in *really* bad?:

    That's nothing. Sometimes infosec will reply that I can't have something despite it being necessary to do my job and we lose customers.

    Tangential: Our company's dev ops won't install nodejs on the servers so we have to commit webpack bundles into the repo.

    "Your PR has conflicts, Shoreline." No shit, Sherlock.



  • @Bulb said in Bad code? As in *really* bad?:

    No idea what causes it

    Based on reputation I'd guess

    ResharperC++


  • And then the murders began.

    @hungrier Unless their C++ product is a hell of a lot better than their C# one, that'd be my guess too.


  • FoxDev

    @Unperverted-Vixen said in Bad code? As in *really* bad?:

    @hungrier Unless their C++ product is a hell of a lot better than their C# one, that'd be my guess too.

    There's no point in using Resharper for C# nowadays anyway: VS itself has all you need already baked in.



  • @Bulb I've found that Visual Studio acts like that a lot less without Resharper.


  • FoxDev

    @RaceProUK said in Bad code? As in *really* bad?:

    @Unperverted-Vixen said in Bad code? As in *really* bad?:

    @hungrier Unless their C++ product is a hell of a lot better than their C# one, that'd be my guess too.

    There's no point in using Resharper for C# nowadays anyway: VS itself has all you need already baked in.

    in fact the only reason i pay for it these days is that it's cheaper to get the subscription that includes it than to get the subscriptions with all their other tools that are still hella useful and awesome.

    also i do occasionally need to use ancient versions of VS for insane client reasons, and in those cases resharper helps keep my blood alcohol levels marginally sub lethal.


  • Impossible Mission - B

    @accalia said in Bad code? As in *really* bad?:

    in fact the only reason i pay for it these days is that it's cheaper to get the subscription that includes it than to get the subscriptions with all their other tools that are still hella useful and awesome.

    This. dotPeek and dotTrace are invaluable to a compiler developer. Reshaper... meh. Nice to have.


  • Discourse touched me in a no-no place

    The worst code I ever saw was this (approximately; it was long ago):

    /* This is type-correct and compiles just fine. */
    int foo(void) {
        char ch = '\0';
    
        (void) scanf("%1s", &ch);
        return ch;
    }
    

    On big-endian systems with conventional alignment rules, it works just fine, returning the next non-whitespace character read from standard input. On little-endian systems, it smashes one byte of the function return address on the stack, and all hell breaks loose in the program shortly after.

    Took me a week to find that (admittedly in rather more complex real-life code).



  • @CarrieVS said in Bad code? As in *really* bad?:

    @WernerCD said in Bad code? As in *really* bad?:

    @flashback said in Bad code? As in *really* bad?:

    Had to call IT and have them do their stuff

    ugh... Developer that can't control his own box? That's a cherry on the shit cake...

    On one of my work computers, I can't even change the mouse settings.

    In a computer lab one time, I saw a pc that was displaying a message box to the effect that "an administrator is required to install the following hardware: mouse."

    The mouse was working fine, in case you wondered.



  • @dkf said in Bad code? As in *really* bad?:

    On big-endian systems with conventional alignment rules, it works just fine,

    Is scanf overwriting just space aligning filler bytes in that case?

    TRWTF is C lack of a good, standard, string type



  • @wharrgarbl said in Bad code? As in *really* bad?:

    @dkf said in Bad code? As in *really* bad?:

    On big-endian systems with conventional alignment rules, it works just fine,

    Is scanf overwriting just space aligning filler bytes in that case?

    TRWTF is C lack of a good, standard, string type

    scanf is reading a one-character, null-terminated string... in other words, two bytes: the character entered, followed by \0.

    Since there's only one byte allocated for the char type, it's blowing up the return address either way, overwriting one byte with \0, but as luck would have it, that's the MSB of the return address on the big-endian system, and it apparently always had a null byte at that location anyway, so nothing Really Bad happened. On the little-endian system, though, it's overwriting the LSB of the return address, which is almost always going to cause Bad Things.



  • @anotherusername said in Bad code? As in *really* bad?:

    Since there's only one byte allocated for the char type,

    There could be 3 unused bytes between the char and the return address because of memory alignment, but I'm not sure as I never tested or thought much about this.



  • @wharrgarbl (& @anotherusername )

    I didn't test either, but I'd expect there'd indeed be padding between the char and the return address - on a little endian system.
    On a big-endian system, however, I'd expect the padding to be before the char, so the return address would immediately follow the char.

    If true, this is a better explanation for why nothing bad happened on the little-endian machine than MSB-clobbering.


  • Discourse touched me in a no-no place

    @CreatedToDislikeThis It's a combination of all your explanations.

    We're taking (and handing to another function) the address of a variable, so it has to be placed on the stack. The stack is actually allocated in chunks the size of a machine word (well, whatever the size of a register is, but that's pretty much the definition of “machine word”); on the 32-bit machines I was using at the time, it was done in 4 byte chunks. But which byte of those 4 is the char? That's where the endianness comes in; it is actually the low byte on a big-endian SPARC and the high byte on a little-endian x86 (and char* doesn't need to be aligned on any architecture), at least with the compilers of the time. The remaining 3 bytes are entirely unused. The next address higher up the stack holds the return address, as the function is otherwise pretty simple and most architectures use a stack that grows down.

    Next, the behaviour of the scanf. That looks forward on standard in until it finds a non-whitespace character (as part of the fix, I tried switching to %c and that broke the code “conventionally” and uninterestingly) and then reads the number of characters specified in the field width (1) into the string pointed at by the matching argument. Which indeed writes two bytes, one into the char we have waiting for it, and another (a \0) into the next higher location.

    On our big-endian system, the next higher location is an unused padding byte. On the little-endian system, the next higher location is part of the return address of the function, and that makes returning from the function become a shit-hit-the-fan event. In my particular case, it managed to struggle on for quite a while (unusual!) and thoroughly shredded the stack and a fair chunk of the heap before actually crashing. The resulting core dump would make the debugger itself dump core, and there was a flaw in the core dumper at the time which meant that it always wrote core dumps to a file core in the current directory, which happened to overwrote the core dump I was trying to analyse. 😆

    The real code which this came from was rather more complex, of course. This particular function was only called a long way into the execution (well after a non-trivial GUI was launched and quite a lot of other I/O had been done) and the actual function that I simplified the sample from was a lot less obvious. In the end, I had to scatter debugging prints through the program just to narrow down where the detonation was kicking off, which got me close enough that I could single-step. When I saw that the return was going to crazyland, my jaw dropped; a bit of investigation later, and a solution wasn't hard to find (a two-character array as a target buffer in the first instance, with switching to a proper parser as a longer term plan, IIRC). But the truly annoying thing is that this code appeared to work fine on Solaris (where we had sophisticated-for-the-time analysis tools such as Purify) because there the overflowing write was always within a single word, which is about the actual limit of resolution of tools that work on object files; you really need source analysis to pick up the trouble.

    It was a good lesson in how abstractions don't always hold when working with computers; low-level details can sometimes come busting right through, no matter how many precautions you think you've taken. And that one's support tools are themselves merely fallible programs…



  • @dkf said in Bad code? As in *really* bad?:

    It was a good lesson in how abstractions don't always hold when working with computers

    FTFY


  • Discourse touched me in a no-no place

    @ben_lubar I write compilers for fun now, which is a good lesson in understanding what is really knowable about programs. Trust me, those fancy assumptions in other languages can get broken, and when that happens you'll be just as confused as I was.



  • @dkf said in Bad code? As in *really* bad?:

    @ben_lubar I write compilers for fun now, which is a good lesson in understanding what is really knowable about programs. Trust me, those fancy assumptions in other languages can get broken, and when that happens you'll be just as confused as I was.

    Any language that lets you write to padding bytes between values without using some sort of "YES I KNOW THIS IS A BAD IDEA" flag on the compiler is not a safe enough language for me.



  • @dkf said in Bad code? As in *really* bad?:

    The resulting core dump would make the debugger itself dump core,

    The WTFs are piling on this one



  • @ben_lubar said in Bad code? As in *really* bad?:

    Any language that lets you write to padding bytes between values without using some sort of "YES I KNOW THIS IS A BAD IDEA" flag on the compiler is not a safe enough language for me.

    Nobody ever tried to sell C as a safe language


  • Impossible Mission - B

    @wharrgarbl said in Bad code? As in *really* bad?:

    TRWTF is C lack of a good, standard, string type

    QFT. Joel Spolsky once said that most of the history of the evolution of the C++ language can be explained in terms of trying to deal with the consequences of this particular :wtf:.


  • Impossible Mission - B

    Having said that, the real TRWTF here isn't C's lack of a real string type, but its lack of a real array type.

    A parameter declared as char* has no way to differentiate between a pointer to a single char (what your code was sending it) and an array of chars (what it was expecting.) The C type system has no mechanism to permit the one and reject the other, even though they're conceptually totally different things.



  • @masonwheeler said in Bad code? As in *really* bad?:

    Having said that, the real TRWTF here isn't C's lack of a real string type, but its lack of a real array type.

    A parameter declared as char* has no way to differentiate between a pointer to a single char (what your code was sending it) and an array of chars (what it was expecting.) The C type system has no mechanism to permit the one and reject the other.

    What about a parameter declared as char foo[]?


  • Impossible Mission - B

    @ben_lubar Is that a real thing? I've never seen that syntax in C before.



  • @masonwheeler C has an array type but it frequently decays into a pointer. char[30] is an array of 30 characters, but char[] is contextual and sometimes a pointer and sometimes an array with a deduced size.


  • FoxDev

    @LB_ IIRC, C compilers implement arrays as pointer+index anyway, which is why there's no bounds-checking: the size of the array isn't included in the compiled code


  • Impossible Mission - B

    @LB_ Yes, I know. I was asking specifically about declaring a function parameter with an array type. AFAIK that's not legal C.



  • @masonwheeler It is definitely legal C, it just decays to a pointer type if you don't specify a size.



  • @LB_ said in Bad code? As in *really* bad?:

    @masonwheeler It is definitely legal C, it just decays to a pointer type if you don't specify a size.q

    Compiler just ignore the size if you specify



  • @wharrgarbl There is a notable difference in the behavior of sizeof, so regardless of the resulting machine code, the size is significant during compilation.



  • @LB_ said in Bad code? As in *really* bad?:

    @wharrgarbl There is a notable difference in the behavior of sizeof, so regardless of the resulting machine code, the size is significant during compilation.

    No

    0_1495931899324_Screenshot_20170527-212828.png



  • @wharrgarbl Ah you're correct, I was remembering the C++ trick to work around it: https://ideone.com/b4Y7bE
    My apologies.



  • @LB_ you made C strings look sane now



  • @RaceProUK said in Bad code? As in *really* bad?:

    @LB_ IIRC, C compilers implement arrays as pointer+index anyway, which is why there's no bounds-checking: the size of the array isn't included in the compiled code

    Arrays and pointers to the first element aren't the same when you use sizeof on them.


  • Discourse touched me in a no-no place

    @LB_ said in Bad code? As in *really* bad?:

    There is a notable difference in the behavior of sizeof

    Indeed there is, except not when the array is passed via a function argument. When you pass an array as a function argument, you might as well have said that it was a pointer for all the observable difference you get; in the language of the language definition, the array is decayed to a pointer to the first element. I suppose there's a little bit of use in using array syntax in function signatures, in that it declares the intent to potentially access multiple elements of the array, but that's about all. What's more, there's a lot of code out there that actually requires that, even where there's a non-zero bound on an array, bounds checking not be done. The major culprits for this are either where you've really got a structure with a variable-length array as last member (a zero-length array would work for this except that it makes some older compilers choke) or where you're doing non-portable type magic (which is unfortunately necessary sometimes; damn hardware vendors!).

    C is fucking annoying in places. But as long as you know what's going on, it's not actually too awful. You just need to be aware that you're about one small step above machine code, no more. That's C's real niche.


  • BINNED

    @RaceProUK cough Ada cough



  • @dkf said in Bad code? As in *really* bad?:

    On big-endian systems with conventional alignment rules, it works just fine, returning the next non-whitespace character read from standard input. On little-endian systems, it smashes one byte of the function return address on the stack, and all hell breaks loose in the program shortly after.

    Are you sure about which system was which? I would expect it to be the other way around, working on x86 and failing on sparc.

    @dkf said in Bad code? As in *really* bad?:

    it is actually the low byte on a big-endian SPARC and the high byte on a little-endian x86

    It is the least-significant byte in both cases. Now little-endian, a.k.a. least-significant-first, stores least-significant byte at the lowest address, while big-endian, a.k.a. most-significant-first, stores that byte at the highest address. So on little-endian it should overwrite the padding … NO.

    Actually, it's totally irrelevant either way. The variable is char and the compiler is free to place it wherever it wants. So the 3 padding bytes can go before, or after, or not be there at all, because the compiler packed the variables together. Since the variable is never used as int, the endianity does not come to play.

    Instead, the reason would be that the two compilers simply packed the local variables differently. The one where it worked had some padding after this variable, or it could have had some other variable there that was not in use at that particular point, so overwriting it didn't matter. The one where it failed packed it right before the return address.

    @dkf said in Bad code? As in *really* bad?:

    and char* doesn't need to be aligned on any architecture

    No, but IIRC on sparc, 32-bit instructions have some advantages over 8-bit ones, which is not the case of x86. So the sparc compiler might have had better reason to actually align the address. The x86 one didn't, so it packed the variable.

    @wharrgarbl said in Bad code? As in *really* bad?:

    Nobody ever tried to sell C as a safe language

    … and yet all the security-critical bits of our infrastructure are written in it.

    @masonwheeler said in Bad code? As in *really* bad?:

    Having said that, the real TRWTF here isn't C's lack of a real string type, but its lack of a real array type.

    C has exactly the right building blocks. It should have a library on top of them though.

    @masonwheeler said in Bad code? As in *really* bad?:

    The C type system has no mechanism to permit the one and reject the other, even though they're conceptually totally different things.

    Actually there would be many ways to implement a (mostly—C is inherently weakly typed and you are always one bad cast away from a disaster) type-safe abstraction for strings. But nobody did.


  • Discourse touched me in a no-no place

    @Bulb said in Bad code? As in *really* bad?:

    Are you sure about which system was which?

    Yes. The code had been developed on Solaris SPARC workstations (definitely big-endian!) and I was porting it to Linux on PC (definitely little-endian) so that I could work on it at home.

    I was too young to know to not work on work when at home…



  • @dkf Then the reason was not endianity, but how each compiler chose to lay out the variables. And neither compiler was laying it out the obvious way into an integer.


Log in to reply