[PATCH] D56713: gn build: Add check-hwasan target.

Peter Collingbourne via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 15 14:39:54 PST 2019


pcc marked an inline comment as done.
pcc added inline comments.


================
Comment at: llvm/utils/gn/secondary/compiler-rt/test/hwasan/BUILD.gn:47
+    } else {
+      deps += [ "//llvm/tools/llvm-symbolizer($host_toolchain)" ]
+    }
----------------
thakis wrote:
> Why do you need the android special case here?
Digging into this more I suppose it's more like a special case for running the tests on the host (I'll adjust the condition here and add a FIXME to make that clear). The reason why it's necessary is that the sanitizer runtime will expect to find `llvm-symbolizer` in `llvm_tools_dir` (via `$PATH`) when running the tests natively because of this code here:
http://llvm-cs.pcc.me.uk/projects/compiler-rt/test/lit.common.cfg#88
The lit code also expects to be able to find other tools like `lld-link` and `llvm-config` (and I believe the compiler as well) in the same location, so the way things work now it needs to point to the host toolchain (unless we want to do a two-stage bootstrap just to run the sanitizer tests, which seems like something that we could support but most likely controlled by an `is_bootstrap_build` type thing in args.gn).

Either way the host toolchain binary isn't going to work on a non-native architecture because the architecture of `llvm-config` has to match the architecture of the sanitizer. So it seems like we should aim to make native consistent with non-native by always using `//llvm/tools/llvm-symbolizer` here and adjusting lit to use the stage2 `llvm-symbolizer` binary in native builds.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56713/new/

https://reviews.llvm.org/D56713





More information about the llvm-commits mailing list