The problem isn't that the file is not there; the problem is that the blacklist got moved, and some versions of the compiler (such as goma) are looking for it in the wrong place; furthermore, when the file is 'implicit', if the compiler can't find it, it continues as if everything is OK.

We need to explicitly pass this file to llvm in Chrome builds, when CFI is enabled, both to make sure llvm is looking for the file in the right place, and to cause llvm to fail if the file can't be found.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/847d298a7a10cb9353e9db9858edb5aa43f2efe8

commit 847d298a7a10cb9353e9db9858edb5aa43f2efe8
Author: Caroline Tice <cmtice@google.com>
Date: Mon May 07 06:51:20 2018

Add -fsanitize-blacklist when CFI is enabled.

In order to work properly CFI needs to use a particular
blacklist.  LLVM used to find the blacklist implicitly, but
the location changed recently and goma builds are looking for
the blacklist in the wrong place.  With this CL, we explicitly
tell LLVM where the blacklist file is.

BUG= chromium:838428 
TEST=Launched two tryjobs on terra-release-tryjob, enabling CFI
on both, but only passing this CL to one of them.  The job with
this CL generated a working chromiumos image; the one without
this CL generated a broken chromiumos image.

Change-Id: I5abaae3cc0c6d32dfa64f517a01fd8ed5f3ed28d
Reviewed-on: https://chromium-review.googlesource.com/1036923
Commit-Ready: Caroline Tice <cmtice@chromium.org>
Tested-by: Caroline Tice <cmtice@chromium.org>
Reviewed-by: Manoj Gupta <manojgupta@chromium.org>

[modify] https://crrev.com/847d298a7a10cb9353e9db9858edb5aa43f2efe8/chromeos-base/chromeos-chrome/chromeos-chrome-9999.ebuild

goma uses our compiler, and our compiler should be able to find it at the correct location automatically, so i don't follow that logic

I don't understand all the ins and outs of how goma works, so I can't explain exactly why they have their own code for finding the blacklists, but they do.  It's here: https://chromium.googlesource.com/infra/goma/client/+/master/client/compile_task.cc#3867

and it does not (or did not -- I think they just changed it) search the 'share' subdirectory, which is where  LLVM moved the blacklist file. See https://buganizer.corp.google.com/issues/79147224

good to see the goma bug is fixed now.  so do we still need to explicitly specify the path ?  can we revert the Chrome change ?

We don't *have* to explicitly specify the path now, but we would prefer to keep this, as it makes things more clear (hidden/implicit flags are really not nice).

sounds to me like you're arguing against having a compiler wrapper at all because all that thing does is stuff lots of random hidden/implicit flags

the problem with the ebuild/flag now is that you're making assumptions about internal llvm/clang filesystem layouts which end projects have no business knowing in the first place.  chrome is going to break if they decide to change the layout such that the location isn't at ${resource_dir}/share/cfi_blacklist.  i don't think it's an improvement and the CL should be reverted.

compiler wrapper has nothing to do in this discussion. 

hidden "options" can be problematic for things like goma and ccache. 
It seems that goma does fully recognize this file and its location now so it does not need an explicit argument. So, it should know to invalidate cache if file contents changed. I doubt ccache knows this. 
Caroline changed the compiler to fail if it does not find the file. So, if compoiler developers decide to change the location, we should easily see a problem in the next version compiler testing. 

the wrapper is relevant in so much that the arguments of "implicit/hidden flags are bad" are exactly what the compiler wrapper does.

ccache doesn't handle the blacklist at all, and this flag/path doesn't change that in any way.  if llvm/clang changes the content of the blacklist, ccache doesn't invalidate the cache.  same for goma.

all the flag does is hardcode internal filesystem paths of another package.  it isn't accomplishing anything useful.  it is on the other hand exposing unrelated internal details of the compiler in the chrome ebuild.

