[PATCH] D59262: [scudo][standalone] Add string utility functions

[Kostya Kortchinsky via Phabricator via llvm-commits]

cryptoad marked 3 inline comments as done.
cryptoad added inline comments.


================
Comment at: lib/scudo/standalone/string_utils.cc:24
+
+uptr getStringLengthN(const char *S, uptr MaxLen) {
+  uptr I = 0;
----------------
hctim wrote:
> vitalybuka wrote:
> > cryptoad wrote:
> > > vitalybuka wrote:
> > > > do you need internal implementation of these functions? Why can't you use libc?
> > > > I would expect that scudo has no the same restrictions as sanitizer_common
> > > With those, I am slightly worried about localization in the libc, and how strings might end up being processed, with possibly at some point heap allocations.
> > > Looking around it looks like strlen/strnlen/strcmp/strncmp are generally safe (glibc/bionic) but it's not an exhaustive assessment.
> > > strtol appears to be far more complicated.
> > > You likely know better about those intricacies, let me know what you think!
> > I would go with as little as possible of custom code.
> > Add them when you hit an issue?
> Similar to this, is there any particular reason why we're not using the current sanitizer printf commons and basically copying the code? Are we trying to avoid bringing compiler-rt along with us wherever scuda goes? If this is the case, it doesn't look like the printf commons have much that we're not using, and doesn't have any large dependencies on other things in compiler-rt, could we use the commons instead?
The reason for the rewrite as a standalone component is to not depend on sanitizer_common anymore.
One of the reasons being that Fuchsia wants Scudo in their libc, and we can't afford to pull in extraneous code.
Some benefits is that Scudo doesn't have the same requirements as a lot of sanitizer (wrt interception for example) and so we can use some standard function that they can't (like strlen as Vitaly pointed out).


Repository:
  rCRT Compiler Runtime

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59262/new/

https://reviews.llvm.org/D59262