Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Everybody makes mistakes when writing comparison functions

karpov2007.medium.com

Post reply on HN
Print view View original

51–60 of 63 posts

Re: Everybody makes mistakes when writing comparison functions

#51

Post by gnulinux »

junon wrote: post #47
gnulinux wrote: post #44

Earlier quoted context omitted.

I write code in Agda (a proof assistant) for a living and this is rarely true. Because (1) even if you prove your program is correct as per spec it doesn't prove it's correct according to user/PM since spec can/will be buggy (2) proving every single theorem about your system is an extraordinary time sink, an engineer needs to know what parts are higher risk and needs to be proven and which parts are corrolaries of ba…

Bugs in spec != bugs in code. This is expressly moving the goalposts.

Even if this were true my point (2) still stands.

But it really isn't true because spec is part of the code. If your formalism is wrong, irregardless of whether implementation is correct, your code will behave wrong. There is absolutely no goal post moving. If my program is wrong, I can't tell my user "well, my unittests are all passing and I have 100% code coverage".

Re: Everybody makes mistakes when writing comparison functions

#52

Post by coldpie »

commandlinefan wrote: post #49
coldpie wrote: post #38

Earlier quoted context omitted.

Having unused parameters is super common, especially when using function-pointer-based APIs like qsort. Arguably they should have annotated the arguments to suppress the warnings, but that starts to get into compiler compatibility (OpenSSL is used on a _lot_ of systems with a _lot_ of compilers) and becomes a much larger question than just setting -Wno-unused-parameter.

> especially when using function-pointer-based APIs like qsort Well, the function should be called somewhere , even if it's ignored when null. I can't think of a legitimate case for an unused parameter - it might be #ifdef-ed out in some cases, but it should still be referenced somewhere.

For qsort, sure, but it's pretty common to not need the void* argument for the start_routine you pass to pthread_create, to come up with one example.

Re: Everybody makes mistakes when writing comparison functions

#53

Post by lelanthran »

jahnu wrote: post #35
stinos wrote: post #28

Earlier quoted context omitted.

it's very ver hard to get an in-production project True, especially for older projects which never cared about warnings to begin with. Still when I encounter one I might try it anyway, gradually if possible, to see what comes out. Not just because it is indeed just joy to have no warnings, but also because more often than not some of those warnings actually tell you there are bugs. Or for example (just had this last…

Absolutely. One strategy I employed with a very old code base with a gajillion warnings was to output them to a file and compare against a reference file at the end of the build. If the output changed we would fix those and maybe any that kept happening if you added new compilation units etc. It at least helped us slowly reverse the trend of accumulating warnings.

I did the same, but it meant that the build could not go in parallel.

I also stored the warnings.txt in git so that before any commit I could simply do a git diff to see what warnings were changed by my additions.

Re: Everybody makes mistakes when writing comparison functions

#54

Post by yegle »

I was a bit disappointed when the focus is not related to cryptography.

I.e. a poorly written comparison function compares multiple expressions and the use of && short-circuit the logic, result in a timing attack.

Would love to read an article about this topic, i.e. how to write a function whose execution time is constant regardless of the inputs, and thus not leak any side-channel information to the attacker.

Re: Everybody makes mistakes when writing comparison functions

#55

Post by tialaramex »

marcosdumay wrote: post #40
tialaramex wrote: post #22

Earlier quoted context omitted.

You've got to love that -Wall rusted in place as meaning not, in fact, all warnings, but instead just some arbitrary set of warnings that some people wanted many years ago and now we mustn't change it because too many projects have code that builds under -Wall -Werror only because the warnings that code triggers weren't covered by -Wall many years ago. https://xkcd.com/1172/ in action

GHC has the correct policy, that -Werror is not recommended, and if you use it there are no guarantees your build won't break on the future. Every compiler should adopt it. And every build tool should stop printing thousands of lines of "build tool passed here" on the default verbosity.

Another benefit to Rust editions too.

In principle Rust isn't obliged to let your clearly bad program compile once the realisation is reached that it's broken. If there's a new warning emitted, and you've told the compiler not to allow warnings, well, to bad, fix the warning. However sheer weight of numbers, as with GCC, might beat that position for some future case, we can imagine that if an urgently needed warning breaks 80% of the most popular crates that's not going to be OK.

We get infinite extra tries - Rust editions mean that by definition there is no Rust 2022 code today, and so we can define up front that Rust 2022 code must not have whatever egregious yet widespread problem couldn't be fixed in Rust 2018 code due to the numbers involved.

When people write new code which defaults to Rust 2022 it gets flagged as bad, forcing them to fix it, but their old Rust 2018 code still compiles unless they want to go to the effort to migrate it.

[Yes I know Mara is poised to ship Rust 2021, but this is a hypothetical so I used a future year instead]

Re: Everybody makes mistakes when writing comparison functions

#56

Post by junon »

gnulinux wrote: post #51
junon wrote: post #47

Earlier quoted context omitted.

Bugs in spec != bugs in code. This is expressly moving the goalposts.

Even if this were true my point (2) still stands. But it really isn't true because spec is part of the code. If your formalism is wrong, irregardless of whether implementation is correct, your code will behave wrong. There is absolutely no goal post moving. If my program is wrong, I can't tell my user "well, my unittests are all passing and I have 100% code coverage".

> Even if this were true my point (2) still stands.

And it is completely beside the point of the original argument. I was never arguing against your second point.

Re: Everybody makes mistakes when writing comparison functions

#57

Post by PVS-Studio »

stinos wrote: post #3

Unused function argument is a warning any sane compiler can spit out and turn into a hard error (so this is not really the best example of what PVS studio can do for you), which makes one wonder: why was this not caught earlier? Warnings not enabled, or ignored, and is that something which is problematic wrt something as major as OpenSSL?

So I want to add an explanation. As I see from the comments, many of you thought that the PVS-Studio analyzer warns about a function's unused argument. That's not quite so. Or, to be precise, this is so, but the analyzer processes this case smarter than most linters or compilers.

It's a bad idea to program an analyzer so that it just issues a warning to unused arguments. Such analyzer would produce many false positives, which is why many developers don't look at (or disable) these warnings in their compilers/analyzers.

The PVS-Studio analyzer implements sort of empirical "magic". PVS-Studio relies on the fact that there are arguments of the same type and some of them are not used, while the other ones are used several times. At the same time, there are a number of exceptions to the rule. For example, the diagnostic is not triggered if the number of unused arguments exceeds two.

All this allows the V751 diagnostic to issue few false positives, which makes the tool surpass its competitors. To be exact, when developing PVS-Studio, we do not implement rules if we cannot make them better than those of the compilers - https://pvs-studio.com/en/blog/posts/0802/ . Thanks to the diagnostic I described above, one can find interesting errors - https://pvs-studio.com/en/blog/examples/v751/ .

P.S. The PVS-Studio analyzer also provides a "stupid" version of this diagnostic - V2537 https://pvs-studio.com/en/docs/warnings/v2537/ . It was developed to check code against MISRA C and MISRA C++ standards. But the case above was special and by default this diagnostic was disabled - same as the other ones related to MISRA.

Re: Everybody makes mistakes when writing comparison functions

#58

Post by PVS-Studio »

enlyth wrote: post #2

The author's article about comparison functions linked in the blog post is worth the read Regarding the OpenSSL example presented here, wouldn't any decent IDE catch an unused parameter in a function? Why is a separate static analyzer necessary for this

If a diagnostic issues warnings to unused arguments - it's a weak diagnostic. PVS-Studio acts differently. Take a look at my clarifying comment below or above (not sure where it would be when you read it).

Re: Everybody makes mistakes when writing comparison functions

#59

Post by PVS-Studio »

So I want to add an explanation. As I see from the comments, many of you thought that the PVS-Studio analyzer warns about a function's unused argument. That's not quite so. Or, to be precise, this is so, but the analyzer processes this case smarter than most linters or compilers. Continue: https://karpov2007.medium.com/a-few-more-words-about-the-pvs...

Re: Everybody makes mistakes when writing comparison functions

#60

Post by AndreyKarpov »

brabel wrote: post #48

As someone used to tools like IntelliJ for Java and Kotlin, and Clippy for Rust, this is like the most basic kind of warning I can imagine. It blows my mind sometimes what IntelliJ, in particular, catches... like code branches that can never execute due to intricate conditional blocks in the same block of code... forgetting to use a method argument is not even worth a mention :D Look at just how many issues IntelliJ…

Unfortunately, I don't fully understand your comment. PVS-Studio has a variety of diagnostics. However, such simple errors still exist. By the way, PVS-Studio found errors in IntelliJ IDEA as well :) - https://pvs-studio.com/en/blog/posts/java/0603/
Post reply on HN