the path to the blacklist itself is only relevant when the flag to turn on cfi is present, and we already have a dedicated flag for that.

chatted with Caroline.

She brings up the point that if you need change this file we are forced to "roll" (get a new ebuild version) which will make goma think there is a new compiler and avoid the problem of possible false positives in the cache. 

We can remove this from the ebuild but we prefer to leave it as is until after branch point (wait until things stabilize with Goma and have this as protection for the branch).

here's a concrete reason to drop it: it breaks simple chrome.  see  issue 838428  comment 5.

Actually I believe the Simple Chrome issue was fixed by a recent GOMA upgrade...

goma isn't required to build simple chrome locally ...

There is not enough information in the comment  issue 838428  to tell if goma was involved in the Simple Chrome workflow that broke or not.  I do know that often GOMA is used with the Simple Chrome workflow (the Chrome OS Simple Chrome workflow uses GOMA by default).  I also know that late last week the GOMA Simple Chrome Workflow was breaking on this because although GOMA had updated their source code, the version being used in the builders had not yet gotten the change.  I also know that goma upgraded its version in the builders, and that over the weekend this Simple Chrome workflow error that I observed at the end of last week went away.  I have asked oshima@ to confirm that they are no longer seeing this issue.  If they are still seeing it, I will reconsider reverting this change immediately.

I'm still seeing this with tot Chromium (git+gclient sync'ed a few minutes ago @r558506.)

yusukes@yusukes2:/ssd2/chromium/src$ rm -rf out_caroline
yusukes@yusukes2:/ssd2/chromium/src$ export BOARD=caroline; cros chrome-sdk --board=$BOARD --log-level=info --internal 
(sdk caroline R68-10677.0.0) yusukes@yusukes2 /ssd2/chromium/src $ autoninja -C out_${SDK_BOARD}/Release chrome chrome_sandbox nacl_helper
...
clang++-7: error: no such file or directory: '/usr/lib64/clang/7.0.0/share/cfi_blacklist.txt'

yusukes@ after you did git+gclient did you log out from all the cros chrome-sdk shells? once you re-enter the cros chrome-sdk toolchain will be updated.

#20
I did git+gclient sync first, then entered the shell (but build still failed.)

 Issue 842948  has been merged into this issue.

ok, I think I have workaround 
in your gn_args, please set use_cfi=false (it should appear as use_cfi=true now, just override that)

I'm seeing this while trying to build for caroline using Simple Chrome. "gn args out_${SDK_BOARD}/Release" shows "is_cfi = false", but I still get errors when building (with goma):

