On Mac, this is the first bad build:

https://ci.chromium.org/buildbot/chromium.clang/ToTMac/2379

Weird. First I built with pinned clang on my Mac, and the test passed. Using Clang built from r347435, as the bot did, my build fails like this:


FAILED: obj/third_party/blink/renderer/bindings/core/v8/bindings_core_impl/image_bitmap_source.o
export DEVELOPER_DIR=/work/chromium/src/build/mac_files/Xcode.app;  ../../../../llvm.combined/build.release/bin/clang++ -MMD -MF obj/third_party/blink/renderer/bindings/core/v8/bindings_core_impl/image_bitmap_source.o.d -DV8_DEPRECATION_WARNINGS -DNO_TCMALLOC -DFULL_SAFE_BROWSING -DSAFE_BROWSING_CSD -DSAFE_BROWSING_DB_LOCAL
-DCHROMIUM_BUILD -DFIELDTRIAL_TESTING_ENABLED -D_LIBCPP_HAS_NO_ALIGNED_ALLOCATION -DCR_XCODE_VERSION=0832 -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D_FORTIFY_SOURCE=2 -DCOMPONENT_BUILD -D__ASSERT_MACROS_DEFINE_VERSIONS_WITHOUT_UNDERSCORE=0 -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DBLINK_CORE_IMPLEMENTATION=1 -DWEBP_EXTERN=extern -DUSE_EGL -DBLINK_IMPLEMENTATION=1 -DINSIDE_BLINK -DU_USING_ICU_NAMESPACE=0 -DU_ENABLE_DYLOAD=0 -DUSE_CHROMIUM_ICU=1 -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE -DUCHAR_TYPE=uint16_t -DGOOGLE_PROTOBUF_NO_RTTI -DGOOGLE_PROTOBUF_NO_STATIC_INITIALIZER -DHAVE_PTHREAD -DPROTOBUF_USE_DLLS -DBORINGSSL_SHARED_LIBRARY -DWEBRTC_NON_STATIC_TRACE_EVENT_HANDLERS=0 -DWEBRTC_CHROMIUM_BUILD -DWEBRTC_POSIX -DWEBRTC_MAC -DABSL_ALLOCATOR_NOTHROW=1 -DNO_MAIN_THREAD_WRAPPING -DSK_HAS_PNG_LIBRARY -DSK_HAS_WEBP_LIBRARY -DSK_HAS_JPEG_LIBRARY -DSKIA_DLL -DGR_GL_IGNORE_ES3_MSAA=0 -DSKCMS_API=__attribute__\(\(visibility\(\"default\"\)\)\) -DSK_SUPPORT_GPU=1 -DSK_GPU_WORKAROUNDS_HEADER=\"gpu/config/gpu_driver_bug_workaround_autogen.h\" -DSK_BUILD_FOR_MAC -DUSING_V8_SHARED -DV8_DEPRECATION_WARNINGS -DSUPPORT_WEBGL2_COMPUTE_CONTEXT=1 -DUSE_LIBJPEG_TURBO=1 -DUSING_V8_SHARED -DV8_DEPRECATION_WARNINGS -DLIBXSLT_STATIC -I../.. -Igen -I../../third_party/libyuv/include -I../../third_party/libwebp/src -I../../third_party/khronos -I../../gpu -I../../third_party/ced/src -I../../third_party/icu/source/common -I../../third_party/icu/source/i18n -I../../third_party/protobuf/src -I../../third_party/protobuf/src -Igen/protoc_out -I../../third_party/boringssl/src/include -I../../third_party/webrtc_overrides -I../../third_party/webrtc -I../../third_party/abseil-cpp -I../../skia/config -I../../skia/ext -I../../third_party/skia/include/c -I../../third_party/skia/include/config -I../../third_party/skia/include/core -I../../third_party/skia/include/docs -I../../third_party/skia/include/effects -I../../third_party/skia/include/encode -I../../third_party/skia/include/gpu -I../../third_party/skia/include/pathops -I../../third_party/skia/include/ports -I../../third_party/skia/include/utils -I../../third_party/skia/include/codec -I../../third_party/skia/src/gpu -I../../third_party/skia/src/sksl -I../../third_party/skia/modules/skottie/include -I../../third_party/angle/include -I../../third_party/angle/src/common/third_party/base -Igen/angle -I../../v8/include -Igen/v8/include -I../../third_party/libjpeg_turbo -I../../third_party/iccjpeg -I../../third_party/libpng -I../../third_party/zlib -I../../third_party/ots/include -I../../v8/include -Igen/v8/include -I../../third_party/libxml/src/include -I../../third_party/libxml/mac/include -I../../third_party/libxslt/src -I../../third_party/snappy/src -I../../third_party/snappy/mac -fno-strict-aliasing -fstack-protector -Wno-builtin-macro-redefined -D__DATE__= -D__TIME__= -D__TIMESTAMP__= -fcolor-diagnostics -fmerge-all-constants -Xclang -mllvm -Xclang -instcombine-lower-dbg-declare=0 -no-canonical-prefixes -arch x86_64 -Wall -Werror -Wextra -Wimplicit-fallthrough -Wthread-safety -Wunguarded-availability -Wno-missing-field-initializers -Wno-unused-parameter -Wno-c++11-narrowing -Wno-unneeded-internal-declaration -Wno-undefined-var-template -Wno-null-pointer-arithmetic -Wno-ignored-pragma-optimize -O2 -fno-omit-frame-pointer -isysroot ../../build/mac_files/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk -mmacosx-version-min=10.9.0 -fvisibility=hidden -Wheader-hygiene -Wstring-conversion -Wtautological-overlap-compare -Wexit-time-destructors -Wglobal-constructors -g0 -Wno-shorten-64-to-32 -DLIBXML_STATIC= -std=c++14 -stdlib=libc++ -fno-exceptions -fno-rtti -fvisibility-inlines-hidden -include obj/third_party/blink/renderer/bindings/core/v8/bindings_core_impl/precompile_core.h-cc -c gen/third_party/blink/renderer/bindings/core/v8/image_bitmap_source.cc -o obj/third_party/blink/renderer/bindings/core/v8/bindings_core_impl/image_bitmap_source.o
In file included from gen/third_party/blink/renderer/bindings/core/v8/image_bitmap_source.cc:16:
In file included from gen/third_party/blink/renderer/bindings/core/v8/v8_html_canvas_element.h:20:
In file included from ../../third_party/blink/renderer/core/html/canvas/html_canvas_element.h:54:
In file included from ../../third_party/blink/renderer/platform/graphics/surface_layer_bridge.h:16:
In file included from ../../third_party/blink/public/platform/web_surface_layer_bridge.h:14:
In file included from ../../third_party/blink/public/platform/web_layer_tree_view.h:36:
In file included from ../../cc/trees/layer_tree_host.h:42:
../../cc/trees/proxy.h:18:10: fatal error: 'cc/trees/task_runner_provider.h' file not found
#include "cc/trees/task_runner_provider.h"
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

But the file is right there:

$ ls -l cc/trees/task_runner_provider.h
-rw-r--r--  1 hwennborg  wheel  3265 Mar 16  2018 cc/trees/task_runner_provider.h


Maybe I'm holding it wrong? Or maybe something's really broken..

This part of the compiler invocation looks a little odd:

-include obj/third_party/blink/renderer/bindings/core/v8/bindings_core_impl/precompile_core.h-cc

I can't find any such file in the build dir. Or anywhere else:

$ find /work/chromium/src -name precompile_core.h-cc

Why isn't the compiler complaining then??

And I'm also not sure where the flag is getting added.



Removing that -include flag from the command line makes the compile succeed.

This file does exit though:

./obj/third_party/blink/renderer/bindings/core/v8/bindings_core_impl/precompile_core.h-cc.gch

Does clang implicitly add the .gch ending?

$ rm ./obj/third_party/blink/renderer/bindings/core/v8/bindings_core_impl/precompile_core.h-cc.gch
$ ninja obj/third_party/blink/renderer/bindings/core/v8/bindings_core_impl/image_bitmap_source.o

Ninja re-built the precompile_core.h-cc.gch file, but image_bitmap_source.o still doesn't built, complaining it can't find cc/trees/task_runner_provider.h

Trying to build wit pch disabled, hoping that's orthogonal to the failure on the bots...

> Weird. First I built with pinned clang on my Mac, and the test passed.

That build was with goma though, which doesn't use pch. So the configs aren't completely comparable..


Since building without goma on mac is taking forever, I'll try to repro on Android in parallel.

The -include flag is getting added by gn, here: https://gn.googlesource.com/gn/+/master/tools/gn/ninja_target_command_util.cc#92

The file is supposed to be created when the pch is getting built: https://gn.googlesource.com/gn/+/master/tools/gn/ninja_target_command_util.cc#154

So there should be a rule for creating that file, and targets using it should depend on that rule.

Which target did you ask ninja to build?

> The -include flag is getting added by gn, here: https://gn.googlesource.com/gn/+/master/tools/gn/ninja_target_command_util.cc#92

Interesting, thanks.

> The file is supposed to be created when the pch is getting built: https://gn.googlesource.com/gn/+/master/tools/gn/ninja_target_command_util.cc#154
>
> So there should be a rule for creating that file, and targets using it should depend on that rule.

What threw me off is that the .gch extension wasn't used with the -include file. The .gch file seems to get built correctly.

I was building first the webkit_unit_tests target, and then trying just obj/third_party/blink/renderer/bindings/core/v8/bindings_core_impl/image_bitmap_source.o

The test failure repros nicely on my Mac when I build with:

clang_use_chrome_plugins = false
llvm_force_head_revision = true
clang_base_path="/work/llvm.combined/build.release"
is_component_build = true
is_debug = false
symbol_level = 0
enable_precompiled_headers = false

$ out/release/webkit_unit_tests --gtest_filter=V8ScriptValueSerializerForModulesTest.DecodeCryptoKeyAES

I'll try to bisect.

Bisection finished:

---
7e2c49d17f64af0dc806b81cc0a69fc1fc4aa057 is the first bad commit
commit 7e2c49d17f64af0dc806b81cc0a69fc1fc4aa057
Author: Bill Wendling <isanbard@gmail.com>
Date:   Wed Nov 21 20:44:18 2018 +0000

    Re-Reinstate 347294 with a fix for the failures.

    Don't try to emit a scalar expression for a non-scalar argument to
    __builtin_constant_p().

    Third time's a charm!
---

This issue was also hit internally: http://b/120048742

According to that, the issue should be fixed in Clang r347531. However, our tests are still failing past that, e.g. https://ci.chromium.org/buildbot/chromium.clang/ToTMac/2395

Well, the internal issue was a compile failure. Our failure is scarier.

Diffing the build artifacts. There is a large handful of files that change, but this one looks highly relevant (V8ScriptValueSerializerForModulesTest.DecodeCryptoKeyAES is the failing test):

obj/third_party/blink/renderer/controller/webkit_unit_tests_sources/v8_script_value_serializer_for_modules_test.o


There are assembly differences in blink::(anonymous namespace)::V8ScriptValueSerializerForModulesTest_DecodeCryptoKeyAES_Test::TestBody() but it's a pretty giant function...

Since this is a Clang codegen change, looking at the IR should be more interesting..

Deleted: invocation.txt 4.7 KB

invocation.txt 4.7 KB View Download

Deleted: a.ll.good 4.2 MB

a.ll.good 4.2 MB Download

Deleted: a.ll.bad 4.2 MB

a.ll.bad 4.2 MB Download

The diff is in @_ZN3WTF12VectorFillerILb1EhNS_18PartitionAllocatorEE17UninitializedFillEPhS3_RKh

The source looks like:

template <typename T, typename Allocator>
struct VectorFiller<true, T, Allocator> {
  STATIC_ONLY(VectorFiller);
  static void UninitializedFill(T* dst, T* dst_end, const T& val) {
    static_assert(sizeof(T) == sizeof(char), "size of type should be one");
#if defined(COMPILER_GCC) && defined(_FORTIFY_SOURCE)
    if (!__builtin_constant_p(dst_end - dst) || (!(dst_end - dst)))           <-----------
      memset(dst, val, dst_end - dst);
#else
    memset(dst, val, dst_end - dst);
#endif
  }
};


And Bill's change concerned __builtin_constant_p, so we're getting closer....







$ diff -u /work/a.ll.good /work/a.ll.bad
--- /work/a.ll.good	2018-11-27 13:35:56.646392708 +0100
+++ /work/a.ll.bad	2018-11-27 13:35:56.610393101 +0100
@@ -32400,20 +32400,46 @@
   store i8* %dst, i8** %dst.addr, align 8
   store i8* %dst_end, i8** %dst_end.addr, align 8
   store i8* %val, i8** %val.addr, align 8
-  %0 = load i8*, i8** %dst.addr, align 8
-  %1 = load i8*, i8** %val.addr, align 8
-  %2 = load i8, i8* %1, align 1
-  %conv = zext i8 %2 to i32
-  %3 = trunc i32 %conv to i8
+  %0 = load i8*, i8** %dst_end.addr, align 8
+  %1 = load i8*, i8** %dst.addr, align 8
+  %sub.ptr.lhs.cast = ptrtoint i8* %0 to i64
+  %sub.ptr.rhs.cast = ptrtoint i8* %1 to i64
+  %sub.ptr.sub = sub i64 %sub.ptr.lhs.cast, %sub.ptr.rhs.cast
+  %2 = call i1 @llvm.is.constant.i64(i64 %sub.ptr.sub)
+  %3 = sext i1 %2 to i32
+  %tobool = icmp ne i32 %3, 0
+  br i1 %tobool, label %lor.lhs.false, label %if.then
+
+lor.lhs.false:                                    ; preds = %entry
   %4 = load i8*, i8** %dst_end.addr, align 8
   %5 = load i8*, i8** %dst.addr, align 8
-  %sub.ptr.lhs.cast = ptrtoint i8* %4 to i64
-  %sub.ptr.rhs.cast = ptrtoint i8* %5 to i64
-  %sub.ptr.sub = sub i64 %sub.ptr.lhs.cast, %sub.ptr.rhs.cast
-  call void @llvm.memset.p0i8.i64(i8* align 1 %0, i8 %3, i64 %sub.ptr.sub, i1 false)
+  %sub.ptr.lhs.cast1 = ptrtoint i8* %4 to i64
+  %sub.ptr.rhs.cast2 = ptrtoint i8* %5 to i64
+  %sub.ptr.sub3 = sub i64 %sub.ptr.lhs.cast1, %sub.ptr.rhs.cast2
+  %tobool4 = icmp ne i64 %sub.ptr.sub3, 0
+  br i1 %tobool4, label %if.end, label %if.then
+
+if.then:                                          ; preds = %lor.lhs.false, %entry
+  %6 = load i8*, i8** %dst.addr, align 8
+  %7 = load i8*, i8** %val.addr, align 8
+  %8 = load i8, i8* %7, align 1
+  %conv = zext i8 %8 to i32
+  %9 = trunc i32 %conv to i8
+  %10 = load i8*, i8** %dst_end.addr, align 8
+  %11 = load i8*, i8** %dst.addr, align 8
+  %sub.ptr.lhs.cast5 = ptrtoint i8* %10 to i64
+  %sub.ptr.rhs.cast6 = ptrtoint i8* %11 to i64
+  %sub.ptr.sub7 = sub i64 %sub.ptr.lhs.cast5, %sub.ptr.rhs.cast6
+  call void @llvm.memset.p0i8.i64(i8* align 1 %6, i8 %9, i64 %sub.ptr.sub7, i1 false)
+  br label %if.end
+
+if.end:                                           ; preds = %if.then, %lor.lhs.false
   ret void
 }
 
+; Function Attrs: nounwind readnone
+declare i1 @llvm.is.constant.i64(i64) #9

The IR diff shows the bad version now has an "if" predicated on @llvm.is.constant.i64 to decide whether or not to do the initialization.

The code was added in https://chromium.googlesource.com/chromium/src/+/7728f5076b20a61376696461d39900d141a3e7fd

The idea is to skip initialization if the size is a known constant that's zero. Why isn't that working then...

Forgot to post before, but there is indeed a zero-initialization missing when diffing the asm. It's in __ZN5blink12_GLOBAL__N_161V8ScriptValueSerializerForModulesTest_DecodeCryptoKeyAES_Test8TestBodyEv, but there's a lot of stuff inlined into it now...


Good asm:

callq	0 <__ZN5blink12_GLOBAL__N_161V8ScriptValueSerializerForModulesTest_DecodeCryptoKeyAES_Test8TestBodyEv+0x71e>
X86_64_RELOC_BRANCH	__ZN3WTF18PartitionAllocator15AllocateBackingEmPKc
movq	%rax, %r12

movq	$0, 8(%rax)   <----
movq	$0, (%rax)    <---- This is missing from the code below.

movq	(%r13), %rax
movb	288(%rax), %cl
movl	$16, %edx
shrq	%cl, %rdx
andl	$7, %edx
movl	808(%rax), %ecx
shrl	$4, %ecx
andl	$1, %ecx
leaq	40(%rdx,%rcx), %rcx
movq	1288(%rax,%rcx,8), %rbx
testq	%rbx, %rbx
jne	42 <__ZN5blink12_GLOBAL__N_161V8ScriptValueSerializerForModulesTest_DecodeCryptoKeyAES_Test8TestBodyEv+0x78d>


Bad asm:

callq	0 <__ZN5blink12_GLOBAL__N_161V8ScriptValueSerializerForModulesTest_DecodeCryptoKeyAES_Test8TestBodyEv+0x71e>
X86_64_RELOC_BRANCH	__ZN3WTF18PartitionAllocator15AllocateBackingEmPKc

movq	(%r12), %rdx
movb	288(%rdx), %cl
movl	$16, %esi
shrq	%cl, %rsi

movq	%rax, %r12

andl	$7, %esi
movl	808(%rdx), %eax
shrl	$4, %eax
andl	$1, %eax
leaq	40(%rsi,%rax), %rax
movq	1288(%rdx,%rax,8), %rbx
testq	%rbx, %rbx
jne	42 <__ZN5blink12_GLOBAL__N_161V8ScriptValueSerializerForModulesTest_DecodeCryptoKeyAES_Test8TestBodyEv+0x77e>

The IR looks weird though

  %0 = load i8*, i8** %dst_end.addr, align 8
  %1 = load i8*, i8** %dst.addr, align 8
  %sub.ptr.lhs.cast = ptrtoint i8* %0 to i64
  %sub.ptr.rhs.cast = ptrtoint i8* %1 to i64

  %sub.ptr.sub = sub i64 %sub.ptr.lhs.cast, %sub.ptr.rhs.cast

  %2 = call i1 @llvm.is.constant.i64(i64 %sub.ptr.sub)
  %3 = sext i1 %2 to i32
  %tobool = icmp ne i32 %3, 0
  br i1 %tobool, label %lor.lhs.false, label %if.then


It's comparing the return value of @llvm.is.constant.i64 against zero, but what about checking %sub.ptr.sub for zero?

Reduced repro:

$ cat /tmp/a.cc
#include <stdio.h>

void f(int *a, int *b) {
  if (!__builtin_constant_p(b - a) || (!(b - a))) {
    printf("(b - a) non-const or non-zero!\n");
  } else {
    printf("(b - a) is a constant zero\n");
  }
}

int arr[] = {1, 2, 3};

int main() {
  f(arr, arr + 3);
  return 0;
}

$ build.release/bin/clang.bad -O2 /tmp/a.cc && ./a.out
(b - a) is a constant zero

Wat!


Interestingly, if we change the if-statement to look like this instead:

  if (!__builtin_constant_p(b - a) || (b - a) != 0) {

$ build.release/bin/clang.bad -O2 /tmp/a.cc && ./a.out
(b - a) non-const or non-zero!

Then it works.

Something's not right.

Even shorter:

$ cat /tmp/b.cc
static bool f(int *a, int *b) {
  return !__builtin_constant_p(b - a) || (!(b - a));
}

int arr[] = {1,2,3};

bool g() {
  return f(arr, arr + 3);
}

$ build.release/bin/clang.good -O2 -S -emit-llvm /tmp/b.cc -o -
; ModuleID = '/tmp/b.cc'
source_filename = "/tmp/b.cc"
target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-apple-macosx10.13.0"

@arr = local_unnamed_addr global [3 x i32] [i32 1, i32 2, i32 3], align 4

; Function Attrs: norecurse nounwind readnone ssp uwtable
define zeroext i1 @_Z1gv() local_unnamed_addr #0 {
entry:
  ret i1 true
}

$ build.release/bin/clang.bad -O2 -S -emit-llvm /tmp/b.cc -o -
; ModuleID = '/tmp/b.cc'
source_filename = "/tmp/b.cc"
target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-apple-macosx10.13.0"

@arr = local_unnamed_addr global [3 x i32] [i32 1, i32 2, i32 3], align 4

; Function Attrs: norecurse nounwind readnone ssp uwtable
define zeroext i1 @_Z1gv() local_unnamed_addr #0 {
entry:
  ret i1 false
}

Reverted in r347656. I'll keep the bug open until the bots have cycled.

I'm seeing this (using your example), which does have the check for (!(b - a)). Are you sure you have all of the __builtin_constant_p() patches? Have you tried it at -O1 to see if the issue is in the front-end or during code generation?

; Function Attrs: nounwind uwtable
define dso_local void @f(i32* %a, i32* %b) local_unnamed_addr #0 {
entry:
  %sub.ptr.lhs.cast = ptrtoint i32* %b to i64
  %sub.ptr.rhs.cast = ptrtoint i32* %a to i64
  %sub.ptr.sub = sub i64 %sub.ptr.lhs.cast, %sub.ptr.rhs.cast
  %sub.ptr.div = ashr exact i64 %sub.ptr.sub, 2
  %0 = tail call i1 @llvm.is.constant.i64(i64 %sub.ptr.div)
  %.not = xor i1 %0, true
  %tobool = icmp eq i64 %sub.ptr.sub, 0                    <--- Here
  %or.cond = or i1 %tobool, %.not                          <--- And here
  br i1 %or.cond, label %if.then, label %if.else

Using Clang r347655 (right before my revert):

Using the example from #19:

$ cat /tmp/a.cc
static bool f(int *a, int *b) {
  return !__builtin_constant_p(b - a) || (!(b - a));
}

int arr[] = {1,2,3};

bool g() {
  return f(arr, arr + 3);
}

$ build.release/bin/clang -cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -O2 -o - -x c++ /tmp/a.cc
[...]
define zeroext i1 @_Z1gv() local_unnamed_addr #0 {
entry:
  ret i1 false
}

