Use html/template to produce source listing. (#831)

We previously used an ad-hoc combination of calls to printf and HTML escaping routines to produce HTML for source code listing. We now use html/template to do the formatting. This should give us HTML generation that is less susceptible to escaping issues. The change keeps the output very similar to the old output (mostly a small number of spacing changes that do not affect what a user can see). Also added a test that does a quick check of the source view displayed in a browser.
google · Feb 24, 2024 · 9dc2734 · 9dc2734
1 parent fb44976
commit 9dc2734
Show file tree

Hide file tree

Showing 11 changed files with 269 additions and 156 deletions.
diff --git a/internal/driver/browser_test.go b/internal/driver/browser_test.go
@@ -101,6 +101,28 @@ func TestFlameGraph(t *testing.T) {
 //go:embed testdata/testflame.js
 var jsCheckFlame string
 
+func TestSource(t *testing.T) {
+	maybeSkipBrowserTest(t)
+
+	prof := makeFakeProfile()
+	server := makeTestServer(t, prof)
+	ctx, cancel := context.WithTimeout(context.Background(), browserDeadline)
+	defer cancel()
+	ctx, cancel = chromedp.NewContext(ctx)
+	defer cancel()
+
+	err := chromedp.Run(ctx,
+		chromedp.Navigate(server.URL+"/source?f=F3"),
+		chromedp.WaitVisible(`#content`, chromedp.ByID),
+		matchRegexp(t, "#content", `F3`),            // Header
+		matchRegexp(t, "#content", `Total:.*100ms`), // Total for function
+		matchRegexp(t, "#content", `\b22\b.*100ms`), // Line 22
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+}
+
 // matchRegexp is a chromedp.Action that fetches the text of the first
 // node that matched query and checks that the text matches regexp re.
 func matchRegexp(t *testing.T, query, re string) chromedp.ActionFunc {

diff --git a/internal/driver/driver.go b/internal/driver/driver.go
@@ -20,6 +20,7 @@ package driver
 import (
 	"bytes"
 	"fmt"
+	"io"
 	"os"
 	"path/filepath"
 	"regexp"
@@ -118,7 +119,14 @@ func generateReport(p *profile.Profile, cmd []string, cfg config, o *plugin.Opti
 
 	// Generate the report.
 	dst := new(bytes.Buffer)
-	if err := report.Generate(dst, rpt, o.Obj); err != nil {
+	switch rpt.OutputFormat() {
+	case report.WebList:
+		// We need template expansion, so generate here instead of in report.
+		err = weblist(dst, rpt, o.Obj)
+	default:
+		err = report.Generate(dst, rpt, o.Obj)
+	}
+	if err != nil {
 		return err
 	}
 	src := dst
@@ -155,6 +163,18 @@ func generateReport(p *profile.Profile, cmd []string, cfg config, o *plugin.Opti
 	return out.Close()
 }
 
+func weblist(dst io.Writer, rpt *report.Report, obj plugin.ObjTool) error {
+	listing, err := report.MakeWebList(rpt, obj, -1)
+	if err != nil {
+		return err
+	}
+	legend := report.ProfileLabels(rpt)
+	return renderHTML(dst, "sourcelisting", rpt, nil, legend, webArgs{
+		Standalone: true,
+		Listing:    listing,
+	})
+}
+
 func applyCommandOverrides(cmd string, outputFormat int, cfg config) config {
 	// Some report types override the trim flag to false below. This is to make
 	// sure the default heuristics of excluding insignificant nodes and edges

diff --git a/internal/driver/html/source.html b/internal/driver/html/source.html
@@ -3,16 +3,70 @@
 <head>
   <meta charset="utf-8">
   <title>{{.Title}}</title>
-  {{template "css" .}}
+  {{if not .Standalone}}{{template "css" .}}{{end}}
   {{template "weblistcss" .}}
   {{template "weblistjs" .}}
 </head>
-<body>
-  {{template "header" .}}
-  <div id="content" class="source">
-    {{.HTMLBody}}
-  </div>
-  {{template "script" .}}
-  <script>viewer(new URL(window.location.href), null);</script>
+<body>{{"\n" -}}
+  {{/* emit different header in standalone mode */ -}}
+  {{if .Standalone}}{{"\n" -}}
+    <div class="legend">{{"" -}}
+      {{range $i, $e := .Legend -}}
+        {{if $i}}<br>{{"\n"}}{{end}}{{. -}}
+      {{end}}<br>Total: {{.Listing.Total -}}
+    </div>{{"" -}}
+  {{else -}}
+    {{template "header" .}}
+    <div id="content" class="source">{{"" -}}
+  {{end -}}
+
+  {{range .Listing.Files -}}
+    {{range .Funcs -}}
+      <h2>{{.Name}}</h2>{{"" -}}
+      <p class="filename">{{.File}}</p>{{"\n" -}}
+      <pre onClick="pprof_toggle_asm(event)">{{"\n" -}}
+        {{printf "  Total:  %10s %10s (flat, cum) %s" .Flat .Cumulative .Percent -}}
+        {{range .Lines -}}{{"\n" -}}
+          {{/* source line */ -}}
+          <span class=line>{{printf " %6d" .Line}}</span>{{" " -}}
+          <span class={{.HTMLClass}}>
+            {{- printf "  %10s %10s %8s  %s " .Flat .Cumulative "" .SrcLine -}}
+          </span>{{"" -}}
+
+          {{if .Instructions -}}
+            {{/* instructions for this source line */ -}}
+            <span class=asm>{{"" -}}
+            {{range .Instructions -}}
+              {{/* separate when we hit a new basic block */ -}}
+              {{if .NewBlock -}}{{printf " %8s %28s\n" "" "⋮"}}{{end -}}
+
+              {{/* inlined calls leading to this instruction */ -}}
+              {{range .InlinedCalls -}}
+                {{printf " %8s %10s %10s %8s  " "" "" "" "" -}}
+                <span class=inlinesrc>{{.SrcLine}}</span>{{" " -}}
+                <span class=unimportant>{{.FileBase}}:{{.Line}}</span>{{"\n" -}}
+              {{end -}}
+
+              {{if not .Synthetic -}}
+                {{/* disassembled instruction */ -}}
+                {{printf " %8s %10s %10s %8x: %s " "" .Flat .Cumulative .Address .Disasm -}}
+                <span class=unimportant>{{.FileLine}}</span>{{"\n" -}}
+              {{end -}}
+            {{end -}}
+            </span>{{"" -}}
+          {{end -}}
+          {{/* end of line */ -}}
+        {{end}}{{"\n" -}}
+      </pre>{{"\n" -}}
+      {{/* end of function */ -}}
+    {{end -}}
+    {{/* end of file */ -}}
+  {{end -}}
+
+  {{if not .Standalone}}{{"\n  " -}}
+    </div>{{"\n" -}}
+    {{template "script" .}}{{"\n" -}}
+    <script>viewer(new URL(window.location.href), null);</script>{{"" -}}
+  {{end}}
 </body>
 </html>
diff --git a/internal/driver/testdata/pprof.cpu.flat.addresses.weblist b/internal/driver/testdata/pprof.cpu.flat.addresses.weblist
@@ -1,10 +1,10 @@
-
 <!DOCTYPE html>
 <html>
 <head>
-<meta charset="UTF-8">
-<title>Pprof listing</title>
-<style type="text/css">
+  <meta charset="utf-8">
+  <title>testbinary cpu</title>
+
+  <style type="text/css">
 body #content{
 font-family: sans-serif;
 }
@@ -31,7 +31,7 @@ color: #008800;
 display: none;
 }
 </style>
-<script type="text/javascript">
+  <script type="text/javascript">
 function pprof_toggle_asm(e) {
   var target;
   if (!e) e = window.event;

diff --git a/internal/driver/webhtml.go b/internal/driver/webhtml.go
@@ -19,8 +19,27 @@ import (
 	"fmt"
 	"html/template"
 	"os"
+	"sync"
+
+	"github.com/google/pprof/internal/report"
+)
+
+var (
+	htmlTemplates    *template.Template // Lazily loaded templates
+	htmlTemplateInit sync.Once
 )
 
+// getHTMLTemplates returns the set of HTML templates used by pprof,
+// initializing them if necessary.
+func getHTMLTemplates() *template.Template {
+	htmlTemplateInit.Do(func() {
+		htmlTemplates = template.New("templategroup")
+		addTemplates(htmlTemplates)
+		report.AddSourceTemplates(htmlTemplates)
+	})
+	return htmlTemplates
+}
+
 //go:embed html
 var embeddedFiles embed.FS
 

diff --git a/internal/driver/webui.go b/internal/driver/webui.go
@@ -18,6 +18,7 @@ import (
 	"bytes"
 	"fmt"
 	"html/template"
+	"io"
 	"net"
 	"net/http"
 	gourl "net/url"
@@ -39,7 +40,6 @@ type webInterface struct {
 	copier       profileCopier
 	options      *plugin.Options
 	help         map[string]string
-	templates    *template.Template
 	settingsFile string
 }
 
@@ -48,15 +48,11 @@ func makeWebInterface(p *profile.Profile, copier profileCopier, opt *plugin.Opti
 	if err != nil {
 		return nil, err
 	}
-	templates := template.New("templategroup")
-	addTemplates(templates)
-	report.AddSourceTemplates(templates)
 	return &webInterface{
 		prof:         p,
 		copier:       copier,
 		options:      opt,
 		help:         make(map[string]string),
-		templates:    templates,
 		settingsFile: settingsFile,
 	}, nil
 }
@@ -82,11 +78,13 @@ type webArgs struct {
 	Total       int64
 	SampleTypes []string
 	Legend      []string
+	Standalone  bool // True for command-line generation of HTML
 	Help        map[string]string
 	Nodes       []string
 	HTMLBody    template.HTML
 	TextBody    string
 	Top         []report.TextItem
+	Listing     report.WebListData
 	FlameGraph  template.JS
 	Stacks      template.JS
 	Configs     []configMenuEntry
@@ -283,21 +281,25 @@ func (ui *webInterface) makeReport(w http.ResponseWriter, req *http.Request,
 	return rpt, catcher.errors
 }
 
-// render generates html using the named template based on the contents of data.
-func (ui *webInterface) render(w http.ResponseWriter, req *http.Request, tmpl string,
-	rpt *report.Report, errList, legend []string, data webArgs) {
+// renderHTML generates html using the named template based on the contents of data.
+func renderHTML(dst io.Writer, tmpl string, rpt *report.Report, errList, legend []string, data webArgs) error {
 	file := getFromLegend(legend, "File: ", "unknown")
 	profile := getFromLegend(legend, "Type: ", "unknown")
 	data.Title = file + " " + profile
 	data.Errors = errList
 	data.Total = rpt.Total()
-	data.SampleTypes = sampleTypes(ui.prof)
 	data.Legend = legend
+	return getHTMLTemplates().ExecuteTemplate(dst, tmpl, data)
+}
+
+// render responds with html generated by passing data to the named template.
+func (ui *webInterface) render(w http.ResponseWriter, req *http.Request, tmpl string,
+	rpt *report.Report, errList, legend []string, data webArgs) {
+	data.SampleTypes = sampleTypes(ui.prof)
 	data.Help = ui.help
 	data.Configs = configMenu(ui.settingsFile, *req.URL)
-
 	html := &bytes.Buffer{}
-	if err := ui.templates.ExecuteTemplate(html, tmpl, data); err != nil {
+	if err := renderHTML(html, tmpl, rpt, errList, legend, data); err != nil {
 		http.Error(w, "internal template error", http.StatusInternalServerError)
 		ui.options.UI.PrintErr(err)
 		return
@@ -410,16 +412,16 @@ func (ui *webInterface) source(w http.ResponseWriter, req *http.Request) {
 	}
 
 	// Generate source listing.
-	var body bytes.Buffer
-	if err := report.PrintWebList(&body, rpt, ui.options.Obj, maxEntries); err != nil {
+	listing, err := report.MakeWebList(rpt, ui.options.Obj, maxEntries)
+	if err != nil {
 		http.Error(w, err.Error(), http.StatusBadRequest)
 		ui.options.UI.PrintErr(err)
 		return
 	}
 
 	legend := report.ProfileLabels(rpt)
 	ui.render(w, req, "sourcelisting", rpt, errList, legend, webArgs{
-		HTMLBody: template.HTML(body.String()),
+		Listing: listing,
 	})
 }
 

diff --git a/internal/report/report.go b/internal/report/report.go
@@ -111,11 +111,10 @@ func Generate(w io.Writer, rpt *Report, obj plugin.ObjTool) error {
 		return printAssembly(w, rpt, obj)
 	case List:
 		return printSource(w, rpt)
-	case WebList:
-		return printWebSource(w, rpt, obj)
 	case Callgrind:
 		return printCallgrind(w, rpt)
 	}
+	// Note: WebList handling is in driver package.
 	return fmt.Errorf("unexpected output format")
 }
 
@@ -1327,6 +1326,9 @@ type Report struct {
 // Total returns the total number of samples in a report.
 func (rpt *Report) Total() int64 { return rpt.total }
 
+// OutputFormat returns the output format for the report.
+func (rpt *Report) OutputFormat() int { return rpt.options.OutputFormat }
+
 func abs64(i int64) int64 {
 	if i < 0 {
 		return -i

diff --git a/internal/report/report_test.go b/internal/report/report_test.go
@@ -112,10 +112,6 @@ func TestFilter(t *testing.T) {
 			name:   "list",
 			format: List,
 		},
-		{
-			name:   "weblist",
-			format: WebList,
-		},
 		{
 			name:   "disasm",
 			format: Dis,