Issue 712789  has been merged into this issue.

Unfortunately, https://codereview.chromium.org/2842063002 seems to introduce a problem with the CrOS sysroot, because clang++ now looks for another clang++ in the current directory and fails with an error message like this:

  clang++: error: unable to execute command: Executable "clang++" doesn't exist!

(Corrected and expanded into a larger message).

$ grep -Rn "clang++" reference.strace
1:execve("/usr/bin/clang++", ["clang++", "-B/usr/bin", "-MMD", "-MF", "host/obj/components/tracing/tool"..., "-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", "-DCR_CLANG_REVISION=\"303910-1\"", "-D_FILE_OFFSET_BITS=64", "-D_LARGEFILE_SOURCE", "-D_LARGEFILE64_SOURCE", "-D__STDC_CONSTANT_MACROS", "-D__STDC_FORMAT_MACROS", "-DNDEBUG", "-DNVALGRIND", "-DDYNAMIC_ANNOTATIONS_ENABLED=0", "-DGOOGLE_PROTOBUF_NO_RTTI", "-DGOOGLE_PROTOBUF_NO_STATIC_INIT"..., "-DHAVE_PTHREAD", "-I../../../../../../../home/rtow"..., "-Ihost/gen", "-I../../../../../../../home/rtow"..., "-fno-strict-aliasing", "--param=ssp-buffer-size=4", ...], [/* 46 vars */]) = 0
273:lstat("/usr/local/bin/clang++", 0x7fff9fc23f00) = -1 ENOENT (No such file or directory)
276:lstat("/usr/bin/clang++", {st_mode=S_IFLNK|0777, st_size=5, ...}) = 0
277:readlink("/usr/bin/clang++", "clang", 4095) = 5
281:stat("/usr/bin/clang++", {st_mode=S_IFREG|0755, st_size=29719664, ...}) = 0
283:access("/usr/local/bin/clang++", R_OK|X_OK) = -1 ENOENT (No such file or directory)
284:access("/usr/bin/clang++", R_OK|X_OK)   = 0
285:stat("/usr/bin/clang++", {st_mode=S_IFREG|0755, st_size=29719664, ...}) = 0
$ grep -Rn "clang++" error.strace
1:execve("/usr/bin/clang++", ["clang++", "-B/usr/bin", "-MMD", "-MF", "host/obj/components/tracing/tool"..., "-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", "-DCR_CLANG_REVISION=\"303910-1\"", "-D_FILE_OFFSET_BITS=64", "-D_LARGEFILE_SOURCE", "-D_LARGEFILE64_SOURCE", "-D__STDC_CONSTANT_MACROS", "-D__STDC_FORMAT_MACROS", "-DNDEBUG", "-DNVALGRIND", "-DDYNAMIC_ANNOTATIONS_ENABLED=0", "-DGOOGLE_PROTOBUF_NO_RTTI", "-DGOOGLE_PROTOBUF_NO_STATIC_INIT"..., "-DHAVE_PTHREAD", "-I../../../../../../../home/rtow"..., "-Ihost/gen", "-I../../../../../../../home/rtow"..., "-fno-strict-aliasing", "--param=ssp-buffer-size=4", ...], [/* 46 vars */]) = 0
271:access("/usr/local/bin/clang++", R_OK|X_OK) = -1 ENOENT (No such file or directory)
272:access("/usr/bin/clang++", R_OK|X_OK)   = 0 <- Searching in PATH
273:stat("/usr/bin/clang++", {st_mode=S_IFREG|0755, st_size=29719664, ...}) = 0
817:access("clang++", F_OK)                 = -1 ENOENT (No such file or directory)
818:write(2, "clang++", 7clang++)                  = 7
824:write(2, "unable to execute command: Execu"..., 62unable to execute command: Executable "clang++" doesn't exist!) = 62

As expected, the first couple of times clang++ appears is when it's searching in the PATH, after that, with -no-canonical-prefixes specified, /usr/bin/clang++ seems to look for a clang++ in the current directory, rather than looking for itself (?).

Above is with the following compiler invocation, inside a CrOS sysroot, at ~/chrome_root/src/c/Release: 

clang++ -B/usr/bin -MMD -MF host/obj/components/tracing/tools/proto_zero_plugin/proto_zero_plugin/proto_zero_generator.o.d -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 -DCR_CLANG_REVISION=\"303910-1\" -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DGOOGLE_PROTOBUF_NO_RTTI -DGOOGLE_PROTOBUF_NO_STATIC_INITIALIZER -DHAVE_PTHREAD -I../../../../../../../home/rtownsend/chrome_root/src -Ihost/gen -I../../../../../../../home/rtownsend/chrome_root/src/third_party/protobuf/src -fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector -Wno-builtin-macro-redefined -D__DATE__= -D__TIME__= -D__TIMESTAMP__= -funwind-tables -fPIC -pipe -fcolor-diagnostics -m64 -march=x86-64 -pthread -Wall -Wextra -Wno-missing-field-initializers -Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default -Wno-unneeded-internal-declaration -Wno-inconsistent-missing-override -Wno-undefined-var-template -Wno-nonportable-include-path -Wno-address-of-packed-member -Wno-unused-lambda-capture -Wno-user-defined-warnings -O2 -fno-ident -fdata-sections -ffunction-sections -fomit-frame-pointer -g2 -gsplit-dwarf -fvisibility=hidden -Wheader-hygiene -Wstring-conversion -Wtautological-overlap-compare -std=gnu++11 -fno-rtti -fno-exceptions -fvisibility-inlines-hidden  -Wno-unknown-warning-option -c ../../../../../../../home/rtownsend/chrome_root/src/components/tracing/tools/proto_zero_plugin/proto_zero_generator.cc -o host/obj/components/tracing/tools/proto_zero_plugin/proto_zero_plugin/proto_zero_generator.o --no-canonical-prefixes

Incidentally, changing the -B flag something non-existing doesn't seem to affect the strace, it's possible that's it's being ignored or is instead used for some other purpose other than deciding which clang tools to call.

Do you understand why this happens with the CrOS sysroot, but not in a regular chrome/linux build? Can you figure out what the relevant difference is between these two setups?

I don't understand in detail the precise difference, but it seems that it's possible to replicate it with a simpler command:

strace -f clang++ -no-canonical-prefixes test.cpp 

This displays the problem inside a cros_sdk chroot, where test.cpp is just an empty `int main() {return 0;}` program.

I'll revert the change for now; let's confirm that the revert fixes the problem ...

+yyanagisawa

How the clang is specified?

All I know about ChromeOS clang is that following patches are applied for them.
https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/master/sys-devel/llvm/files

When I hear about the bug, it reminds me:
https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/master/sys-devel/llvm/files/clang-executable-detection.patch

Let me check...

maybe clang bug?
I put clang bundled with chromium in PATH env and executed followings, then I could see the same error.
$ clang -no-canonical-prefixes -v -c /tmp/empty.cc 
clang version 5.0.0 (trunk 303910)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: chrome_gn/src/third_party/llvm-build/Release+Asserts/bin
Found candidate GCC installation: /usr/lib/gcc/i686-linux-gnu/4.8
Found candidate GCC installation: /usr/lib/gcc/i686-linux-gnu/4.8.4
Found candidate GCC installation: /usr/lib/gcc/i686-linux-gnu/4.9
Found candidate GCC installation: /usr/lib/gcc/i686-linux-gnu/4.9.3
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.4
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.4.7
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.6
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.6.4
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.7
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.7.3
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.8
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.8.4
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.9
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.9.3
Selected GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.8
Candidate multilib: .;@m64
Candidate multilib: 32;@m32
Candidate multilib: x32;@mx32
Selected multilib: .;@m64
 "clang" -cc1 -triple x86_64-unknown-linux-gnu -emit-obj -mrelax-all -disable-free -main-file-name empty.cc -mrelocation-model static -mthread-model posix -mdisable-fp-elim -fmath-errno -masm-verbose -mconstructor-aliases -munwind-tables -fuse-init-array -target-cpu x86-64 -v -dwarf-column-info -debugger-tuning=gdb -coverage-notes-file /usr/local/google/home/yyanagisawa/empty.gcno -resource-dir lib/clang/5.0.0 -internal-isystem /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8 -internal-isystem /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/x86_64-linux-gnu/c++/4.8 -internal-isystem /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/x86_64-linux-gnu/c++/4.8 -internal-isystem /usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/backward -internal-isystem /usr/local/include -internal-isystem lib/clang/5.0.0/include -internal-externc-isystem /usr/include/x86_64-linux-gnu -internal-externc-isystem /include -internal-externc-isystem /usr/include -fdeprecated-macro -fdebug-compilation-dir /usr/local/google/home/yyanagisawa -ferror-limit 19 -fmessage-length 80 -fobjc-runtime=gcc -fcxx-exceptions -fexceptions -fdiagnostics-show-option -fcolor-diagnostics -o empty.o -x c++ /tmp/empty.cc
clang: error: unable to execute command: Executable "clang" doesn't exist!

I hope to listen to advice from who knows clang well, but I guess clang uses non canonical path where it should not do that?
https://github.com/llvm-mirror/clang/blob/master/tools/driver/driver.cpp#L425
and Driver gets wrong clang path.

RE the situation on CrOS/cros_sdk - would it be better to patch it so that this flag is no longer included (but is for all other builds) or would it be better to take a bit more time and try to resolve the underlying problem?

I will take a look to see why it fails in ChromeOS

removing patch in #14 does not help.
clang++  -no-canonical-prefixes a.cc  fails
while
/usr/bin/clang++  -no-canonical-prefixes a.cc  works fine.

hans, do you have time to look at comment 15?

I can disable this option in ChromeOS, do we need to do this now?

I reverted my change, so there's no urgency on this, I don't think.

Looks like this is https://bugs.llvm.org/show_bug.cgi?id=9576
I can take a look at that tomorrow.

For non-ChromeOS we always invoke clang with an explicit path though, like ../../third_party/llvm-build/Release+Asserts/bin/clang++ and not relying on PATH, which avoids this problem. So I guess ChromeOS is doing something different.

we could have the build process pass an absolute path too, but seems like it'd just be papering over the clang bug ?

it'd also make for messier log output, but maybe that's a moot point :)

