diff mbox series

[v12,1/4] Enable ASan Address Sanitization

Message ID 20211019135841.2004819-1-zhihongx.peng@intel.com (mailing list archive)
State Superseded, archived
Headers show
Series [v12,1/4] Enable ASan Address Sanitization | expand

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Peng, ZhihongX Oct. 19, 2021, 1:58 p.m. UTC
From: Zhihong Peng <zhihongx.peng@intel.com>

`AddressSanitizer
<https://github.com/google/sanitizers/wiki/AddressSanitizer>`_ (ASan)
is a widely-used debugging tool to detect memory access errors.
It helps 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.lin@intel.com>
Signed-off-by: Zhihong Peng <zhihongx.peng@intel.com>
---
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.
---
 config/meson.build                     | 16 +++++++++++
 devtools/words-case.txt                |  1 +
 doc/guides/prog_guide/asan.rst         | 40 ++++++++++++++++++++++++++
 doc/guides/prog_guide/index.rst        |  1 +
 doc/guides/rel_notes/release_21_11.rst |  9 ++++++
 5 files changed, 67 insertions(+)
 create mode 100644 doc/guides/prog_guide/asan.rst

Comments

Peng, ZhihongX Oct. 19, 2021, 2:47 p.m. UTC | #1
Hi, David

The v12 version has been submitted, and patch 3 and 4 has been acked.

V10 information:

I have compiled passed on the x86/arm/ppc platforms, directory targets  as below :
build-arm64-bluefield  build-arm64-host-clang  build-clang-shared  build-gcc-shared  build-ppc64le-power8
build-arm64-dpaa build-arm64-octeontx2   build-clang-static  build-gcc-static  build-x86-generic

We do not support the windows platform due to standard google document(https://github.com/google/sanitizers/wiki/AddressSanitizer)
also not support this.
We also sent our cross-compilation log to you in an other email. Passed to run unit test  for dpdk-testpmd simple on x86 platform.
What else is blocking the process ? 
What's any action we should do for the code merge? High appreciate for your check and feedback.

> -----Original Message-----
> From: Peng, ZhihongX <zhihongx.peng@intel.com>
> Sent: Tuesday, October 19, 2021 9:59 PM
> To: david.marchand@redhat.com; Burakov, Anatoly
> <anatoly.burakov@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; stephen@networkplumber.org;
> Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>
> Cc: dev@dpdk.org; Lin, Xueqin <xueqin.lin@intel.com>; Peng, ZhihongX
> <zhihongx.peng@intel.com>
> Subject: [PATCH v12 1/4] Enable ASan Address Sanitization
> 
> From: Zhihong Peng <zhihongx.peng@intel.com>
> 
> `AddressSanitizer
> <https://github.com/google/sanitizers/wiki/AddressSanitizer>`_ (ASan) is a
> widely-used debugging tool to detect memory access errors.
> It helps 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.lin@intel.com>
> Signed-off-by: Zhihong Peng <zhihongx.peng@intel.com>
> ---
> 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.
> ---
>  config/meson.build                     | 16 +++++++++++
>  devtools/words-case.txt                |  1 +
>  doc/guides/prog_guide/asan.rst         | 40 ++++++++++++++++++++++++++
>  doc/guides/prog_guide/index.rst        |  1 +
>  doc/guides/rel_notes/release_21_11.rst |  9 ++++++
>  5 files changed, 67 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
> +
> +    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..969676ebe8
> --- /dev/null
> +++ b/doc/guides/prog_guide/asan.rst
> @@ -0,0 +1,40 @@
> +.. SPDX-License-Identifier: BSD-3-Clause
> +   Copyright(c) 2021 Intel Corporation
> +
> +Running Address Sanitizer
> +=========================
> +
> +`AddressSanitizer
> +<https://github.com/google/sanitizers/wiki/AddressSanitizer>`_ (ASan)
> +is a widely-used debugging tool to detect memory access errors.
> +It helps 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+).
> +
> +Usage
> +-----
> +
> +meson build
> +^^^^^^^^^^^
> +
> +To enable ASan in meson build system, use following meson build
> command:
> +
> +Example usage::
> +
> +* gcc::
> +
> +      meson build -Dbuildtype=debug -Db_sanitize=address
> +      ninja -C build
> +
> +* clang::
> +
> +      meson build -Dbuildtype=debug -Db_lundef=false -Db_sanitize=address
> +      ninja -C build
> +
> +.. Note::
> +
> +  a) To compile with gcc in centos, libasan needs to be installed separately.
> +  b) If the program being tested uses cmdline you will need to execute the
> +     "stty echo" command when a 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 d5435a64aa..63d9fef1b4 100644
> --- a/doc/guides/rel_notes/release_21_11.rst
> +++ b/doc/guides/rel_notes/release_21_11.rst
> @@ -167,6 +167,15 @@ New Features
>    * Added tests to verify tunnel header verification in IPsec inbound.
>    * Added tests to verify inner checksum.
> 
> +* **Enable ASan Address Sanitization.**
> +
> +  `AddressSanitizer
> +  <https://github.com/google/sanitizers/wiki/AddressSanitizer>`_ (ASan)
> + is a widely-used debugging tool to detect memory access errors.
> +  It helps 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.
> +
> 
>  Removed Items
>  -------------
> --
> 2.25.1
Mcnamara, John Oct. 19, 2021, 3:17 p.m. UTC | #2
> > diff --git a/doc/guides/prog_guide/asan.rst
> > b/doc/guides/prog_guide/asan.rst new file mode 100644 index
> > 0000000000..969676ebe8
> > --- /dev/null
> > +++ b/doc/guides/prog_guide/asan.rst
> > @@ -0,0 +1,40 @@
> > +.. SPDX-License-Identifier: BSD-3-Clause
> > +   Copyright(c) 2021 Intel Corporation
> > +


