[PATCH] Fixes issue when debugging debug builds of Perf.

[PATCH] Fixes issue when debugging debug builds of Perf.

[James Clark]

When a 'make DEBUG=1' build is done, the command parser
is still built with -O6 and is hard to step through.

This change also moves EXTRA_WARNINGS and EXTRA_FLAGS to
the end of the compilation line, otherwise they cannot be
used to override the default values.

Signed-off-by: James Clark <james.clark@arm.com>
---
 tools/lib/subcmd/Makefile | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tools/lib/subcmd/Makefile b/tools/lib/subcmd/Makefile
index 5b2cd5e58df0..1c777a72bb39 100644
--- a/tools/lib/subcmd/Makefile
+++ b/tools/lib/subcmd/Makefile
@@ -19,8 +19,7 @@ MAKEFLAGS += --no-print-directory
 
 LIBFILE = $(OUTPUT)libsubcmd.a
 
-CFLAGS := $(EXTRA_WARNINGS) $(EXTRA_CFLAGS)
-CFLAGS += -ggdb3 -Wall -Wextra -std=gnu99 -fPIC
+CFLAGS := -ggdb3 -Wall -Wextra -std=gnu99 -fPIC
 
 ifeq ($(DEBUG),0)
   ifeq ($(feature-fortify-source), 1)
@@ -28,7 +27,9 @@ ifeq ($(DEBUG),0)
   endif
 endif
 
-ifeq ($(CC_NO_CLANG), 0)
+ifeq ($(DEBUG),1)
+  CFLAGS += -O0
+else ifeq ($(CC_NO_CLANG), 0)
   CFLAGS += -O3
 else
   CFLAGS += -O6
@@ -43,6 +44,8 @@ CFLAGS += -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE
 
 CFLAGS += -I$(srctree)/tools/include/
 
+CFLAGS += $(EXTRA_WARNINGS) $(EXTRA_CFLAGS)
+
 SUBCMD_IN := $(OUTPUT)libsubcmd-in.o
 
 all:
-- 
2.23.0

Re: [PATCH] Fixes issue when debugging debug builds of Perf.

[Arnaldo Carvalho de Melo]

Em Mon, Oct 28, 2019 at 11:34:01AM +0000, James Clark escreveu:
> When a 'make DEBUG=1' build is done, the command parser
> is still built with -O6 and is hard to step through.
> 
> This change also moves EXTRA_WARNINGS and EXTRA_FLAGS to
> the end of the compilation line, otherwise they cannot be
> used to override the default values.

The patch came mangled, so I'm applying by hand, and separating it into
two patches, the first for the first paragraph and the other for the
second, ok?

- Arnaldo
 
> Signed-off-by: James Clark <james.clark@arm.com>
> ---
>  tools/lib/subcmd/Makefile | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/lib/subcmd/Makefile b/tools/lib/subcmd/Makefile
> index 5b2cd5e58df0..1c777a72bb39 100644
> --- a/tools/lib/subcmd/Makefile
> +++ b/tools/lib/subcmd/Makefile
> @@ -19,8 +19,7 @@ MAKEFLAGS += --no-print-directory
>  
>  LIBFILE = $(OUTPUT)libsubcmd.a
>  
> -CFLAGS := $(EXTRA_WARNINGS) $(EXTRA_CFLAGS)
> -CFLAGS += -ggdb3 -Wall -Wextra -std=gnu99 -fPIC
> +CFLAGS := -ggdb3 -Wall -Wextra -std=gnu99 -fPIC
>  
>  ifeq ($(DEBUG),0)
>    ifeq ($(feature-fortify-source), 1)
> @@ -28,7 +27,9 @@ ifeq ($(DEBUG),0)
>    endif
>  endif
>  
> -ifeq ($(CC_NO_CLANG), 0)
> +ifeq ($(DEBUG),1)
> +  CFLAGS += -O0
> +else ifeq ($(CC_NO_CLANG), 0)
>    CFLAGS += -O3
>  else
>    CFLAGS += -O6
> @@ -43,6 +44,8 @@ CFLAGS += -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE
>  
>  CFLAGS += -I$(srctree)/tools/include/
>  
> +CFLAGS += $(EXTRA_WARNINGS) $(EXTRA_CFLAGS)
> +
>  SUBCMD_IN := $(OUTPUT)libsubcmd-in.o
>  
>  all:
> -- 
> 2.23.0

