From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 3A668A0032; Fri, 29 Oct 2021 11:22:11 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B44B3410E0; Fri, 29 Oct 2021 11:22:10 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id A1B0740688 for ; Fri, 29 Oct 2021 11:22:08 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1635499328; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=KuJ+ErAm24/e6XNNBz4b8p2/gaQuK/+p42ZYur74kQU=; b=BXAdtAeH+vGSi1nZIG5eynMCBh5U91jIdhObMY2h6vDH9xMd1ofGFYGe1ebyQIMux54IUF uSggIGAjIkgMB0ik/jfmwkmi6xrgaAySnPVW9XvEx6RyabKv/wCdX456gFAQnks8sBXuld djwWo0i6wS0RGIu3m+vzJIJPZAE+igE= Received: from mail-lj1-f198.google.com (mail-lj1-f198.google.com [209.85.208.198]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-504-mlDLft-fMlW6zJGCUMVFhA-1; Fri, 29 Oct 2021 05:22:04 -0400 X-MC-Unique: mlDLft-fMlW6zJGCUMVFhA-1 Received: by mail-lj1-f198.google.com with SMTP id v13-20020a2e2f0d000000b0021126b5cca2so2836569ljv.19 for ; Fri, 29 Oct 2021 02:22:04 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=KuJ+ErAm24/e6XNNBz4b8p2/gaQuK/+p42ZYur74kQU=; b=45uUPTZze/HYufZ5A9XYcAhSm+oKiP+fteD1HaiK3MYTl441cWxD8Wgv9kfH443Py5 zLn117mWQRMQJk8Gg25AFG2W4zIuYYkpIZX9ZwYRkezmkp9dEVXWy/QzVu5RPbs3uzxQ Qh0xuLXO4eztovpSkB27KRvbfFQ5Z+aCg696H81K8n9QENTS4DcqReNSODhl+JlHor6O KXdss4n24t2iRZUHy7RSWQYw8h/3E4PPxrrpIvAFtNtbjYffHaWmbAljmCG4Tv+8ii4T 4c/5SVLu+QG/KSZhiy9p1kMe/pcslh3p5coqlfm+GmUjO5cPejg6d7ZpvUzVFaqUVcnj CICA== X-Gm-Message-State: AOAM533xjveC/erBY7EXHelpxQdDU3hK6VSL9/5gmRLJwddEdo9HRHwl yDvgvsaQAPeVqtVmqpmmB2q3TPofvdqiOICXOS8K5mD29Sa8+hXk2U1fBHj6pGra0orTUK4qPV5 Wqf/ko5JhEk7wik63kJQ= X-Received: by 2002:ac2:4bc1:: with SMTP id o1mr9015498lfq.553.1635499323119; Fri, 29 Oct 2021 02:22:03 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxz5uERjCiB9PUhnrUnApWavwPduhMpMNhWpjqrn8TSly6lAOvgIneiRR/HwR0x3Lne4V3KTbpt+doSxk5OT4c= X-Received: by 2002:ac2:4bc1:: with SMTP id o1mr9015467lfq.553.1635499322810; Fri, 29 Oct 2021 02:22:02 -0700 (PDT) MIME-Version: 1.0 References: <20211019151524.2005442-4-zhihongx.peng@intel.com> <20211020074643.3004385-1-zhihongx.peng@intel.com> In-Reply-To: <20211020074643.3004385-1-zhihongx.peng@intel.com> From: David Marchand Date: Fri, 29 Oct 2021 11:21:51 +0200 Message-ID: To: Zhihong Peng Cc: Thomas Monjalon , "Burakov, Anatoly" , "Ananyev, Konstantin" , Stephen Hemminger , Cristian Dumitrescu , "Mcnamara, John" , Bruce Richardson , dev , Xueqin Lin Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dmarchan@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH v13 1/4] enable ASan AddressSanitizer X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Replying on this patch since there is no cover letter. This series looks acceptable to me for rc2. Patch 3 and 4 will be merged first, since they fix issues that would be hit with ASan enabled. I have comments mainly on rewording in commitlogs and documentation. If you are fine with those comments, I'll fix them before pushing this series. On Wed, Oct 20, 2021 at 9:47 AM wrote: > > From: Zhihong Peng > > `AddressSanitizer > `_ (ASan) > is a widely-used debugging tool to detect memory access errors. > It helps to detect issues like use-after-free, various kinds of buffer > overruns in C/C++ programs, and other similar errors, as well as > printing out detailed debug information whenever an error is detected. > > We can enable ASan by adding below compilation options: > -Dbuildtype=debug -Db_lundef=false -Db_sanitize=address > "-Dbuildtype=debug": This is a non-essential option. When this option > is added, if a memory error occurs, ASan can clearly show where the > code is wrong. > "-Db_lundef=false": When use clang to compile DPDK, this option must > be added. > > Signed-off-by: Xueqin Lin > Signed-off-by: Zhihong Peng > Acked-by: John McNamara This patch affects the build process so it should be reflected in the patch title. I find the commitlog hard to read. Suggesting following rewording: """ build: enable AddressSanitizer AddressSanitizer [1] a.k.a. ASan is a widely-used debugging tool to detect memory access errors. It helps to detect issues like use-after-free, various kinds of buffer overruns in C/C++ programs, and other similar errors, as well as printing out detailed debug information whenever an error is detected. ASan is integrated with gcc and clang and can be enabled via a meson option: -Db_sanitize=address See the documentation for details (especially regarding clang). Enabling ASan has an impact on performance since additional checks are added to generated binaries. Enabling ASan with Windows is currently not supported in DPDK. 1: https://github.com/google/sanitizers/wiki/AddressSanitizer """ > --- > v7: 1) Split doc and code into two. > 2) Modify asan.rst doc > v8: No change. > v9: 1) Add the check of libasan library. > 2) Add release notes. > v10:1) Split doc and code into two. > 2) Meson supports asan. > v11:Modify the document. > v12:No change. > v13:Modify the document. > --- > config/meson.build | 16 ++++++++++++++ > devtools/words-case.txt | 1 + > doc/guides/prog_guide/asan.rst | 30 ++++++++++++++++++++++++++ > doc/guides/prog_guide/index.rst | 1 + > doc/guides/rel_notes/release_21_11.rst | 9 ++++++++ > 5 files changed, 57 insertions(+) > create mode 100644 doc/guides/prog_guide/asan.rst > > diff --git a/config/meson.build b/config/meson.build > index 4cdf589e20..f02b0e9c6d 100644 > --- a/config/meson.build > +++ b/config/meson.build > @@ -411,6 +411,22 @@ if get_option('b_lto') > endif > endif > > +if get_option('b_sanitize') == 'address' or get_option('b_sanitize') == 'address,undefined' > + if is_windows > + error('ASan is not supported on windows') > + endif I see clang started supporting ASan for Windows in version 8. https://releases.llvm.org/8.0.0/tools/clang/docs/AddressSanitizer.html I also found some blog about adding ASan support in MSVC. https://devblogs.microsoft.com/cppblog/addresssanitizer-asan-for-windows-with-msvc/ Keeping this limitation is acceptable for now, but I added a mention in commitlog. > + > + if cc.get_id() == 'gcc' > + asan_dep = cc.find_library('asan', required: true) > + if (not cc.links('int main(int argc, char *argv[]) { return 0; }', > + dependencies: asan_dep)) > + error('broken dependency, "libasan"') > + endif > + add_project_link_arguments('-lasan', language: 'c') > + dpdk_extra_ldflags += '-lasan' > + endif > +endif > + > if get_option('default_library') == 'both' > error( ''' > Unsupported value "both" for "default_library" option. > diff --git a/devtools/words-case.txt b/devtools/words-case.txt > index 0bbad48626..ada6910fa0 100644 > --- a/devtools/words-case.txt > +++ b/devtools/words-case.txt > @@ -5,6 +5,7 @@ API > Arm > armv7 > armv8 > +ASan > BAR > CRC > DCB > diff --git a/doc/guides/prog_guide/asan.rst b/doc/guides/prog_guide/asan.rst > new file mode 100644 > index 0000000000..6888fc9a87 > --- /dev/null > +++ b/doc/guides/prog_guide/asan.rst > @@ -0,0 +1,30 @@ > +.. SPDX-License-Identifier: BSD-3-Clause > + Copyright(c) 2021 Intel Corporation > + > +Running AddressSanitizer > +======================== > + > +`AddressSanitizer > +`_ (ASan) > +is a widely-used debugging tool to detect memory access errors. > +It helps to detect issues like use-after-free, various kinds of buffer > +overruns in C/C++ programs, and other similar errors, as well as > +printing out detailed debug information whenever an error is detected. > + > +AddressSanitizer is a part of LLVM (3.1+) and GCC (4.8+). > + > +Add following meson build commands to enable ASan in the meson build system: > + > +* gcc:: > + > + -Dbuildtype=debug -Db_sanitize=address > + > +* clang:: > + > + -Dbuildtype=debug -Db_lundef=false -Db_sanitize=address Suggests adding some explanations here and replacing like: """ ... AddressSanitizer is a part of LLVM (3.1+) and GCC (4.8+). Enabling ASan is done by passing the -Db_sanitize=address option to the meson build system, see :ref:`linux_gsg_compiling_dpdk` for details. The way ASan is integrated with clang requires to allow undefined symbols when linking code. To do this, the -Db_lundef=false option must be added. Additionally, passing -Dbuildtype=debug option might help getting more readable ASan reports. Example:: - gcc: meson setup -Db_sanitize=address - clang: meson setup -Db_sanitize=address -Db_lundef=false """ > + > +.. Note:: > + > + a) If compile with gcc in centos, libasan needs to be installed separately. > + b) If the program is tested using cmdline, you may need to execute the > + "stty echo" command when an error occurs. """ - The libasan package must be installed when compiling with gcc in Centos/RHEL. - If the program is tested using cmdline, you may need to execute the "stty echo" command when an error occurs. """ > diff --git a/doc/guides/prog_guide/index.rst b/doc/guides/prog_guide/index.rst > index 89af28dacb..b95c460b19 100644 > --- a/doc/guides/prog_guide/index.rst > +++ b/doc/guides/prog_guide/index.rst > @@ -71,4 +71,5 @@ Programmer's Guide > writing_efficient_code > lto > profile_app > + asan > glossary > diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst > index 3362c52a73..10f4275b1b 100644 > --- a/doc/guides/rel_notes/release_21_11.rst > +++ b/doc/guides/rel_notes/release_21_11.rst > @@ -173,6 +173,15 @@ New Features > * Added tests to verify tunnel header verification in IPsec inbound. > * Added tests to verify inner checksum. > > +* **Enable ASan AddressSanitizer.** Should be in past form: * **Added ASan support.** > + > + `AddressSanitizer > + `_ (ASan) > + is a widely-used debugging tool to detect memory access errors. > + It helps to detect issues like use-after-free, various kinds of buffer > + overruns in C/C++ programs, and other similar errors, as well as > + printing out detailed debug information whenever an error is detected. -- David Marchand