Re: [Xen-devel] [PATCH v3 1/2] xen: add config option to include failing condition in BUG_ON() message

[Jürgen Groß <jgross@xxxxxxxx>]

On 21.01.20 12:38, Julien Grall wrote:
> Hi Juergen,
>
> On 21/01/2020 10:13, Juergen Gross wrote:
> > Today a triggering BUG_ON() will only print source file and line
> > information. Add the possibility to print the triggering condition like
> > ASSERT().
> Any reason to only limit the change for BUG_ON? How about WARN_ON?
Hmm, why not.

Any objections?

> > Do that by introducing BUG_VERBOSE() and add a Kconfig option to make
> > BUG_ON use BUG_VERBOSE().
> >
> > Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> > ---
> > V3:
> > - move kconfig option out of DEBUG || expert section (Jan Beulich)
> > - fix ARM build (Jan Beulich)
> > - eliminate BUG_ON_VERBOSE() (Jan Beulich)
> > ---
> >   xen/Kconfig.debug         | 8 +++++++-
> >   xen/include/asm-arm/bug.h | 6 ++++--
> >   xen/include/asm-x86/bug.h | 5 +++--
> >   xen/include/xen/lib.h     | 4 ++++
> >   4 files changed, 18 insertions(+), 5 deletions(-)
> >
> > diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
> > index b3511e81a2..75c855e4ae 100644
> > --- a/xen/Kconfig.debug
> > +++ b/xen/Kconfig.debug
> > @@ -11,6 +11,13 @@ config DEBUG
> >         You probably want to say 'N' here.
> > +config DEBUG_BUGVERBOSE
> > +    bool "Verbose BUG_ON messages"
> > +    default DEBUG
> > +    ---help---
> > +      In case a BUG_ON triggers additionally print the triggering
> > +      condition on the console.
> > +
> >   if DEBUG || EXPERT = "y"
> >   config CRASH_DEBUG
> > @@ -81,7 +88,6 @@ config PERF_ARRAYS
> >       ---help---
> >         Enables software performance counter array histograms.
> > -
> While I agree this should be dropped this is a spurious line, this feels 
> a bit out of context. So I would suggest to mention it in the commit 
> message or split it in a separate patch.
In fact this is the result of moving the new option in V3.

I'll mention the change in the commit message.

> >   config VERBOSE_DEBUG
> >       bool "Verbose debug messages"
> >       default DEBUG
> > diff --git a/xen/include/asm-arm/bug.h b/xen/include/asm-arm/bug.h
> > index 36c803357c..90b231e77a 100644
> > --- a/xen/include/asm-arm/bug.h
> > +++ b/xen/include/asm-arm/bug.h
> > @@ -60,11 +60,13 @@ struct bug_frame {
> >   #define WARN() BUG_FRAME(BUGFRAME_warn, __LINE__, __FILE__, 0, "")
> > -#define BUG() do {                                              \
> > -    BUG_FRAME(BUGFRAME_bug,  __LINE__, __FILE__, 0, "");        \
> > +#define BUG_VERBOSE(msg) do {                                   \
> > +    BUG_FRAME(BUGFRAME_bug,  __LINE__, __FILE__, 0, msg);       \
> I am afraid this is not going to be enough to make it work on Arm. You 
> also need to update do_bug_frame() to print the string.
> I would actually expect a similar change required on the x86 side. Do 
> you mind explaining how this works?
You are right, of course.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel