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

[enh]

On Thu, Jul 18, 2019 at 11:41 PM Rob Landley <rob at landley.net> wrote:
>
> 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?

`sudo apt-get install clang`?

i've been using this on the _host_, not for Android.

> 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.

i'm used to the usual configure thing of

# Tell configure what tools to use.
target_host=aarch64-linux-android
export AR=$target_host-ar
export AS=$target_host-clang
export CC=$target_host-clang
export CXX=$target_host-clang++
export LD=$target_host-ld
export STRIP=$target_host-strip

https://developer.android.com/ndk/guides/standalone_toolchain#building_open_source_projects_using_standalone_toolchains

if you wanted to make that easier you could have something like

export CC="${CROSS_COMPILE}${CC}"

but what we did in the Android build system was the equivalent of a
CLANG=1 that took care of everything. (and then a NOT_CLANG=1 and then
a hard-wired "you're getting clang".)

> It looks like the meat of the patch is just:
>
> +  CFLAGS="-fsanitize=address -O1 -g -fno-omit-frame-pointer
> -fno-optimize-sibling-calls $CFLAGS"

yep.

> 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.

yeah, Google first started to implemented it in GCC, but gave up and
reimplemented it much better in clang. i don't actually know what the
state of the GCC stuff is beyond "was never as good". i thought they'd
removed it, tbh.

> > 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.

build with CC="clang" and see all the warnings. the current test isn't
working, but my guess as to why was wrong and "worked" by accident.

> > I've also fixed (and modernized) the "are we root?" check in the
> > hostname test,
>
> Was it broken, or just ugly? (Fixed implies broken?)

the _test_ was okay, but the _output_ was wrong and confusing.

> Rob