code review 73990043: cmd/ld: restore the call graph dump

cmd/ld: restore the call graph dump

Before the switch to liblink, the linkers accepted the -c flag
to print the call graph. This change restores the functionality.

This came in handy when I was trying to audit the use of SSE
instructions inside the Plan 9 note handler.

[ality, 2014-03-11 10:37:14]

Hello golang-codereviews@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go

[bradfitz, 2014-03-11 17:32:23]

R=rsc


On Tue, Mar 11, 2014 at 3:37 AM, <ality@pbrane.org> wrote:

> Reviewers: golang-codereviews,
>
> Message:
> Hello golang-codereviews@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> cmd/gc, cmd/cc: add -callgraph command to dump the call graph
>
> Before the switch to liblink, the linkers accepted the -c flag
> to print the call graph. This change restores the functionality
> but the flags now have to be given to the compilers.
>
> This came in handy when I was trying to audit the use of SSE
> instructions inside the Plan 9 note handler.
>
> Please review this at https://codereview.appspot.com/73990043/
>
> Affected files (+19, -4 lines):
>   M include/link.h
>   M src/cmd/cc/cc.h
>   M src/cmd/cc/lex.c
>   M src/cmd/gc/go.h
>   M src/cmd/gc/lex.c
>   M src/cmd/ld/pobj.c
>   M src/liblink/obj5.c
>   M src/liblink/obj6.c
>   M src/liblink/obj8.c
>
>
> Index: include/link.h
> ===================================================================
> --- a/include/link.h
> +++ b/include/link.h
> @@ -332,6 +332,7 @@
>         int32   debugdivmod;    // -M flag in 5l
>         int32   debugfloat;     // -F flag in 5l
>         int32   debugpcln;      // -O flag in linker
> +       int32   debugcalls;     // -callgraph flag in compilers
>         int32   flag_shared;    // -shared flag in linker
>         int32   iself;
>         Biobuf* bso;    // for -v flag
> Index: src/cmd/cc/cc.h
> ===================================================================
> --- a/src/cmd/cc/cc.h
> +++ b/src/cmd/cc/cc.h
> @@ -521,6 +521,7 @@
>  EXTERN int     textflag;
>  EXTERN int     dataflag;
>  EXTERN int     flag_largemodel;
> +EXTERN int     flag_callgraph;
>  EXTERN int     ncontin;
>  EXTERN int     canreach;
>  EXTERN int     warnreach;
> Index: src/cmd/cc/lex.c
> ===================================================================
> --- a/src/cmd/cc/lex.c
> +++ b/src/cmd/cc/lex.c
> @@ -199,9 +199,11 @@
>         flagcount("v", "increase debug verbosity", &debug['v']);
>         if(thechar == '6')
>                 flagcount("largemodel", "generate code that assumes a
> large memory model", &flag_largemodel);
> +       flagcount("callgraph", "dump call graph", &flag_callgraph);
>
>         flagparse(&argc, &argv, usage);
>         ctxt->debugasm = debug['S'];
> +       ctxt->debugcalls = flag_callgraph;
>
>         if(argc < 1 && outfile == 0)
>                 usage();
> Index: src/cmd/gc/go.h
> ===================================================================
> --- a/src/cmd/gc/go.h
> +++ b/src/cmd/gc/go.h
> @@ -974,6 +974,7 @@
>  EXTERN char*   flag_installsuffix;
>  EXTERN int     flag_race;
>  EXTERN int     flag_largemodel;
> +EXTERN int     flag_callgraph;
>  EXTERN int     noescape;
>  EXTERN int     debuglive;
>  EXTERN Link*   ctxt;
> Index: src/cmd/gc/lex.c
> ===================================================================
> --- a/src/cmd/gc/lex.c
> +++ b/src/cmd/gc/lex.c
> @@ -315,9 +315,11 @@
>         flagcount("y", "debug declarations in canned imports (with -d)",
> &debug['y']);
>         if(thechar == '6')
>                 flagcount("largemodel", "generate code that assumes a
> large memory model", &flag_largemodel);
> +       flagcount("callgraph", "dump call graph", &flag_callgraph);
>
>         flagparse(&argc, &argv, usage);
>         ctxt->debugasm = debug['S'];
> +       ctxt->debugcalls = flag_callgraph;
>
>         if(argc < 1)
>                 usage();
> Index: src/cmd/ld/pobj.c
> ===================================================================
> --- a/src/cmd/ld/pobj.c
> +++ b/src/cmd/ld/pobj.c
> @@ -94,7 +94,6 @@
>         flagfn2("X", "name value: define string data", addstrdata);
>         flagcount("Z", "clear stack frame on entry", &debug['Z']);
>         flagcount("a", "disassemble output", &debug['a']);
> -       flagcount("c", "dump call graph", &debug['c']);
>         flagcount("d", "disable dynamic executable", &debug['d']);
>         flagstr("extld", "linker to run in external mode", &extld);
>         flagstr("extldflags", "flags for external linker", &extldflags);
> Index: src/liblink/obj5.c
> ===================================================================
> --- a/src/liblink/obj5.c
> +++ b/src/liblink/obj5.c
> @@ -101,8 +101,11 @@
>         switch(p->as) {
>         case AB:
>         case ABL:
> -               if(p->to.type == D_OREG && (p->to.name == D_EXTERN || p->
> to.name == D_STATIC) && p->to.sym != nil)
> +               if(p->to.type == D_OREG && (p->to.name == D_EXTERN || p->
> to.name == D_STATIC) && p->to.sym != nil) {
> +                       if(ctxt->debugcalls)
> +                               Bprint(ctxt->bso, "%s calls %s\n",
> ctxt->cursym->name, p->to.sym->name);
>                         p->to.type = D_BRANCH;
> +               }
>                 break;
>         }
>
> Index: src/liblink/obj6.c
> ===================================================================
> --- a/src/liblink/obj6.c
> +++ b/src/liblink/obj6.c
> @@ -230,8 +230,11 @@
>         case ACALL:
>         case AJMP:
>         case ARET:
> -               if((p->to.type == D_EXTERN || p->to.type == D_STATIC) &&
> p->to.sym != nil)
> +               if((p->to.type == D_EXTERN || p->to.type == D_STATIC) &&
> p->to.sym != nil) {
> +                       if(ctxt->debugcalls)
> +                               Bprint(ctxt->bso, "%s calls %s\n",
> ctxt->cursym->name, p->to.sym->name);
>                         p->to.type = D_BRANCH;
> +               }
>                 break;
>         }
>
> Index: src/liblink/obj8.c
> ===================================================================
> --- a/src/liblink/obj8.c
> +++ b/src/liblink/obj8.c
> @@ -170,8 +170,11 @@
>         case ACALL:
>         case AJMP:
>         case ARET:
> -               if((p->to.type == D_EXTERN || p->to.type == D_STATIC) &&
> p->to.sym != nil)
> +               if((p->to.type == D_EXTERN || p->to.type == D_STATIC) &&
> p->to.sym != nil) {
> +                       if(ctxt->debugcalls)
> +                               Bprint(ctxt->bso, "%s calls %s\n",
> ctxt->cursym->name, p->to.sym->name);
>                         p->to.type = D_BRANCH;
> +               }
>                 break;
>         }
>
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

