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.p...@intel.com> wrote:
>
> > > -----Original Message-----
> > > From: Stephen Hemminger <step...@networkplumber.org>
> > > Sent: Friday, September 10, 2021 10:48 AM
> > > To: Peng, ZhihongX <zhihongx.p...@intel.com>
> > > Cc: Burakov, Anatoly <anatoly.bura...@intel.com>; Ananyev, Konstantin
> > > <konstantin.anan...@intel.com>; dev@dpdk.org; Lin, Xueqin
> > > <xueqin....@intel.com>
> > > Subject: Re: [PATCH] Enable AddressSanitizer feature on DPDK
> > >
> > > On Fri, 10 Sep 2021 02:01:47 +0000
> > > zhihongx.p...@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.
Is something like the following what you had in mind?
#ifdef __SANITIZE_ADDRESS__
#define RTE_MALLOC_ASAN
#elif defined(__has_feature) && __has_feature(address_sanitizer)
#define RTE_MALLOC_ASAN
#endif
/Bruce
