Re: [dpdk-dev] [PATCH] Enable AddressSanitizer feature on DPDK

["Lin, Xueqin" <xueqin.lin@intel.com>]

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Monday, September 13, 2021 11:46 PM
> To: Richardson, Bruce <bruce.richardson@intel.com>
> Cc: Peng, ZhihongX <zhihongx.peng@intel.com>; Burakov, Anatoly
> <anatoly.burakov@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; dev@dpdk.org; Lin, Xueqin
> <xueqin.lin@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] Enable AddressSanitizer feature on DPDK
> 
> On Mon, 13 Sep 2021 16:22:13 +0100
> Bruce Richardson <bruce.richardson@intel.com> wrote:
> 
> > On Mon, Sep 13, 2021 at 08:05:58AM -0700, Stephen Hemminger wrote:
> > > On Mon, 13 Sep 2021 05:27:12 +0000
> > > "Peng, ZhihongX" <zhihongx.peng@intel.com> wrote:
> > >
> > > > > -----Original Message-----
> > > > > From: Stephen Hemminger <stephen@networkplumber.org>
> > > > > Sent: Friday, September 10, 2021 10:48 AM
> > > > > To: Peng, ZhihongX <zhihongx.peng@intel.com>
> > > > > Cc: Burakov, Anatoly <anatoly.burakov@intel.com>; Ananyev,
> > > > > Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org; Lin,
> > > > > Xueqin <xueqin.lin@intel.com>
> > > > > Subject: Re: [PATCH] Enable AddressSanitizer feature on DPDK
> > > > >
> > > > > On Fri, 10 Sep 2021 02:01:47 +0000 zhihongx.peng@intel.com
> > > > > wrote:
> > > > >
> > > > > >
> > > > > > +if get_option('b_sanitize').startswith('address')
> > > > > > +	cflags += '-DRTE_MALLOC_ASAN'
> > > > > > +endif
> > > > > > +
> > > > >
> > > > > This looks great, but can we make it just do-the-right-thing and
> > > > > get rid of the nerd knobs (i.e no meson configure).
> > > > >
> >
> > There are no new meson options being added here. Turning on/off
> > address sanitizing is a built-in meson option that is there already.
> >
> > > > > The address sanitizer already has a way to detect if enabled.
> > > > >
> > > > > GCC uses:
> > > > > __SANITIZE_ADDRESS__
> > > > >
> > > > > Clang uses:
> > > > > #if defined(__has_feature)
> > > > > #  if __has_feature(address_sanitizer)
> > > >
> > > > Tried this method you said. It can run successfully. Because gcc
> > > > and clang have different Methods for determining whether to turn
> > > > on the asan function, so if you judge the two methods in the code, it
> feels not simple to judge in meson.
> > >
> > > There is already compiler specific #ifdef's why not do this contained in
> one header file?
> > >
> > > The point is DPDK is trying to get away from having configuration
> > > settings if at all possible. Configuration creates dependency
> > > nightmares and also leaves many code paths as never tested.
> >
> > Not sure I follow your point here. We need some macro to easily tell
> > if we are running with address sanitization enabled or not, so as to
> > avoid having the multi-compiler detection rules all over the place.
> > The only question is where it's better to have this in a header file or a
> meson.build file.
> > Given your objection and the fact that the meson.build code above
> > looks a little awkward, I'd suggest putting the conditional checks in
> malloc_elem.h.
> 
> NVM working of meson as commn base seems like good option.


+1, good option for the tool enable, thanks a lot for your guide and positive feedback. 
 We will update them in V2 and send V2 soon.
High appreciate that you can ACK this feature patch if no other objection, thanks.