/usr/local/google/home/derat/chrome/.cros_cache/common/goma+2/gomacc x86_64-cros-linux-gnu-clang -B/usr/local/google/home/derat/chrome/.cros_cache/chrome-sdk/tarballs/caroline+10677.0.0+target_toolchain/usr/x86_64-pc-linux-gnu/x86_64-cros-linux-gnu/binutils-bin/2.27.0-gold -Wno-unknown-warning-option -MMD -MF obj/native_client/src/trusted/service_runtime/sel/sel_memory.o.d -DNACL_X86_64_ZERO_BASED_SANDBOX=0 -DV8_DEPRECATION_WARNINGS -DUSE_UDEV -DUSE_AURA=1 -DUSE_NSS_CERTS=1 -DUSE_OZONE=1 -DFULL_SAFE_BROWSING -DSAFE_BROWSING_CSD -DSAFE_BROWSING_DB_LOCAL -DCHROMIUM_BUILD -DFIELDTRIAL_TESTING_ENABLED -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -DCR_CLANG_REVISION=\"331747-1\" -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -DOS_CHROMEOS -DCR_SYSROOT_HASH=4e7db513b0faeea8fb410f70c9909e8736f5c0ab -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -D_POSIX_C_SOURCE=199506 -D_XOPEN_SOURCE=600 -D_GNU_SOURCE=1 -D__STDC_LIMIT_MACROS=1 -I../.. -Igen -fno-strict-aliasing -Wno-builtin-macro-redefined -D__DATE__= -D__TIME__= -D__TIMESTAMP__= -funwind-tables -fPIC -pipe -B../../third_party/binutils/Linux_x64/Release/bin -pthread -fcolor-diagnostics -fmerge-all-constants -no-canonical-prefixes -m64 -march=x86-64 -Wall -Wextra -Wimplicit-fallthrough -Wthread-safety -Wno-missing-field-initializers -Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default -Wno-unneeded-internal-declaration -Wno-undefined-var-template -Wno-nonportable-include-path -Wno-address-of-packed-member -Wno-unused-lambda-capture -Wno-user-defined-warnings -Wno-enum-compare-switch -Wno-null-pointer-arithmetic -Wno-ignored-pragma-optimize -O2 -fno-ident -fdata-sections -ffunction-sections -fno-omit-frame-pointer -g2 -gsplit-dwarf -ggnu-pubnames -fvisibility=hidden -Wheader-hygiene -Wstring-conversion -Wtautological-overlap-compare -std=gnu11 --sysroot=../../../.cros_cache/chrome-sdk/tarballs/caroline+10677.0.0+sysroot_chromeos-base_chromeos-chrome.tar.xz -pipe -pipe -march=corei7 -fno-split-dwarf-inlining -fdebug-info-for-profiling -fsanitize-blacklist=/usr/lib64/clang/7.0.0/share/cfi_blacklist.txt -Wno-unknown-warning-option -Wno-inline-asm -c ../../native_client/src/trusted/service_runtime/posix/sel_memory.c -o obj/native_client/src/trusted/service_runtime/sel/sel_memory.o                                                                                                                                                         
clang-7: error: no such file or directory: '/usr/lib64/clang/7.0.0/share/cfi_blacklist.txt'                                                                                                                                                                                     
[458/42951] CC obj/native_client/src/trusted/service_runtime/sel/nacl_bootstrap_args.o                                                                                                                                                                                          
FAILED: obj/native_client/src/trusted/service_runtime/sel/nacl_bootstrap_args.o

sorry, I think you need to set 2 things to false:
is_cfi=false
use_cfi_cast=false

Both is_cfi and use_cfi_cast appear to be set to false by default by Simple Chrome, but it looks like the path is hardcoded in two other args:

cros_target_extra_cflags = "-pipe -pipe -march=corei7 -fno-split-dwarf-inlining -fdebug-info-for-profiling -fsanitize-blacklist=/usr/lib64/clang/7.0.0/share/cfi_blacklist.txt -Wno-unknown-warning-option -Wno-inline-asm"

cros_target_extra_cxxflags = "-pipe -pipe -pipe -march=corei7 -fno-split-dwarf-inlining -fdebug-info-for-profiling -fsanitize-blacklist=/usr/lib64/clang/7.0.0/share/cfi_blacklist.txt -D__google_stl_debug_vector=1 -Wno-unknown-warning-option -stdlib=libc++ -Wno-inline-asm"

After deleting -fsanitize-blacklist=/usr/lib64/clang/7.0.0/share/cfi_blacklist.txt from both of those, I appear to be able to build for caroline.

thanks for trying derat@

I chumped the revert. This should fix the problem. 

---
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/eb01f9d9ab140a8bcf76d9bb0a87f34c9bb88326

commit eb01f9d9ab140a8bcf76d9bb0a87f34c9bb88326
Author: Caroline Tice <cmtice@google.com>
Date: Tue May 15 06:44:18 2018

Revert "Add -fsanitize-blacklist when CFI is enabled."

This reverts commit 847d298a7a10cb9353e9db9858edb5aa43f2efe8.

Reason for revert: This change breaks the simple chrome workflow. We believe the reason is that the path is installed in a different place in simple chrome and the ebuild workflows.
---

thanks!