There's an interesting subtlety mentioned in the author's explanation http://www.linusakesson.net/programming/underhanded/2015-spo... that doesn't seem to be mentioned here: the preprocessing had to be carefully designed because the squashing also amplifies any residual noise, so if that wasn't thoroughly removed the comparison would fail because it'd mostly be comparing noise rather than signal. While the type confusion is the only suspicious thing, the rest of the code did have to be crafted to make it work, it's just that the other changes are reasonable and non-suspicious.
> In contrast there are masters in the martial arts who learned their art as a means of survival and became masters in a realistic and hostile environment. We don’t have anyone like this in the programming profession, or at least I haven’t met any.
“I wish I knew what to tell you that would lead you to write good Forth. I can demonstrate. I have demonstrated in the past, ad nauseam, applications where I can reduce the amount of code by 90% percent and in some cases 99%. It can be done, but in a case by case basis. The general principle still eludes me.”
I have a background in system design, programming, and martial arts. That was a great essay. Really enjoyed it. So thanks for linking it. I'm glad I've finally gotten into the Master phase in the sense of philosophy rather than claiming a certain talent level.
I'm sick of all the needless complexity in what people push. I'm old enough to have seen it repeat in many fads over time. I've seen designs that beautifully and simply (for user/developer) handled their requirements. I've even seen the master programmers he thinks don't exist that write maintainable, good code under deadlines. I've seen amateurs following good principles come close enough. So, I'd like to see more people emulating the principles of mastery he espouses using any proven method to get there and avoiding common pitfalls.
A significant rise in amateurs on that path would itself deliver a better baseline than what today's experts are pushing. I'll take a well-trained amateur that hates complexity over an expert any day. 80/20 rule says I don't need many experts anyway for most jobs.
> I'll take a well-trained amateur that hates complexity over an expert any day.
I would consider someone sufficiently well-trained, yet wise enough to understand the value in simplicity, humble enough to listen to the ideas of others and keep learning, and curious enough to actively consult others for dissenting ideas to be one of tomorrow's experts.
Very well worded haha. Given my background, I should throw in an exception for INFOSEC. If it's high security, you want as many experts as you can get to review various aspects for pitfalls and possible suggestions. Just too much to worry about for one master and good amateurs as complexity grows. Still leave final decision and priorities on any of that to the leader who was a master or an expert that's a cut above the rest in wise decision-making. That should filter BS and committee-think while getting review benefits.
That's actually been my recommendation for a while for high assurance. What you think of it?
Somewhat related anecdote: Trying go get WordPress to adopt a CSPRNG was painful for a year.
Then I started paragonie/random_compat and like 30 other people pitched in to improve it, and then I suggested just using that (so new code can be written against PHP 7's API). And so the problem was solved.
I'd tend to agree that, for security matters, a small team of people focused on success with the knowledge and/or resources they need to execute on their own initiatives gets a better result than a large team with varied interests and use cases.
That was my intention. Dion Hulse really pulled through on WP's end, and he's quick to pull in upstream changes into their trunk branch for testing. :)
> In contrast there are masters in the martial arts who learned their art as a means of survival and became masters in a realistic and hostile environment. We don’t have anyone like this in the programming profession, or at least I haven’t met any.
I'm wondering if Zed would consider say Carmack for example, such a master coder?
And if, like me, you didn't knew what this contest was about, the explanation is one click away [1]:
"a competition that challenges coders to solve a simple data processing problem by writing innocent-looking C code that is as readable, clear, and seemingly trustworthy as possible, yet covertly implements a malicious function."
Those for whom the name Linus Åkesson is unfamiliar are highly encouraged to visit his site at http://www.linusakesson.net/ --- he has a lot of other interesting articles on programming and the demoscene.
As I understood the comment from the context, it was to chide the typedef-er for trying to save on typing by defining frivolous and obfuscating typedefs. He appears to be saying the defining a typedef to save on typing a long name is a false savings, as I understood it.
Interestingly, in our codebase at work we use completely opposite approach, but the code is C++, not C. We try everything to have its own type, so instead of "unsigned long", we would use "widget_counter_t", for example. I almost never encountered a situation, where I would need to know that widget_counter_t is actually unsigned long. And in rare case I need to know, I just hit F12 in Visual Studio and look at the type. But normally, variable type should semantically define what the variable holds, not describe the byte representation of that variable.
Is there a specific reason for using typedefs rather than (packed) structs? With typedefs you get no real type safety (you can pass a widget_counter_t to a method that expects a foo_t if both typedef to the same thing), do you?
This is probably an amateur C question, but would the typedef in the winning entry be considered good practice if you want to swap the type easily? Like being able to change to a long double if a detector with higher precision is made, or int after noticing that all incoming data are ints anyways? I'd also probably give the type more descriptive name (which breaks the beautiful bug). Or would there be a better way to handle that sort of situation?
That's a fine question to ask. Yes, using a typedef for this purpose is good practice. It's debatable if it's a good idea to add the abstraction of a typedef if you don't need the flexibility, but if you are trying to write generic code in C, a typedef like this is the best approach.
The only real alternative would be a a preprocessor define, which is even more "heavyweight". A define written in uppercase might be better if you want to signify that it's something changeable, but the typedef is probably slightly safer.
The real issue here is not the typedef, but the name choice. "float_t" is a reserved name, and it's unlikely to be a good idea to typedef a reserved name. Counter to common practice, all typenames ending with "_t" are actually reserved by Posix[1] and thus arguably should be avoided for user code.
I'm with Linus here, typedefs in C make the code contex sensitive to parse not only for compilers but for humans too. Syntax highlighting will make `struct gadget * gadget` highlighted correctly but `gadget_t * gadget` doesn't get highlit in a typical text editor (without some kind of smart, context aware highlighting).
No, that doesn't catch it because "float_t" is properly defined as a 'float' by including <math.h>, instead of a 'double' in the local header. But I thought you might be on to something with adding extra warnings, so I tried it out on http://gcc.godbolt.org.
Nope, no warnings at all for spectral_contrast.c with GCC, ICC, or Clang even with "-Wall -Wextra -pedantic". Then I thought to try it on MSVC at http://webcompiler.cloudapp.net/. To my surprise, it caught it pretty clearly with /W4:
main.cpp(11): warning C4244: '/=': conversion from 'double'
to 'float_t', possible loss of data
With that hint, I searched for other GCC warnings and found -Wconversion. Indeed, with that (or -Wfloat-conversion) GCC picks up the scent pretty well: http://goo.gl/9xq3fG
In function 'void normalize(float_t*, int)':
11 : warning: conversion to 'float_t {aka float}' from
'double' may alter its value [-Wfloat-conversion]
ICC gives an excellent error message with -Wconversion also, perhaps the clearest of the bunch: http://goo.gl/cjXLjq
warning #2259: non-pointer conversion from "double" to
"float_t={float}" may lose significant bits
for(i = 0; i < length; i++) v[i] /= magnitude;
Clang remained silent with all the options I tried, but perhaps I missed the right one.
I tried to compile it, and I also couldn't find any other flag, than -Wconversion, that would detect this:
for(i = 0; i < length; i++) sum += a[i] * b[i];
^
I managed to hide the error if return values, which are double, are replaced with float_t. I believe in that case the bug stays in, but -Wconversion doesn't detect it.
Of course the warning can always be silenced by doing something like:
for(i = 0; i < length; i++) sum += ( double )( a[i] * b[i] );
and adding a misleading comment about the explicit cast.
The interesting thing about changing return types is that you can alter the return type of spectral_contrast to float_t without affecting behavior, since float is promoted to double for argument passing (it's passed and returned in the xmm registers).
Thanks for mentioning that option. I hadn't known about it.
Clang does give a warning with -Weverything, but I don't understand what it means: http://goo.gl/M9qjmx
13 : warning: no previous prototype for function spectral_contrast' [-Wmissing-prototypes]
double spectral_contrast(float_t *a, float_t *b, int length) {
It gives the same warning if I change all the float_t's to float, or if I change all the float_t's to double, so I think it's not actually useful or relevant.
It's effectively catching the fact that `spectral_contrast.c` does not `#include "match.h"` — this is indeed directly a part of the underhandedness.
The warning occurs because the function is not static — and therefore callable by other modules. Since it's missing a forward definition (a previous prototype), those modules must blindly declare the prototypes themselves… and their prototypes can get out of sync with the actual definition. That's what's happening here. `match.h` has the forward prototype, but it's very subtly different from the definition. Were match.h included, there'd be a much more glaring warning (or maybe even error).
Reading through the source made me look twice though, as I am used to writing C++ so seeing variables defined but not initialised, non-const array starting points as parameters with a separate length parameter (instead of just a reference to a container or a const reference to enforce read-only nature), memcpy instead of copy constructors or copy assignment operators.
Basically, it made me realise how impossible I would find it to write good C!
Really clever entry though, very impressive. I spotted all the issues I mentioned above, looking for mistakes in them and completely missing the actual problem.
Beautiful. I'd just looked up float_t, and realized that typedef'ing it to double was almost certainly a 'tell', but I still failed to spot the failure to include the header.
I presumed it was simply going to try to slip in a 'float' for a 'float_t' and hope it wasn't noticed, although it seemed unlikely to be a winning strategy.
One other thing that seemed odd was the continued use 'double' after the typedef. I wonder if there's a way to make it work where all the usages are replaced by 'float_t'.
> I wonder if there's a way to make it work where all the usages are replaced by 'float_t'.
Yes, in fact you don't have to change anything else. The trick is that on x86-64 float is promoted to double for argument passing, so the mismatch in the return type on spectral_contrast has no effect.
Just FYI his name is Linus (just like Linus Torvalds). It's a common name for Swedes (Torvalds in from the Swedish minority in Finland).
The Linux name is a play on Linus' name.
The Å in Åkesson can be typed on a US keyboard by hitting alt+a. To make ä & ö you'd hit `alt+u a` and `alt+u o`. To make a ø, which is the Norwegian/Danish way to say ö, you type `alt-o`.
EDIT: the keyboard things are on a US layout on a Mac. For a standard keyboard it might be the same, or not. I don't know...
At least in Swedish they are called "Finlandssvenskar", which means Finland-swedes. That is why I count them as Swedes just like the Swedes living in Sweden, Norway, Denmark or anywhere else.
If you're on a Mac and need to write an unfamiliar accent, then by far the easiest is to just hold down the keyboard key of the base letter (or in this case shift + 'a' since its a capital letter) and then select the appropriate accent from the list - just like you can on iOS.
You can disable it - I've got it disabled on mine. I don't remember how, though. There are no obvious options in the System Preferences. I wonder if it's buried somewhere in Karabiner (formerly KeyRemap4Macbook).
