Thread (117 messages) 117 messages, 11 authors, 2021-11-03

Re: [dpdk-dev] [PATCH v6 1/2] Enable ASan for memory detector on DPDK

From: Peng, ZhihongX <hidden>
Date: 2021-10-14 06:33:56

-----Original Message-----
From: Richardson, Bruce <redacted>
Sent: Wednesday, October 13, 2021 4:00 PM
To: David Marchand <redacted>
Cc: Peng, ZhihongX <redacted>; Burakov, Anatoly
[off-list ref]; Ananyev, Konstantin
[off-list ref]; Stephen Hemminger
[off-list ref]; dev [off-list ref]; Lin, Xueqin
[off-list ref]; Thomas Monjalon [off-list ref]
Subject: Re: [dpdk-dev] [PATCH v6 1/2] Enable ASan for memory detector on
DPDK

On Thu, Sep 30, 2021 at 10:20:00AM +0200, David Marchand wrote:
quoted
Hello,

I see v6 is superseded in pw, I have been cleaning my queue... maybe my
fault.
quoted

On Thu, Sep 30, 2021 at 7:37 AM [off-list ref] wrote:
quoted
From: Zhihong Peng <redacted>

AddressSanitizer (ASan) is a google memory error detect standard
tool. It could help to detect use-after-free and
{heap,stack,global}-buffer overflow bugs in C/C++ programs, print
detailed error information when error happens, large improve debug
efficiency.

`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.
This patch mixes how to use ASan and instrumenting the DPDK mem
allocator.
quoted
I would split this patch in two.

The first patch can add the documentation on enabling/using ASan and
describe the known issues on enabling it.
I'd find it better (from a user pov) if we hide all those details
about b_lundef and installation of libasan on Centos.

Something like (only quickly tested):
diff --git a/config/meson.build b/config/meson.build index
4cdf589e20..7d8b71da79 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -411,6 +411,33 @@ if get_option('b_lto')
     endif
 endif

+if get_option('b_sanitize') == 'address'
+    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
+
 if get_option('default_library') == 'both'
     error( '''
  Unsupported value "both" for "default_library" option.

Bruce, do you see an issue with this approach?
Apologies for delayed reply on this.

No issue with this approach on my end, seems reasonable. Just watch out
that b_sanitize can have "address,undefined" as a possible value, so if we
want to support that, we can't just check directly for the literal string
"address"
quoted
Then a second patch adds the rte_malloc instrumentation, with a check
at configuration time.

     endif
     add_project_link_arguments('-lasan', language: 'c')
     dpdk_extra_ldflags += '-lasan'
+    if arch_subdir == 'x86'
+        asan_check_code = '''
+#ifdef __SANITIZE_ADDRESS__
+#define RTE_MALLOC_ASAN
+#elif defined(__has_feature)
+# if __has_feature(address_sanitizer) #define RTE_MALLOC_ASAN #
endif
quoted
+#endif
+
+#ifndef RTE_MALLOC_ASAN
+#error ASan not available.
+#endif
+'''
+        if cc.compiles(asan_check_code)
+            dpdk_conf.set10('RTE_MALLOC_ASAN', true)
+        endif
+    endif
 endif
Apologies, but I haven't been tracking this set in much detail.
Do we really need this second configuration check? Should it, or could it, be
merged into the check above for the asan library presence?
All code:
if get_option('b_sanitize') == 'address' or get_option('b_sanitize') == 'undefined'
    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
    endif

    if exec_env == 'linux' and arch_subdir == 'x86'
        dpdk_conf.set10('RTE_MALLOC_ASAN', true)
    endif
endif

Bruce, is this code correct?
Thanks!
/Bruce
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help