it looks like this should not be a P1 anymore. So, lowering priority.

> we could have the build process pass an absolute path too, but seems like it'd just be papering over the clang bug ?
> it'd also make for messier log output, but maybe that's a moot point :)

I don't know much about the ChromeOS build process, maybe you scrub PATH before invoking the compiler, but at least in Chrome, we use an explicit path so that there's no doubt about which version of Clang gets invoked.

inside of the CrOS SDK, we fully control everything, so there is no real chance of a diff toolchain getting in the way -- we only have one for the target.  sure, if someone did something dumb and installed stuff into /usr/local, it'd probably blow up, but that is not supported anywhere and you'd have to try pretty hard to screw that up.

in the simple chrome workflow, we mung PATH so that our toolchain is always found first.

Imho it's good if cr and cros builds are as similar as possible, so making CrOS use a qualified path to clang sounds like a good change anyhow (especially if it protects against /use/local shenanigans). So I'd suggest doing that.

I have looked into the implementation. When -no-canonical-prefixes is used, Clang uses argv[0] to get the path and the name of the executable. When clang is in $PATH, this will be just "clang". The error is thrown by https://llvm.googlesource.com/llvm/+/master/lib/Support/Unix/Program.inc#174 which checks whether the executable exists using access(2) and as expected fails because "clang" is an invalid path.

