Disallow nullptr arguments for {CopyChars}

This allows to remove special casing for the {count == 0} case, which
was needed because {memmove} does not accept {nullptr} arguments even
if the {count} is zero.

R=leszeks@chromium.org

Bug: v8:9396
Change-Id: Iaef3cdbbffa74c2ba1c4e4501dafd943282cbcd9
Cq-Include-Trybots: luci.v8.try:v8_linux64_ubsan_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1807366
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63838}

include/v8config.h

@@ -170,6 +170,7 @@
 //  V8_HAS_ATTRIBUTE_ALWAYS_INLINE      - __attribute__((always_inline))
 //                                        supported
 //  V8_HAS_ATTRIBUTE_DEPRECATED         - __attribute__((deprecated)) supported
+//  V8_HAS_ATTRIBUTE_NONNULL            - __attribute__((nonnull)) supported
 //  V8_HAS_ATTRIBUTE_NOINLINE           - __attribute__((noinline)) supported
 //  V8_HAS_ATTRIBUTE_UNUSED             - __attribute__((unused)) supported
 //  V8_HAS_ATTRIBUTE_VISIBILITY         - __attribute__((visibility)) supported
@@ -210,6 +211,7 @@
 # define V8_HAS_ATTRIBUTE_DEPRECATED (__has_attribute(deprecated))
 # define V8_HAS_ATTRIBUTE_DEPRECATED_MESSAGE \
     (__has_extension(attribute_deprecated_with_message))
+# define V8_HAS_ATTRIBUTE_NONNULL (__has_attribute(nonnull))
 # define V8_HAS_ATTRIBUTE_NOINLINE (__has_attribute(noinline))
 # define V8_HAS_ATTRIBUTE_UNUSED (__has_attribute(unused))
 # define V8_HAS_ATTRIBUTE_VISIBILITY (__has_attribute(visibility))
@@ -309,6 +311,17 @@
 # define V8_ASSUME_ALIGNED(ptr) (ptr)
 #endif
 
+
+// A macro to mark specific arguments as non-null.
+// Use like:
+//   int add(int* x, int y, int* z) V8_NONNULL(1, 3) { return *x + y + *z; }
+#if V8_HAS_ATTRIBUTE_NONNULL
+# define V8_NONNULL(...) __attribute__((nonnull(__VA_ARGS__)))
+#else
+# define V8_NONNULL(...) /* NOT SUPPORTED */
+#endif
+
+
 // A macro used to tell the compiler to never inline a particular function.
 // Don't bother for debug builds.
 // Use like:

src/api/api.cc

@@ -5289,7 +5289,7 @@ static inline int WriteHelper(i::Isolate* isolate, const String* string,
   int end = start + length;
   if ((length == -1) || (length > str->length() - start)) end = str->length();
   if (end < 0) return 0;
-  i::String::WriteToFlat(*str, buffer, start, end);
+  if (start < end) i::String::WriteToFlat(*str, buffer, start, end);
   if (!(options & String::NO_NULL_TERMINATION) &&
       (length == -1 || end - start < length)) {
     buffer[end - start] = '\0';

src/objects/string.cc

@@ -601,9 +601,8 @@ void String::WriteToFlat(String src, sinkchar* sink, int f, int t) {
   String source = src;
   int from = f;
   int to = t;
-  while (true) {
+  while (from < to) {
     DCHECK_LE(0, from);
-    DCHECK_LE(from, to);
     DCHECK_LE(to, source.length());
     switch (StringShape(source).full_representation_tag()) {
       case kOneByteStringTag | kExternalStringTag: {
@@ -681,6 +680,7 @@ void String::WriteToFlat(String src, sinkchar* sink, int f, int t) {
         break;
     }
   }
+  DCHECK_EQ(from, to);
 }
 
 template <typename SourceChar>

src/strings/uri.cc

@@ -195,10 +195,14 @@ MaybeHandle<String> Uri::Decode(Isolate* isolate, Handle<String> uri,
       String);
 
   DisallowHeapAllocation no_gc;
-  CopyChars(result->GetChars(no_gc), one_byte_buffer.data(),
-            one_byte_buffer.size());
-  CopyChars(result->GetChars(no_gc) + one_byte_buffer.size(),
-            two_byte_buffer.data(), two_byte_buffer.size());
+  uc16* chars = result->GetChars(no_gc);
+  if (!one_byte_buffer.empty()) {
+    CopyChars(chars, one_byte_buffer.data(), one_byte_buffer.size());
+    chars += one_byte_buffer.size();
+  }
+  if (!two_byte_buffer.empty()) {
+    CopyChars(chars, two_byte_buffer.data(), two_byte_buffer.size());
+  }
 
   return result;
 }

src/utils/memcopy.h

@@ -200,16 +200,16 @@ inline void MemsetPointer(T** dest, U* value, size_t counter) {
 // Copy from 8bit/16bit chars to 8bit/16bit chars. Values are zero-extended if
 // needed. Ranges are not allowed to overlap unless the type sizes match (hence
 // {memmove} is used internally).
+// The separate declaration is needed for the V8_NONNULL, which is not allowed
+// on a definition.
+template <typename SrcType, typename DstType>
+void CopyChars(DstType* dst, const SrcType* src, size_t count) V8_NONNULL(1, 2);
+
 template <typename SrcType, typename DstType>
 void CopyChars(DstType* dst, const SrcType* src, size_t count) {
   STATIC_ASSERT(std::is_integral<SrcType>::value);
   STATIC_ASSERT(std::is_integral<DstType>::value);
 
-  // This special case for {count == 0} allows to pass {nullptr} as {dst} or
-  // {src} in that case.
-  // TODO(clemensh): Remove this and make {dst} and {src} nonnull.
-  if (count == 0) return;
-
   // If the size of {SrcType} and {DstType} matches, we switch to the more
   // general and potentially faster {memmove}. Note that this decision is made
   // statically at compile time.