I don't know at what point things went wrong exactly. Maybe your change triggered some already existing problem?

Wait, am I getting this backwards, and the bug was really in our source code:

template <typename T, typename Allocator>
struct VectorFiller<true, T, Allocator> {
  STATIC_ONLY(VectorFiller);
  static void UninitializedFill(T* dst, T* dst_end, const T& val) {
    static_assert(sizeof(T) == sizeof(char), "size of type should be one");
#if defined(COMPILER_GCC) && defined(_FORTIFY_SOURCE)
    if (!__builtin_constant_p(dst_end - dst) || (!(dst_end - dst)))           <-----------
      memset(dst, val, dst_end - dst);
#else
    memset(dst, val, dst_end - dst);
#endif
  }
};


I think it's supposed to call memset() if the size is non-constant or non-zero. But "(!(dst_end - dst))" doesn't check for non-zero, it's checking for zero!

How did this ever work?

I have a hypothesis for what's going on. *Technically* what clang is doing is correct (the best KIND of correct!). You're passing in two pointers (to the same data structure, otherwise the ghost of Ritchie will haunt you). The instruction combining pass looks at the resulting calculations on those pointers and says, "Oh! I know that these must be constant." This makes the __bcp() call true. Following the same logic, the "!(b - a)" is false because they're not pointing to the same thing.