-- 

- Arnaldo

Re: [PATCH] Fixes issue when debugging debug builds of Perf.

[Arnaldo Carvalho de Melo]

Em Tue, Oct 29, 2019 at 11:00:52AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Mon, Oct 28, 2019 at 11:34:01AM +0000, James Clark escreveu:
> > When a 'make DEBUG=1' build is done, the command parser
> > is still built with -O6 and is hard to step through.
> > 
> > This change also moves EXTRA_WARNINGS and EXTRA_FLAGS to
> > the end of the compilation line, otherwise they cannot be
> > used to override the default values.
> 
> The patch came mangled, so I'm applying by hand, and separating it into
> two patches, the first for the first paragraph and the other for the
> second, ok?

So, this is what I mean by mangled:

diff --git a/tools/lib/subcmd/Makefile b/tools/lib/subcmd/Makefile
index 5b2cd5e58df0..1c777a72bb39 100644
--- a/tools/lib/subcmd/Makefile
+++ b/tools/lib/subcmd/Makefile
@@ -19,8 +19,7 @@ MAKEFLAGS +=3D --no-print-directory
=20
 LIBFILE =3D $(OUTPUT)libsubcmd.a
=20
-CFLAGS :=3D $(EXTRA_WARNINGS) $(EXTRA_CFLAGS)
-CFLAGS +=3D -ggdb3 -Wall -Wextra -std=3Dgnu99 -fPIC
+CFLAGS :=3D -ggdb3 -Wall -Wextra -std=3Dgnu99 -fPIC
=20
 ifeq ($(DEBUG),0)
   ifeq ($(feature-fortify-source), 1)
@@ -28,7 +27,9 @@ ifeq ($(DEBUG),0)
   endif
 endif
=20
-ifeq ($(CC_NO_CLANG), 0)


--------------------

And here is the first patch out of your larger one, I changed the
subject line to reflect that this is not tools/perf specific, as
tools/objtool/ also uses libsubcmd, added Josh, objtool's maintainer so
that he is made aware.

Thanks,

- Arnaldo

commit a554275abf9f13054595d3155b835668dab74bf9
Author: James Clark <James.Clark@arm.com>
Date:   Mon Oct 28 11:34:01 2019 +0000

    libsubcmd: Move EXTRA_FLAGS to the end to allow overriding existing flags
    
    Move EXTRA_WARNINGS and EXTRA_FLAGS to the end of the compilation line,
    otherwise they cannot be used to override the default values.
    
    Signed-off-by: James Clark <james.clark@arm.com>
    Cc: Adrian Hunter <adrian.hunter@intel.com>
    Cc: Ian Rogers <irogers@google.com>
    Cc: James Clark <james.clark@arm.com>
    Cc: Jiri Olsa <jolsa@kernel.org>
    Cc: Josh Poimboeuf <jpoimboe@redhat.com>
    Cc: Namhyung Kim <namhyung@kernel.org>
    Cc: nd <nd@arm.com>
    Link: http://lore.kernel.org/lkml/20191028113340.4282-1-james.clark@arm.com
    [ split from a larger patch ]
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/lib/subcmd/Makefile b/tools/lib/subcmd/Makefile
index 5b2cd5e58df0..352c6062deba 100644
--- a/tools/lib/subcmd/Makefile
+++ b/tools/lib/subcmd/Makefile
@@ -19,8 +19,7 @@ MAKEFLAGS += --no-print-directory
 
 LIBFILE = $(OUTPUT)libsubcmd.a
 
-CFLAGS := $(EXTRA_WARNINGS) $(EXTRA_CFLAGS)
-CFLAGS += -ggdb3 -Wall -Wextra -std=gnu99 -fPIC
+CFLAGS := -ggdb3 -Wall -Wextra -std=gnu99 -fPIC
 
 ifeq ($(DEBUG),0)
   ifeq ($(feature-fortify-source), 1)
@@ -43,6 +42,8 @@ CFLAGS += -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE
 
 CFLAGS += -I$(srctree)/tools/include/
 
+CFLAGS += $(EXTRA_WARNINGS) $(EXTRA_CFLAGS)
+
 SUBCMD_IN := $(OUTPUT)libsubcmd-in.o
 
 all:

Re: [PATCH] Fixes issue when debugging debug builds of Perf.

[Arnaldo Carvalho de Melo]

