4 ms·
No, you would put the assert inside the VOS_memcpy_s() implementation, and you would not make that a library function, but a static inline function, so the comp
by phkamp 7y ago
No, you would put the assert inside the VOS_memcpy_s() implementation, and you would not make that a library function, but a static inline function, so the compiler can see the arguments every time it is called.
As for disabling asserts in the code you ship: That is such a quaint 1980'ies thing to do, and people should have stopped doing it long time ago.
First, asserts are usually free, in the sense that the values checked are already in registers.
Second, any assert the compiler can evaluate is free, because no code will be generated.
Third, asserts can often enable the compiler to produce better and faster code, because it knows more about the values it produces code for.
And this is not theory: Nobody has ever accused Varnish Cache of being slow, despite the fact that approx 10% of all source lines are asserts which cannot be compiled out.
My money is on Huaweis function doing the check with an assert which was compiled out for deliverable code.
Why else would the have the function in the first place ?
The fact that code is generated for the function as shown, indicates that it was not a static inline function, because the compiler would have reduced that to just the regular memcpy call, so it was probably in a library.
So yeah: Industry Best Practice Incompetence.
- Buge 7y ago>As for disabling asserts in the code you ship: That is such a quaint 1980'ies thing to do, and people should have stopped doing it long time ago. Sqlite makes all its asserts noop in release mode[1]. Chrome disables DCHECK() in release mode[2], and it's used many times[3]. RE2 has debug only checking[4]. In Rust debug mode, signed integer overflow panics, in release mode it wraps[5]. Firefox disables MOZ_ASSERT() in release mode[6], and it's used many times[7]. I doubt Huawei is more modern and less 1980s than all these other projects. >My money is on Huaweis function doing the check with an assert which was compiled out for deliverable code. Are you saying it was compiled out because asserts were disabled, or because the compiler could reason about the arguments at every callsite? It seems unlikely to me that the compiler could reason about the arguments at every callsite. [1] https://www.sqlite.org/assert.html https://www.sqlite.org/assert.html [2] https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md#check_dcheck_and-notreached https://chromium.googlesource.com/chromium/src/+/master/styl... [3] https://cs.chromium.org/search/?q=DCHECK%5C(+case:yes&sq=package:chromium&type=cs https://cs.chromium.org/search/?q=DCHECK%5C(+case:yes&sq=pac... [4] https://github.com/google/re2/blob/master/util/logging.h https://github.com/google/re2/blob/master/util/logging.h [5] http://huonw.github.io/blog/2016/04/myths-and-legends-about-integer-overflow-in-rust/ http://huonw.github.io/blog/2016/04/myths-and-legends-about-... [6] https://dxr.mozilla.org/mozilla-central/source/mfbt/Assertions.h#378 https://dxr.mozilla.org/mozilla-central/source/mfbt/Assertio... [7] https://dxr.mozilla.org/mozilla-central/search?q=MOZ_ASSERT(&redirect=true https://dxr.mozilla.org/mozilla-central/search?q=MOZ_ASSERT(...
- phkamp 7y agoYes, the FOSS industry needs to grow up about software quality. It is natural to have very expensive checks in development and test-environments, they help and aid development and QA to no end, and of course they should be disabled in production. But the run-of-the-mill assert which documents integer ranges, pointer non-null-ness, data structure relationships and so on, cost nothing in performance and should be left in production code. And as I said: We developed Varnish Cache to prove this point, and we overwhelmingly have: One bad CVE since 2006, and "only" about 20% of all web-traffic goes through Varnish at some point or other. Re Huawei: The report claims that there where no checks at all, and I'm saying they have not documented evidence for that claim, because the lack of checking code could be either disabled asserts in production code or the compiler reducing it as dead code.