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

[Stephen Hemminger <stephen@networkplumber.org>]

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.