Em Tue, Oct 29, 2019 at 11:18:52AM -0300, Arnaldo Carvalho de Melo escreveu:
> And here is the first patch out of your larger one, I changed the
> subject line to reflect that this is not tools/perf specific, as
> tools/objtool/ also uses libsubcmd, added Josh, objtool's maintainer so
> that he is made aware.

And the second patch:


commit d0381449fd9ab733ec2daf527263da9f73f1e94e
Author: James Clark <James.Clark@arm.com>
Date:   Mon Oct 28 11:34:01 2019 +0000

    libsubcmd: Use -O0 with DEBUG=1
    
    When a 'make DEBUG=1' build is done, the command parser is still built
    with -O6 and is hard to step through, fix it making it use -O0 in that
    case.
    
    Signed-off-by: James Clark <james.clark@arm.com>
    Cc: Adrian Hunter <adrian.hunter@intel.com>
    Cc: Ian Rogers <irogers@google.com>
    Cc: Jiri Olsa <jolsa@kernel.org>
    Cc: Josh Poimboeuf <jpoimboe@redhat.com>
    Cc: Namhyung Kim <namhyung@kernel.org>
    Cc: nd <nd@arm.com>
    Link: http://lore.kernel.org/lkml/20191028113340.4282-1-james.clark@arm.com
    [ split from a larger patch ]
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/lib/subcmd/Makefile b/tools/lib/subcmd/Makefile
index 352c6062deba..1c777a72bb39 100644
--- a/tools/lib/subcmd/Makefile
+++ b/tools/lib/subcmd/Makefile
@@ -27,7 +27,9 @@ ifeq ($(DEBUG),0)
   endif
 endif
 
-ifeq ($(CC_NO_CLANG), 0)
+ifeq ($(DEBUG),1)
+  CFLAGS += -O0
+else ifeq ($(CC_NO_CLANG), 0)
   CFLAGS += -O3
 else
   CFLAGS += -O6

Re: [PATCH] Fixes issue when debugging debug builds of Perf.

[James Clark]

Hi Arnaldo,

Thanks for that, yes separating them looks better.
I will try to break down commits in the future.

> The patch came mangled, so I'm applying by hand, and separating it into
> two patches, the first for the first paragraph and the other for the
> second, ok?

By mangled do you mean the quoted printables "=3D" and "=20"?

It seems like git send-email falls back to this behavior by default:

         
     --transfer-encoding=(7bit|8bit|quoted-printable|base64|auto)

       Specify the transfer encoding to be used to send the message over SMTP.
       7bit will fail upon encountering a non-ASCII message. quoted-printable can be
       useful when the repository contains files that contain carriage returns, but
       makes the raw patch email file (as saved from a MUA) much harder to inspect
       manually. base64 is even more fool proof, but also even more opaque. auto will
       use 8bit when possible, and quoted-printable otherwise.


I copied my raw patch and was able to successfully apply it with git am, even with this escaping. Although I
did upgrade to a newer version of git (2.23.0).

If I view the patch that you created, then it doesn't have quoted printable escaping. So there does
seem to be a difference somewhere.
Do you think I should use "git send-email --transfer-encoding=7bit"?


Thanks
James

On 29/10/2019 14:26, Arnaldo Carvalho de Melo wrote:
> Em Tue, Oct 29, 2019 at 11:18:52AM -0300, Arnaldo Carvalho de Melo escreveu:
>> And here is the first patch out of your larger one, I changed the
>> subject line to reflect that this is not tools/perf specific, as
>> tools/objtool/ also uses libsubcmd, added Josh, objtool's maintainer so
>> that he is made aware.
> 
> And the second patch:
> 
> 
> commit d0381449fd9ab733ec2daf527263da9f73f1e94e
> Author: James Clark <James.Clark@arm.com>
> Date:   Mon Oct 28 11:34:01 2019 +0000
> 
>     libsubcmd: Use -O0 with DEBUG=1
>     
>     When a 'make DEBUG=1' build is done, the command parser is still built
>     with -O6 and is hard to step through, fix it making it use -O0 in that
>     case.
>     
>     Signed-off-by: James Clark <james.clark@arm.com>
>     Cc: Adrian Hunter <adrian.hunter@intel.com>
>     Cc: Ian Rogers <irogers@google.com>
>     Cc: Jiri Olsa <jolsa@kernel.org>
>     Cc: Josh Poimboeuf <jpoimboe@redhat.com>
>     Cc: Namhyung Kim <namhyung@kernel.org>
>     Cc: nd <nd@arm.com>
>     Link: http://lore.kernel.org/lkml/20191028113340.4282-1-james.clark@arm.com
>     [ split from a larger patch ]
>     Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> diff --git a/tools/lib/subcmd/Makefile b/tools/lib/subcmd/Makefile
> index 352c6062deba..1c777a72bb39 100644
> --- a/tools/lib/subcmd/Makefile
> +++ b/tools/lib/subcmd/Makefile
> @@ -27,7 +27,9 @@ ifeq ($(DEBUG),0)
>    endif
>  endif
>  
> -ifeq ($(CC_NO_CLANG), 0)
> +ifeq ($(DEBUG),1)
> +  CFLAGS += -O0
> +else ifeq ($(CC_NO_CLANG), 0)
>    CFLAGS += -O3
>  else
>    CFLAGS += -O6
> 