> > ...

> > +
> > +Usage
> > +-----
> > +
> > +meson build
> > +^^^^^^^^^^^

These 2 additional header levels aren't needed. If you do another version you can leave them out. Apart from that:

Acked-by: John McNamara <john.mcnamara@intel.com>
David Marchand Oct. 19, 2021, 3:20 p.m. UTC | #3
On Tue, Oct 19, 2021 at 4:50 PM Peng, ZhihongX <zhihongx.peng@intel.com> wrote:
> The v12 version has been submitted, and patch 3 and 4 has been acked.
>
> V10 information:
>
> I have compiled passed on the x86/arm/ppc platforms, directory targets  as below :
> build-arm64-bluefield  build-arm64-host-clang  build-clang-shared  build-gcc-shared  build-ppc64le-power8
> build-arm64-dpaa build-arm64-octeontx2   build-clang-static  build-gcc-static  build-x86-generic
>
> We do not support the windows platform due to standard google document(https://github.com/google/sanitizers/wiki/AddressSanitizer)
> also not support this.
> We also sent our cross-compilation log to you in an other email. Passed to run unit test  for dpdk-testpmd simple on x86 platform.
> What else is blocking the process ?
> What's any action we should do for the code merge? High appreciate for your check and feedback.

This series has seen so many revisions (often numbered the same, and
while I was writting this reply, here is a new revision) in the last
days that I put it at the end of my queue simply to wait for a stable
state.

By the way, simply pinging me to get your patches merged is easy, but
if you care about the dpdk project, there are other series that need
reviews/checks when merging, you are welcome to review and test
patches.
Peng, ZhihongX Oct. 20, 2021, 1:55 a.m. UTC | #4
> -----Original Message-----
> From: Mcnamara, John <john.mcnamara@intel.com>
> Sent: Tuesday, October 19, 2021 11:18 PM
> To: Peng, ZhihongX <zhihongx.peng@intel.com>;
> david.marchand@redhat.com; Burakov, Anatoly
> <anatoly.burakov@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; stephen@networkplumber.org;
> Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>
> Cc: dev@dpdk.org; Lin, Xueqin <xueqin.lin@intel.com>
> Subject: RE: [PATCH v12 1/4] Enable ASan Address Sanitization
> 
> 
> > > diff --git a/doc/guides/prog_guide/asan.rst
> > > b/doc/guides/prog_guide/asan.rst new file mode 100644 index
> > > 0000000000..969676ebe8
> > > --- /dev/null
> > > +++ b/doc/guides/prog_guide/asan.rst
> > > @@ -0,0 +1,40 @@
> > > +.. SPDX-License-Identifier: BSD-3-Clause
> > > +   Copyright(c) 2021 Intel Corporation
> > > +
> 
> 
> > > ...
> 
> > > +
> > > +Usage
> > > +-----
> > > +
> > > +meson build
> > > +^^^^^^^^^^^
> 
> These 2 additional header levels aren't needed. If you do another version
> you can leave them out. Apart from that:

I will fix it in v13.

> Acked-by: John McNamara <john.mcnamara@intel.com>
Lin, Xueqin Oct. 20, 2021, 2:41 a.m. UTC | #5
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Tuesday, October 19, 2021 11:21 PM
> To: Peng, ZhihongX <zhihongx.peng@intel.com>
> Cc: Burakov, Anatoly <anatoly.burakov@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; stephen@networkplumber.org;
> Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; dev@dpdk.org; Lin, Xueqin
> <xueqin.lin@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v12 1/4] Enable ASan Address Sanitization
> 
> On Tue, Oct 19, 2021 at 4:50 PM Peng, ZhihongX <zhihongx.peng@intel.com>
> wrote:
> > The v12 version has been submitted, and patch 3 and 4 has been acked.
> >
> > V10 information:
> >
> > I have compiled passed on the x86/arm/ppc platforms, directory targets  as
> below :
> > build-arm64-bluefield  build-arm64-host-clang  build-clang-shared  build-
> gcc-shared  build-ppc64le-power8
> > build-arm64-dpaa build-arm64-octeontx2   build-clang-static  build-gcc-
> static  build-x86-generic
> >
> > We do not support the windows platform due to standard google
> > document(https://github.com/google/sanitizers/wiki/AddressSanitizer)
> > also not support this.
> > We also sent our cross-compilation log to you in an other email. Passed to
> run unit test  for dpdk-testpmd simple on x86 platform.
> > What else is blocking the process ?
> > What's any action we should do for the code merge? High appreciate for
> your check and feedback.
> 
> This series has seen so many revisions (often numbered the same, and while
> I was writting this reply, here is a new revision) in the last days that I put it at
> the end of my queue simply to wait for a stable state.
> 
> By the way, simply pinging me to get your patches merged is easy, but if you
> care about the dpdk project, there are other series that need reviews/checks
> when merging, you are welcome to review and test patches.

Really sorry to bring you so many trouble, apart from ASan implementation patch, also have other error build, doc... patches, recently receive some comments from other domain experts and maintainers.
We are active to deal with them and have so many revisions. For the same numbers, it is our fault, sorry for that, and thanks a lot for your and reviewers' great support. 
Today we will send v13 version, and all the fix will be in this version. 
We also care about the quality, make sure to run cross-compilation build and unit test passed on our platforms then send the patch.
Hope that everything is well and could meet the expectation,  thanks a lot. 

> 
> 
> --
> David Marchand
diff mbox series

Patch

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
+
+    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..969676ebe8
--- /dev/null
+++ b/doc/guides/prog_guide/asan.rst
@@ -0,0 +1,40 @@ 
+.. SPDX-License-Identifier: BSD-3-Clause
+   Copyright(c) 2021 Intel Corporation
+
+Running Address Sanitizer
+=========================
+
+`AddressSanitizer
+<https://github.com/google/sanitizers/wiki/AddressSanitizer>`_ (ASan)
+is a widely-used debugging tool to detect memory access errors.
+It helps 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+).
+
+Usage
+-----
+
+meson build
+^^^^^^^^^^^
+
+To enable ASan in meson build system, use following meson build command:
+
+Example usage::
+
+* gcc::
+
+      meson build -Dbuildtype=debug -Db_sanitize=address
+      ninja -C build
+
+* clang::
+
+      meson build -Dbuildtype=debug -Db_lundef=false -Db_sanitize=address
+      ninja -C build
+
+.. Note::
+
+  a) To compile with gcc in centos, libasan needs to be installed separately.
+  b) If the program being tested uses cmdline you will need to execute the
+     "stty echo" command when a 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 d5435a64aa..63d9fef1b4 100644
--- a/doc/guides/rel_notes/release_21_11.rst
+++ b/doc/guides/rel_notes/release_21_11.rst
@@ -167,6 +167,15 @@  New Features
   * Added tests to verify tunnel header verification in IPsec inbound.
   * Added tests to verify inner checksum.
 
+* **Enable ASan Address Sanitization.**
+
+  `AddressSanitizer
+  <https://github.com/google/sanitizers/wiki/AddressSanitizer>`_ (ASan)
+  is a widely-used debugging tool to detect memory access errors.
+  It helps 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.
+
 
 Removed Items
 -------------