> -----Original Message----- > From: David Marchand <david.march...@redhat.com> > Sent: Friday, October 29, 2021 5:22 PM > To: Peng, ZhihongX <zhihongx.p...@intel.com> > Cc: Thomas Monjalon <tho...@monjalon.net>; Burakov, Anatoly > <anatoly.bura...@intel.com>; Ananyev, Konstantin > <konstantin.anan...@intel.com>; Stephen Hemminger > <step...@networkplumber.org>; Dumitrescu, Cristian > <cristian.dumitre...@intel.com>; Mcnamara, John > <john.mcnam...@intel.com>; Richardson, Bruce > <bruce.richard...@intel.com>; dev <dev@dpdk.org>; Lin, Xueqin > <xueqin....@intel.com> > Subject: Re: [PATCH v13 1/4] enable ASan AddressSanitizer > > 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.
Thank you very much for your help, I agree with this modification, it's good. You have done a lot of work for asan patch, thank you again. Regards, Peng,Zhihong > > > On Wed, Oct 20, 2021 at 9:47 AM <zhihongx.p...@intel.com> wrote: > > > > From: Zhihong Peng <zhihongx.p...@intel.com> > > > > `AddressSanitizer > > <https://github.com/google/sanitizers/wiki/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 <xueqin....@intel.com> > > Signed-off-by: Zhihong Peng <zhihongx.p...@intel.com> > > Acked-by: John McNamara <john.mcnam...@intel.com> > > 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 > > +<https://github.com/google/sanitizers/wiki/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 <build_dir> > - clang: meson setup -Db_sanitize=address -Db_lundef=false <build_dir> """ > > > + > > +.. 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 > > + <https://github.com/google/sanitizers/wiki/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