Re: [PATCH] Fixes issue when debugging debug builds of Perf.

[Arnaldo Carvalho de Melo]

Em Wed, Oct 30, 2019 at 11:03:43AM +0000, James Clark escreveu:
> Hi Arnaldo,
> 
> Thanks for that, yes separating them looks better.
> I will try to break down commits in the future.
> 
> > The patch came mangled, so I'm applying by hand, and separating it into
> > two patches, the first for the first paragraph and the other for the
> > second, ok?
> 
> By mangled do you mean the quoted printables "=3D" and "=20"?
> 
> It seems like git send-email falls back to this behavior by default:
> 
>          
>      --transfer-encoding=(7bit|8bit|quoted-printable|base64|auto)
> 
>        Specify the transfer encoding to be used to send the message over SMTP.
>        7bit will fail upon encountering a non-ASCII message. quoted-printable can be
>        useful when the repository contains files that contain carriage returns, but
>        makes the raw patch email file (as saved from a MUA) much harder to inspect
>        manually. base64 is even more fool proof, but also even more opaque. auto will
>        use 8bit when possible, and quoted-printable otherwise.
> 
> 
> I copied my raw patch and was able to successfully apply it with git am, even with this escaping. Although I
> did upgrade to a newer version of git (2.23.0).
> 
> If I view the patch that you created, then it doesn't have quoted printable escaping. So there does
> seem to be a difference somewhere.
> Do you think I should use "git send-email --transfer-encoding=7bit"?

Well, I'm using mutt defaults, I'd say take a look at:

~/git/linux/Documentation/process/email-clients.rst

There is a sesion for your mail agent, maybe it helps.

- Arnaldo
 
> 
> Thanks
> James
> 
> On 29/10/2019 14:26, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Oct 29, 2019 at 11:18:52AM -0300, Arnaldo Carvalho de Melo escreveu:
> >> And here is the first patch out of your larger one, I changed the
> >> subject line to reflect that this is not tools/perf specific, as
> >> tools/objtool/ also uses libsubcmd, added Josh, objtool's maintainer so
> >> that he is made aware.
> > 
> > And the second patch:
> > 
> > 
> > commit d0381449fd9ab733ec2daf527263da9f73f1e94e
> > Author: James Clark <James.Clark@arm.com>
> > Date:   Mon Oct 28 11:34:01 2019 +0000
> > 
> >     libsubcmd: Use -O0 with DEBUG=1
> >     
> >     When a 'make DEBUG=1' build is done, the command parser is still built
> >     with -O6 and is hard to step through, fix it making it use -O0 in that
> >     case.
> >     
> >     Signed-off-by: James Clark <james.clark@arm.com>
> >     Cc: Adrian Hunter <adrian.hunter@intel.com>
> >     Cc: Ian Rogers <irogers@google.com>
> >     Cc: Jiri Olsa <jolsa@kernel.org>
> >     Cc: Josh Poimboeuf <jpoimboe@redhat.com>
> >     Cc: Namhyung Kim <namhyung@kernel.org>
> >     Cc: nd <nd@arm.com>
> >     Link: http://lore.kernel.org/lkml/20191028113340.4282-1-james.clark@arm.com
> >     [ split from a larger patch ]
> >     Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > 
> > diff --git a/tools/lib/subcmd/Makefile b/tools/lib/subcmd/Makefile
> > index 352c6062deba..1c777a72bb39 100644
> > --- a/tools/lib/subcmd/Makefile
> > +++ b/tools/lib/subcmd/Makefile
> > @@ -27,7 +27,9 @@ ifeq ($(DEBUG),0)
> >    endif
> >  endif
> >  
> > -ifeq ($(CC_NO_CLANG), 0)
> > +ifeq ($(DEBUG),1)
> > +  CFLAGS += -O0
> > +else ifeq ($(CC_NO_CLANG), 0)
> >    CFLAGS += -O3
> >  else
> >    CFLAGS += -O6
> > 

