CFI: download LLVM Gold plugin, if requested by env var or GYP_DEFINES.

tools/clang/scripts/update.sh

Index: tools/clang/scripts/update.sh
diff --git a/tools/clang/scripts/update.sh b/tools/clang/scripts/update.sh
index 36b6e81eae7f3c4457b394dc5ab81b43c96b6e1d..231d4073a0e1dd4fda23c9afc2a9b0e07a869693 100755
--- a/tools/clang/scripts/update.sh
+++ b/tools/clang/scripts/update.sh
@@ -53,6 +53,10 @@ if [[ -z "$LLVM_DOWNLOAD_GOLD_PLUGIN" ]]; then
   LLVM_DOWNLOAD_GOLD_PLUGIN=
 fi
 
+if [[ "$GYP_DEFINES" =~ .*(cfi_vptr|use_lto|use_lto_o2|official_build)=1.* ]]; then

[scottmg, 2015/09/28 22:09:41]

Is "official_build=1" how CrOS does it? On Windows it's `buildtype=Official`.

I've also been burned by this crazy way that compile.py sets official build
https://code.google.com/p/chromium/codesearch#chromium/build/scripts/slave/co...

I don't know if you have to take care of that here or not (probably not since I
guess the bot will set one of the other things you check for above?)

[hans, 2015/09/28 22:11:04]

Seems like a lot of different options :-/ Can you rewrite this so it fits under
80 columns? Maybe a for-loop over them would be cleaner?

Also, should the if-block be Linux-only? I assume we don't want to do this when
official_build=1 on Mac.

[Nico, 2015/10/08 02:27:24]

Yeah, I think you need buildtype=Official (and maybe branding=Chrome)
(https://code.google.com/p/chromium/codesearch#chromium/src/build/linux/sysroo...).

I'd expect that almost nobody uses these other toggles, and those who do know
what to do.

[krasin, 2015/10/09 21:33:51]

Done in https://codereview.chromium.org/1385403006

+  # LLVM Gold plugin is required to build with this configuration

[hans, 2015/09/28 22:11:04]

ultra nit: period at the end of the comment.

[krasin, 2015/10/01 21:07:46]

Done.

+  LLVM_DOWNLOAD_GOLD_PLUGIN=1
+fi
 
 # Die if any command dies, error on undefined variable expansions.
 set -eu
@@ -235,6 +239,11 @@ if [[ -f "${STAMP_FILE}" ]]; then
        [[ "${PREVIOUSLY_BUILT_REVISON}" = \
           "${PACKAGE_VERSION}" ]]; then
     echo "Clang already at ${PACKAGE_VERSION}"
+    if [[ -n "${LLVM_DOWNLOAD_GOLD_PLUGIN}" ]]; then

[hans, 2015/09/28 22:11:04]

The code below (line 285) already downloads the gold plugin if this variable is
set.

It would be better if the code here just checked if the gold plugin is already
downloaded, and if not skips the exit 0 below. That would correspond nicely to
the comment above: "Check if there's anything to be done, exit early if not."

[krasin, 2015/10/09 21:33:51]

Good idea. Done in https://codereview.chromium.org/1385403006

+      # TODO(krasin): make a stamp for gold plugin to avoid
+      # downloading it every time.
+      ${THIS_DIR}/../../../build/download_gold_plugin.py
+    fi
     exit 0
   fi
 fi