[Toybox] [PATCH] Add ASAN=1 to the build system.

[Rob Landley]

On 7/18/19 9:02 PM, enh via Toybox wrote:
> Just use `ASAN=1 make test_grep` or whatever.
> 
> You'll probably want to set $ASAN_SYMBOLIZER_PATH to point to
> llvm-symbolizer, but Debian makes that annoying by calling the
> symbolizer /usr/bin/llvm-symbolizer-4.0 or whatever, and ASan refuses to
> use it:
> 
>   ==43370==ERROR: External symbolizer path is set to
>   '/usr/bin/llvm-symbolizer-4.0' which isn't a known symbolizer. Please
>   set the path to the llvm-symbolizer binary or other known tool.
> 
> My usual workaround for this is to drop an llvm-symbolizer symlink in
> the current directory, and I'm happy to automate that in the script to
> make it require no knowledge of any of this nonsense, but haven't done
> so in this initial patch.

I've never gotten Debian's llvm to work, how do I use this with the Android NDK?

The first thing I do with each ndk is add an "llvm-cc" symlink to clang, so it
works as a normally prefixed compiler. (CROSS_COMPILE=/path/to/llvm-)

First thing this does:

+if [ ! -z "$ASAN" ]; then
+  # ASan isn't hard to set up, but it's a lot of little things...
+  # First, it's clang-only.
+  CC="clang"

Is break that. It looks like the meat of the patch is just:

+  CFLAGS="-fsanitize=address -O1 -g -fno-omit-frame-pointer
-fno-optimize-sibling-calls $CFLAGS"

Strangely enough, running gcc with -fsanitize=address did _not_ die with an
error like I expected it to? Oh well, I've stopped trying to understand the gcc
developers. "Doesn't work with gcc" is fine for me, any more than enabling
libmudflap or something on a compiler that hasn't got it won't work.

> This patch also fixes the -Wstring-plus-int check, because ASan requires
> clang, and clang defaults to having that warning on, so things get quite
> noisy.

Is there a -WIamnotaC++developerIknowhowCworks ?

Seriously, let C++ developers take over writing a C compiler and before long
it's warning about doing POINTER ARITHMETIC. (Evil! Beware!) If they want
-Wtraining-wheels they should have an option for it, _not_ on by default.

As for the change itself... what did you fix?

-  [ -z "$(probecc -Wno-string-plus-int <<< \#warn warn 2>&1 | grep
string-plus-int)" ] &&
+  [ -z "$(probecc -Werror -Wno-string-plus-int <<< \#warn warn 2>&1 | grep
string-plus-int)" ] &&

All you did was add -Werror to a test we're not checking the return code of?

The test is -z so it's checking for an empty string. In the $() substitution we
redirect stderr into the pipe and grep for "string-plus-int" meaning "did the
compiler echo back to us the command line option it didn't understand", and when
it DIDN'T the string is empty so -z triggers and we && echo the flag into the file.

How does -Werror "fix" this?

Ah, I see what you did: probecc uses $1 (not "$@") so it tests its _first_
argument and ignores the rest. You put -Werror in the first argument so it never
sees -Wno-string-plus-int, and thus the string should always be zero and the
test is hardwired to succeed on any compiler.

What did you _mean_ to do? Why was it failing for you before? I'm confused.

> I've also fixed (and modernized) the "are we root?" check in the
> hostname test,

Was it broken, or just ugly? (Fixed implies broken?)

Rob