So...what to do...

GCC prohibits __bcp() on pointers. It may expand that to pointer arithmetic.

However, it makes me wonder why you have the check in the first place, since whether or not it's constant seems to be irrelevant. But anyway...

I'm going to strict-ify the code-gen check so that we don't take into account pointer arithmetic.

Doh! I posted my last comment without looking at Hans' comments. :-(

Sorry about the mess, the Blink code really threw me off.

Patching it here: https://chromium-review.googlesource.com/c/chromium/src/+/1352363

I've re-committed Bill's change in r347756

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/07cc0e51afdd256619fc525f769b36c09dde9962

commit 07cc0e51afdd256619fc525f769b36c09dde9962
Author: Hans Wennborg <hans@chromium.org>
Date: Wed Nov 28 14:31:30 2018

Make sure VectorFiller::UninitializedFill always fills

The code previously looked like this:

  #if defined(COMPILER_GCC) && defined(_FORTIFY_SOURCE)
      if (!__builtin_constant_p(dst_end - dst) || (!(dst_end - dst)))
        memset(dst, val, dst_end - dst);
  #else
      memset(dst, val, dst_end - dst);
  #endif

The #ifdef was added a long time ago, in http://crrev.com/7728f5076b20 with the
motivation:

> There is a warning in memset in glibc that gets triggered through a
> warndecl when the fill-value of memset is a non-zero constant and the
> size is zero. This warning is enabled when building with
> -D_FORTIFY_SOURCE=2. This patch fixes the warning.

The idea was presumably to skip the memset if the size is a constant zero.
However, that's not what the code is doing. It's skipping the memset if the
size is constant *non-zero*.

How did this ever work?

I don't know about GCC, but until recently, Clang would not return true for the
__builtin_constant_p expression, so the memset would always run. However, Clang
gained support for this in r347417, and now webkit_unittests started failing in
builds that use FORTIFY_SOURCE.

The desired code would be:

  if (!__builtin_constant_p(dst_end - dst) || (dst_end - dst != 0))
    memset(dst, val, dst_end - dst);

But since I couldn't reproduce any warning about memset getting called with
zero size, I think it's better to drop the whole thing and simplify the code.

Bug:  908372 
Change-Id: I166b88af27ca8beacba746892d5fde09ecf1c326
Reviewed-on: https://chromium-review.googlesource.com/c/1352363
Commit-Queue: Hans Wennborg <hans@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611686}
[modify] https://crrev.com/07cc0e51afdd256619fc525f769b36c09dde9962/third_party/blink/renderer/platform/wtf/vector.h
[modify] https://crrev.com/07cc0e51afdd256619fc525f769b36c09dde9962/third_party/blink/renderer/platform/wtf/vector_test.cc