Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190724234500.253358-1-nums@google.com> <20190807203812.GA20129@kernel.org>
In-Reply-To: <20190807203812.GA20129@kernel.org>
From:   Ian Rogers <irogers@google.com>
Date:   Thu, 22 Aug 2019 14:29:03 -0700
Message-ID: <CAP-5=fURDxS6DSjc34MNOgqb_UX4YKHT5uHHtzi0X9qVQ8ZqcQ@mail.gmail.com>
Subject: Re: [PATCH 0/3] Perf uninitialized value fixes
To:     Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
Cc:     Numfor Mbiziwo-Tiapo <nums@google.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Jiri Olsa <jolsa@redhat.com>,
        Namhyung Kim <namhyung@kernel.org>,
        Song Liu <songliubraving@fb.com>, mbd@fb.com,
        LKML <linux-kernel@vger.kernel.org>,
        Stephane Eranian <eranian@google.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Aug 7, 2019 at 1:38 PM Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
> Em Wed, Jul 24, 2019 at 04:44:57PM -0700, Numfor Mbiziwo-Tiapo escreveu:
> > These patches are all warnings that the MSAN (Memory Sanitizer) build
> > of perf has caught.
> >
> > To build perf with MSAN enabled run:
> > make -C tools/perf CLANG=1 CC=clang EXTRA_CFLAGS="-fsanitize=memory\
> >  -fsanitize-memory-track-origins"
> >
> > (The -fsanitizer-memory-track-origins makes the bugs clearer but
> > isn't strictly necessary.)
> >
> > (Additionally, llvm might have to be installed and clang might have to
> > be specified as the compiler - export CC=/usr/bin/clang).
> >
> > The patches "Fix util.c use of uninitialized value warning" and "Fix
> > annotate.c use of uninitialized value error" build on top of each other
> > (the changes in Fix util.c use of uninitialized value warning must be
> > made first).
> >
> > When running the commands provided in the repro instructions, MSAN will
> > generate false positive uninitialized memory errors. This is happening
> > because libc is not MSAN-instrumented. Finding a way to build libc with
> > MSAN will get rid of these false positives and allow the real warnings
> > mentioned in the patches to be shown.
>
> So this is because I'm not running a glibc linked with MSAN? Do you have
> any pointer to help building glibc with MSAN? I want to do that inside a
> container so that I can use these sanitizers, thanks,
>
> [root@quaco ~]# perf record -o - ls / | perf --no-pager annotate -i -  --stdio
> ==29732==WARNING: MemorySanitizer: use-of-uninitialized-value
> ==29733==WARNING: MemorySanitizer: use-of-uninitialized-value
>     #0 0xcc136d in add_path /home/acme/git/perf/tools/lib/subcmd/exec-cmd.c:130:6
>     #1 0xcc075e in setup_path /home/acme/git/perf/tools/lib/subcmd/exec-cmd.c:146:2
>     #2 0x71298d in main /home/acme/git/perf/tools/perf/perf.c:512:2
>     #0 0xcc136d in add_path /home/acme/git/perf/tools/lib/subcmd/exec-cmd.c:130:6
>     #1 0xcc075e in setup_path /home/acme/git/perf/tools/lib/subcmd/exec-cmd.c:146:2
>     #2 0x71298d in main /home/acme/git/perf/tools/perf/perf.c:512:2
>     #3 0x7f45b9e29f32 in __libc_start_main (/lib64/libc.so.6+0x23f32)
>     #4 0x447dcd in _start (/home/acme/bin/perf+0x447dcd)
>
>   Uninitialized value was created by a heap allocation
>     #3 0x7fd6433cff32 in __libc_start_main (/lib64/libc.so.6+0x23f32)
>     #4 0x447dcd in _start (/home/acme/bin/perf+0x447dcd)
>
>   Uninitialized value was created by a heap allocation
>     #0 0x4507d2 in malloc /home/acme/git/llvm/projects/compiler-rt/lib/msan/msan_interceptors.cc:916:3
>     #1 0x7f45b9e7fc47 in __vasprintf_internal (/lib64/libc.so.6+0x79c47)
>
> SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/acme/git/perf/tools/lib/subcmd/exec-cmd.c:130:6 in add_path
> Exiting
>     #0 0x4507d2 in malloc /home/acme/git/llvm/projects/compiler-rt/lib/msan/msan_interceptors.cc:916:3
>     #1 0x7fd643425c47 in __vasprintf_internal (/lib64/libc.so.6+0x79c47)
>
> SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/acme/git/perf/tools/lib/subcmd/exec-cmd.c:130:6 in add_path
> Exiting
> [root@quaco ~]#
>
> > Numfor Mbiziwo-Tiapo (3):
> >   Fix util.c use of uninitialized value warning
> >   Fix annotate.c use of uninitialized value error
> >   Fix sched-messaging.c use of uninitialized value errors
> >
> >  tools/perf/bench/sched-messaging.c |  3 ++-
> >  tools/perf/util/annotate.c         | 15 +++++++++++----
> >  tools/perf/util/header.c           |  2 +-
> >  3 files changed, 14 insertions(+), 6 deletions(-)

Thanks Arnaldo! Debugging the issue it isn't down glibc, there are
interceptors in the sanitizers for asprintf to recognize it as a
source of memory allocation. The problem is the sanitizers don't
support _FORTIFY_SOURCE [1] and this is causing the false positives.
The following patch works to resolve the false-positive issue for me:

-----
--- a/tools/lib/subcmd/Makefile
+++ b/tools/lib/subcmd/Makefile
@@ -20,7 +20,13 @@ MAKEFLAGS += --no-print-directory
LIBFILE = $(OUTPUT)libsubcmd.a

CFLAGS := $(EXTRA_WARNINGS) $(EXTRA_CFLAGS)
-CFLAGS += -ggdb3 -Wall -Wextra -std=gnu99 -U_FORTIFY_SOURCE
-D_FORTIFY_SOURCE=2 -fPIC
+CFLAGS += -ggdb3 -Wall -Wextra -std=gnu99 -fPIC
+
+ifeq ($(DEBUG),0)
+  ifeq ($(feature-fortify-source), 1)
+    CFLAGS += -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2
+  endif
+endif

ifeq ($(CC_NO_CLANG), 0)
  CFLAGS += -O3
-----

Thanks,
Ian

[1] https://github.com/google/sanitizers/wiki/AddressSanitizer#faq

> > --
> > 2.22.0.657.g960e92d24f-goog
>
> --
>
> - Arnaldo