On Thu, Sep 30, 2021 at 10:20:00AM +0200, David Marchand wrote:
> Hello,
> 
> I see v6 is superseded in pw, I have been cleaning my queue... maybe my fault.
> 
> 
> On Thu, Sep 30, 2021 at 7:37 AM <zhihongx.p...@intel.com> wrote:
> >
> > From: Zhihong Peng <zhihongx.p...@intel.com>
> >
> > 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.
> 
> 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"

> 
> 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
> +#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?

/Bruce

Reply via email to