To handle this case, we could need to use Driver::GetProgramPath (https://llvm.googlesource.com/clang/+/master/lib/Driver/Driver.cpp#3654) to resolve the name to a path. I tried that and it fixes the issue. I'll send the patch to upstream and see what others think about this solution.

> To handle this case, we could need to use Driver::GetProgramPath (https://llvm.googlesource.com/clang/+/master/lib/Driver/Driver.cpp#3654) to resolve the name to a path. I tried that and it fixes the issue. I'll send the patch to upstream and see what others think about this solution.

That sounds good. Please cc me on the patch.

@hans, can I reassign this to you to re-land the change in #5 once we have this working?

phosek landed his fix to clang in r305600, which we rolled to a few days ago, so this should be go to go.

Maybe it's easiest if you just reopen and commit https://codereview.chromium.org/2842063002/ ?

ah, okay. I can do that.

Trying again: https://crrev.com/2974863002 .

ChromeOS clang does not have this fix yet. clang upgrade will be done after next branch point, can you wait till then?

Ah. Yes, I can wait.

Can we enable this for all but full-cros builds in the meantime? Only the "simplechrome" bots use a custom toolchain, everything else uses the same compiler.

I've posted a CL to do what thakis@ suggests in https://chromium-review.googlesource.com/c/571440/ .

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

commit 19e9083e33618e4fc3f05dcea7577843dc863d18
Author: Dirk Pranke <dpranke@chromium.org>
Date: Fri Jul 14 17:31:04 2017

Reland of Pass -no-canonical-prefixes to clang by default.

This is a reland of patchset #1 of https://codereview.chromium.org/2936053003,
except that we do not pass the flag when using the ChromeOS custom
toolchains, either.

TBR=hans@chromium.org, manojgupta@chromium.org, thakis@chromium.org
BUG= 712797 

Change-Id: I14f56c9b29234dd693719b017828c6250202a9d0
Reviewed-on: https://chromium-review.googlesource.com/571440
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Dirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486793}
[modify] https://crrev.com/19e9083e33618e4fc3f05dcea7577843dc863d18/build/config/compiler/BUILD.gn

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

commit b1ca34a0edebb9b81f7db3c58b759029a64d7baf
Author: Dirk Pranke <dpranke@chromium.org>
Date: Fri Jul 14 18:38:53 2017

Revert "Reland of Pass -no-canonical-prefixes to clang by default."

This reverts commit 19e9083e33618e4fc3f05dcea7577843dc863d18.

Reason for revert: This might be causing the Linux CFI failures in https://luci-milo.appspot.com/buildbot/chromium.memory/Linux%20CFI/26 and following.

Original change's description:
> Reland of Pass -no-canonical-prefixes to clang by default.
> 
> This is a reland of patchset #1 of https://codereview.chromium.org/2936053003,
> except that we do not pass the flag when using the ChromeOS custom
> toolchains, either.
> 
> TBR=hans@chromium.org, manojgupta@chromium.org, thakis@chromium.org
> BUG= 712797 
> 
> Change-Id: I14f56c9b29234dd693719b017828c6250202a9d0
> Reviewed-on: https://chromium-review.googlesource.com/571440
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Reviewed-by: Nico Weber <thakis@chromium.org>
> Commit-Queue: Dirk Pranke <dpranke@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#486793}

TBR=thakis@chromium.org,hans@chromium.org,dpranke@chromium.org,manojgupta@chromium.org

Change-Id: Iffa28553b78c79fec8daed93658ba755bd17fe1b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  712797 
Reviewed-on: https://chromium-review.googlesource.com/572004
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Dirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486818}
[modify] https://crrev.com/b1ca34a0edebb9b81f7db3c58b759029a64d7baf/build/config/compiler/BUILD.gn