[rsc, 2014-03-12 12:09:46]

I'd prefer to keep this in the linker. You can implement it by: (1) wait
until deadcode has run, (2) for each sym in textp list, (3) for each reloc
in sym's reloc list; (4) if reloc is itself a text sym with type D_PCREL,
print the message.

[ality, 2014-03-12 12:15:44]

Russ Cox <rsc@golang.org> once said:
> I'd prefer to keep this in the linker. You can implement it by: (1) wait
> until deadcode has run, (2) for each sym in textp list, (3) for each reloc
> in sym's reloc list; (4) if reloc is itself a text sym with type D_PCREL,
> print the message.

Okay, I'll try that instead.

Thanks,
  Anthony

[ality, 2014-03-14 21:59:38]

Hello golang-codereviews@googlegroups.com, bradfitz@golang.org, rsc@golang.org
(cc: golang-codereviews@googlegroups.com),

Please take another look.

[rsc, 2014-03-24 18:58:41]

LGTM

Much nicer, thanks.

[rsc, 2014-04-17 02:42:06]

*** Submitted as https://code.google.com/p/go/source/detail?r=5afa3eb2db7c ***

cmd/ld: restore the call graph dump

Before the switch to liblink, the linkers accepted the -c flag
to print the call graph. This change restores the functionality.

This came in handy when I was trying to audit the use of SSE
instructions inside the Plan 9 note handler.

LGTM=rsc
R=golang-codereviews, bradfitz, rsc
CC=golang-codereviews
https://codereview.appspot.com/73990043

Committer: Russ Cox <rsc@golang.org>