-- 

- Arnaldo

[tip: perf/core] libsubcmd: Use -O0 with DEBUG=1

[tip-bot2 for James Clark]

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     22bd8f1b5a1dd168ba4eba27cb17643a11012f5d
Gitweb:        https://git.kernel.org/tip/22bd8f1b5a1dd168ba4eba27cb17643a11012f5d
Author:        James Clark <James.Clark@arm.com>
AuthorDate:    Mon, 28 Oct 2019 11:34:01 
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Wed, 06 Nov 2019 15:49:39 -03:00

libsubcmd: Use -O0 with DEBUG=1

When a 'make DEBUG=1' build is done, the command parser is still built
with -O6 and is hard to step through, fix it making it use -O0 in that
case.

Signed-off-by: James Clark <james.clark@arm.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Ian Rogers <irogers@google.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: nd <nd@arm.com>
Link: http://lore.kernel.org/lkml/20191028113340.4282-1-james.clark@arm.com
[ split from a larger patch ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/subcmd/Makefile | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/lib/subcmd/Makefile b/tools/lib/subcmd/Makefile
index 352c606..1c777a7 100644
--- a/tools/lib/subcmd/Makefile
+++ b/tools/lib/subcmd/Makefile
@@ -27,7 +27,9 @@ ifeq ($(DEBUG),0)
   endif
 endif
 
-ifeq ($(CC_NO_CLANG), 0)
+ifeq ($(DEBUG),1)
+  CFLAGS += -O0
+else ifeq ($(CC_NO_CLANG), 0)
   CFLAGS += -O3
 else
   CFLAGS += -O6

[tip: perf/core] libsubcmd: Move EXTRA_FLAGS to the end to allow overriding existing flags

[tip-bot2 for James Clark]

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     d894967fcaa469cb4c43544855f6fcc18045d526
Gitweb:        https://git.kernel.org/tip/d894967fcaa469cb4c43544855f6fcc18045d526
Author:        James Clark <James.Clark@arm.com>
AuthorDate:    Mon, 28 Oct 2019 11:34:01 
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Wed, 06 Nov 2019 15:49:39 -03:00

libsubcmd: Move EXTRA_FLAGS to the end to allow overriding existing flags

Move EXTRA_WARNINGS and EXTRA_FLAGS to the end of the compilation line,
otherwise they cannot be used to override the default values.

Signed-off-by: James Clark <james.clark@arm.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Ian Rogers <irogers@google.com>
Cc: James Clark <james.clark@arm.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: nd <nd@arm.com>
Link: http://lore.kernel.org/lkml/20191028113340.4282-1-james.clark@arm.com
[ split from a larger patch ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/subcmd/Makefile | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/lib/subcmd/Makefile b/tools/lib/subcmd/Makefile
index 5b2cd5e..352c606 100644
--- a/tools/lib/subcmd/Makefile
+++ b/tools/lib/subcmd/Makefile
@@ -19,8 +19,7 @@ MAKEFLAGS += --no-print-directory
 
 LIBFILE = $(OUTPUT)libsubcmd.a
 
-CFLAGS := $(EXTRA_WARNINGS) $(EXTRA_CFLAGS)
-CFLAGS += -ggdb3 -Wall -Wextra -std=gnu99 -fPIC
+CFLAGS := -ggdb3 -Wall -Wextra -std=gnu99 -fPIC
 
 ifeq ($(DEBUG),0)
   ifeq ($(feature-fortify-source), 1)
@@ -43,6 +42,8 @@ CFLAGS += -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE
 
 CFLAGS += -I$(srctree)/tools/include/
 
+CFLAGS += $(EXTRA_WARNINGS) $(EXTRA_CFLAGS)
+
 SUBCMD_IN := $(OUTPUT)libsubcmd-in.o
 
 all: