←back to thread

1371 points yett | 2 comments | | HN request time: 0.412s | source
Show context
jandrese ◴[] No.43774091[source]
> Not ignore the compilation warnings – this code most likely threw a warning in the original code that was either ignored or disabled!

What compiler error would you expect here? Maybe not checking the return value from scanf to make sure it matches the number of parameters? Otherwise this seems like a data file error that the compiler would have no clue about.

replies(3): >>43775089 #>>43777497 #>>43778191 #
burch45 ◴[] No.43775089[source]
Undefined behavior to access the uninitialized memory. A sanitizer would have flagged that.
replies(2): >>43775220 #>>43775229 #
jandrese ◴[] No.43775220[source]
The compiler has no way of knowing that the memory would be undefined, not unless it somehow can verify the data file. The most I think it can do is flag the program for not checking the return value of scanf, but even that is unlikely to be true since the program probably was checking for end of file which is also in the return value. It was failing to check the number of matched parameters. This is the kind of error that is easy to miss given the semantics of scanf.
replies(2): >>43775241 #>>43775941 #
nayuki ◴[] No.43775941[source]
> The compiler has no way of knowing that the memory would be undefined

Yes it would. -fsanitize=address does a bunch of instrumentation - it allocates shadow memory to keep track of what main memory is defined, and it checks every read and write address against the shadow memory. It is a combination of compile-time instrumentation and run-time checking. And yes, it is expensive, so it should be used for debugging and not the final release.

https://clang.llvm.org/docs/AddressSanitizer.html , https://learn.microsoft.com/en-us/cpp/sanitizers/asan?view=m...

replies(3): >>43776057 #>>43776483 #>>43776929 #
1. bri3d ◴[] No.43776929[source]
I tried this with clang ASAN. Nothing happens. It won't catch this bug. ASAN detects the presence of incorrect behavior, not the absence of correct behavior.

There's no use-after-free, use-after-return, use-after-scope, or OOB access here. It's a case of "an allocated stack variable is dynamically read without being initialized only in a runtime case," which afaik no standard analyzer will catch.

The best way to identify this would be to require all locals to be initialized as a matter of policy (very unlikely to fly in a games studio, especially back then, due to the perceived performance overhead) or to debug with a form of stack initialization enabled, like "-ftrivial-auto-var-init=pattern" which while it doesn't catch the issue statically, does make it appear pretty quickly in QA (I tested).

replies(1): >>43777042 #
2. nayuki ◴[] No.43777042[source]
Thanks for the investigation. Oops, it seems like MSan (memory sanitizer) is the appropriate tool that detects uninitialized reads? https://stackoverflow.com/questions/68576464/clang-sanitizer...

I only use UBSan and ASan on my own programs because I tend not to make mistakes about initialization. So my knowledge is incomplete with respect to auditing other people's code, which can have different classes of errors than mine.

Thank goodness that every language that is newer than C and C++ doesn't repeat these design mistakes, and doesn't require these awkward sanitizer tools that are introduced decades after the fact.