manojgupta@, I'm going to reassign this to you to fix the CrOS build when you're ready. Let me know if you're not comfortable with that?

Isn't the current blocker here that CFI can't handle this flag? 

oh, I suppose that's true.

pcc@, are you a good person to make sure that the CFI builder can deal with this?

As far as I recall, the problem was that goma wasn't sending the default sanitizer blacklist file to goma when -no-canonical-prefixes was specified. This would potentially affect any bot that uses goma + sanitizers, not just the CFI bot. It's probably best if someone on the goma team looks into this.

Assigned to shinyak@ because he implemented the initial support for default blacklists, but feel free to reassign.

Sorry for late response. 
Ah, maybe goma was assuming --resource-dir is absolute. However, with -no-canonical-prefixes, it's not.

I'll fix it soon.

upstream: b/65471322

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/goma/client/+/d3f6c3a07734b475ee14ceddd90a0ead1a666e63

commit d3f6c3a07734b475ee14ceddd90a0ead1a666e63
Author: Shinya Kawanaka <shinyak@google.com>
Date: Mon Sep 11 03:50:48 2017

shinyak@ Is this done? 

Oops, sorry.
From goma version 139, which has been released on 2017-09-18, goma is not assuming --resource-dir is absolute. So I believe we can test -no-canonical-prefixes again.

Move owner back to pcc@.

Maybe dpranke@ can try relanding his patch now.

Done: https://chromium-review.googlesource.com/c/chromium/src/+/701464

Not sure how I keep getting on the hook here ... :).

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

commit 952c0b68c69983473ca3353d413537b15880f261
Author: Dirk Pranke <dpranke@chromium.org>
Date: Fri Oct 06 05:01:51 2017

Pass -no-canonical-prefixes to clang where possible.

This makes builds slightly more position-independent and
should improve goma caching. This is a reland of
r486793 now that the issues have hopefully been addressed.

R=pcc@chromium.org, thakis@chromium.org, manojgupta@chromium.org
BUG= 712797 

Change-Id: Ief5392b2f64b6c4bc5192f65ac2cdda65733fe9b
Reviewed-on: https://chromium-review.googlesource.com/701464
Commit-Queue: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506973}
[modify] https://crrev.com/952c0b68c69983473ca3353d413537b15880f261/build/config/compiler/BUILD.gn

Let's see if this sticks ...