Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Goal: Source mapping PC to TEAL #3726

Merged
merged 10 commits into from
Apr 20, 2022
28 changes: 26 additions & 2 deletions cmd/goal/clerk.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package main

import (
"encoding/base64"
"encoding/json"
"fmt"
"io"
"os"
Expand Down Expand Up @@ -49,6 +50,7 @@ var (
rejectsFilename string
closeToAddress string
noProgramOutput bool
writeSourceMap bool
signProgram bool
programSource string
argB64Strings []string
Expand Down Expand Up @@ -123,6 +125,7 @@ func init() {

compileCmd.Flags().BoolVarP(&disassemble, "disassemble", "D", false, "disassemble a compiled program")
compileCmd.Flags().BoolVarP(&noProgramOutput, "no-out", "n", false, "don't write contract program binary")
compileCmd.Flags().BoolVarP(&writeSourceMap, "map", "m", false, "write out assembly map")
Copy link
Contributor

@algoidurovic algoidurovic Apr 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think it'd be nice if the source map format version was included here or somewhere else in the documentation easily accessible to the user.

compileCmd.Flags().BoolVarP(&signProgram, "sign", "s", false, "sign program, output is a binary signed LogicSig record")
compileCmd.Flags().StringVarP(&outFilename, "outfile", "o", "", "Filename to write program bytes or signed LogicSig to")
compileCmd.Flags().StringVarP(&account, "account", "a", "", "Account address to sign the program (If not specified, uses default account)")
Expand Down Expand Up @@ -935,7 +938,7 @@ func mustReadFile(fname string) []byte {
return contents
}

func assembleFile(fname string, printWarnings bool) (program []byte) {
func assembleFileImpl(fname string, printWarnings bool) *logic.OpStream {
text, err := readFile(fname)
if err != nil {
reportErrorf("%s: %s", fname, err)
Expand Down Expand Up @@ -967,9 +970,19 @@ func assembleFile(fname string, printWarnings bool) (program []byte) {
reportWarnRawf("%d warning%s", len(ops.Warnings), plural)
}

return ops
}

func assembleFile(fname string, printWarnings bool) (program []byte) {
ops := assembleFileImpl(fname, printWarnings)
return ops.Program
}

func assembleFileWithMap(fname string, printWarnings bool) ([]byte, logic.SourceMap) {
ops := assembleFileImpl(fname, printWarnings)
return ops.Program, logic.GetSourceMap([]string{fname}, ops.OffsetToLine)
}

func disassembleFile(fname, outname string) {
program, err := readFile(fname)
if err != nil {
Expand Down Expand Up @@ -1025,7 +1038,7 @@ var compileCmd = &cobra.Command{
}
}
shouldPrintAdditionalInfo := outname != stdoutFilenameValue
program := assembleFile(fname, true)
program, sourceMap := assembleFileWithMap(fname, true)
outblob := program
if signProgram {
dataDir := ensureSingleDataDir()
Expand Down Expand Up @@ -1056,6 +1069,17 @@ var compileCmd = &cobra.Command{
reportErrorf("%s: %s", outname, err)
}
}
if writeSourceMap {
mapname := fname + ".map"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@algochoi small question/request, was there a specific reason you chose fname + ".map" for the map file name? With this the source map is given the name of the input file, which I think is less intuitive/accurate.

In JS it's common practice for the map file to be named after the produced output file, since that's the starting point from which you can go "backwards" with the map, so I was expecting outname + ".map" here (idk what should happen when the output is stdout, maybe an error?).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this the source map is given the name of the input file, which I think is less intuitive/accurate.

That is a fair point, and I can see the confusion. I can open up a PR to fix this.

idk what should happen when the output is stdout, maybe an error?

This what I initially thought, but after playing around with the commands, I think the source map should always be output to the file because the no-out flag says don't write contract program binary, and a source map isn't necessarily the binary.

But alternatively, we could also output the source map to stdout if the no output flag is raised. That sounds fine too, depending on how you interpret the no-out flag.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a fair point, and I can see the confusion. I can open up a PR to fix this.

That would be great, thanks!

I think the source map should always be output to the file because the no-out flag says don't write contract program binary, and a source map isn't necessarily the binary.

That's definitely a valid interpretation, but what do you name the source map file then? I think <input>.tok.map would work.

Though to be fair, I think it's quite rare to use the no-out flag, so failing in this case would not be that bad in my opinion.

pcblob, err := json.Marshal(sourceMap)
if err != nil {
reportErrorf("%s: %s", mapname, err)
}
err = writeFile(mapname, pcblob, 0666)
if err != nil {
reportErrorf("%s: %s", mapname, err)
}
}
if !signProgram && shouldPrintAdditionalInfo {
pd := logic.HashProgram(program)
addr := basics.Address(pd)
Expand Down
2 changes: 1 addition & 1 deletion data/transactions/logic/assembler.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ func (ops *OpStream) RecordSourceLine() {
ops.OffsetToLine[ops.pending.Len()] = ops.sourceLine - 1
}

// ReferToLabel records an opcode label refence to resolve later
// ReferToLabel records an opcode label reference to resolve later
func (ops *OpStream) ReferToLabel(pc int, label string) {
ops.labelReferences = append(ops.labelReferences, labelReference{ops.sourceLine, pc, label})
}
Expand Down
94 changes: 94 additions & 0 deletions data/transactions/logic/sourcemap.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
// Copyright (C) 2019-2022 Algorand, Inc.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also thought of putting this file in the tealdbg package, but I don't think it can be exported from there.

// This file is part of go-algorand
//
// go-algorand is free software: you can redistribute it and/or modify
// it under the terms of the GNU Affero General Public License as
// published by the Free Software Foundation, either version 3 of the
// License, or (at your option) any later version.
//
// go-algorand is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU Affero General Public License for more details.
//
// You should have received a copy of the GNU Affero General Public License
// along with go-algorand. If not, see <https://www.gnu.org/licenses/>.

package logic

import (
"bytes"
"strings"
)

// sourceMapVersion is currently 3: https://sourcemaps.info/spec.html
const sourceMapVersion = 3
const b64table string = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"

// SourceMap contains details from the source to assembly process
// currently contains map of TEAL source line to assembled bytecode position
// and details about the template variables contained in the source file
type SourceMap struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I had to check the spec for some of the fields here so it might be useful to describe the fields in a comment.

Spec:
"Line 1: The entire file is a single JSON object
Line 2: File version (always the first entry in the object) and must be a positive integer.
Line 3: An optional name of the generated code that this source map is associated with.
Line 4: An optional source root, useful for relocating source files on a server or removing repeated values in the “sources” entry. This value is prepended to the individual entries in the “source” field.
Line 5: A list of original sources used by the “mappings” entry.
Line 6: A list of symbol names used by the “mappings” entry.
Line 7: A string with the encoded mapping data in base64 VLQ encoding. Each semicolon separates a line in the generated code. Note that multiple lines in the generated code may point to the same line in the source code."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point - I eventually settled on linking the sourcemap specs URL instead in the comments, so the interested user can look at the original doc in more detail. Linking to URLs also might

Version int `json:"version"`
File string `json:"file"`
SourceRoot string `json:"sourceRoot"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we might want to make some of these fields optional in the json serialization depending on how closely we want to follow the spec.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, added omitempty field for File and SourceRoot

Sources []string `json:"sources"`
Names []string `json:"names"`
Mapping string `json:"mapping"`
}

// GetSourceMap returns a struct containing details about
// the assembled file and encoded mappings to the source file.
func GetSourceMap(sourceNames []string, offsetToLine map[int]int) SourceMap {
maxPC := 0
for pc := range offsetToLine {
if pc > maxPC {
maxPC = pc
}
}

// Array where index is the PC and value is the line.
pcToLine := make([]string, maxPC+1)
for pc := range pcToLine {
if line, ok := offsetToLine[pc]; ok {
pcToLine[pc] = MakeSourceMapLine(0, 0, line, 0)
} else {
pcToLine[pc] = ""
}
}

// Encode the source map into a string
encodedMapping := strings.Join(pcToLine, ";")

return SourceMap{
Version: sourceMapVersion,
File: "", // Assembled file does not have a name.
Sources: sourceNames,
Names: []string{}, // TEAL code does not generate any names.
Mapping: encodedMapping,
}
}

// IntToVLQ writes out value to bytes.Buffer
func IntToVLQ(v int, buf *bytes.Buffer) {
v <<= 1
if v < 0 {
v = -v
v |= 1
}
for v >= 32 {
buf.WriteByte(b64table[32|(v&31)])
v >>= 5
}
buf.WriteByte(b64table[v])
}

// MakeSourceMapLine creates source map mapping's line entry
func MakeSourceMapLine(tcol, sindex, sline, scol int) string {
buf := bytes.NewBuffer(nil)
IntToVLQ(tcol, buf)
IntToVLQ(sindex, buf)
IntToVLQ(sline, buf)
IntToVLQ(scol, buf)
return buf.String()
}
63 changes: 63 additions & 0 deletions data/transactions/logic/sourcemap_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// Copyright (C) 2019-2022 Algorand, Inc.
// This file is part of go-algorand
//
// go-algorand is free software: you can redistribute it and/or modify
// it under the terms of the GNU Affero General Public License as
// published by the Free Software Foundation, either version 3 of the
// License, or (at your option) any later version.
//
// go-algorand is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU Affero General Public License for more details.
//
// You should have received a copy of the GNU Affero General Public License
// along with go-algorand. If not, see <https://www.gnu.org/licenses/>.

package logic

import (
"strings"
"testing"

"github.com/algorand/go-algorand/test/partitiontest"
"github.com/stretchr/testify/require"
)

func TestGetSourceMap(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this test doesn't cover the (unlikely) edge case where the line number is large enough that a left shift turns it negative.

partitiontest.PartitionTest(t)
a := require.New(t)

sourceNames := []string{"test.teal"}
offsetToLine := map[int]int{
1: 1,
2: 2,
3: 5,
}
actualSourceMap := GetSourceMap(sourceNames, offsetToLine)

a.Equal(sourceMapVersion, actualSourceMap.Version)
a.Equal("", actualSourceMap.File)
a.Equal(sourceNames, actualSourceMap.Sources)
a.Equal([]string{}, actualSourceMap.Names)

// Check encoding for each line.
splitMapping := strings.Split(actualSourceMap.Mapping, ";")
for pc := range splitMapping {
if line, ok := offsetToLine[pc]; ok {
a.Equal(MakeSourceMapLine(0, 0, line, 0), splitMapping[pc])
}
}
}

func TestVLQ(t *testing.T) {
partitiontest.PartitionTest(t)
a := require.New(t)

a.Equal("AAAA", MakeSourceMapLine(0, 0, 0, 0))
a.Equal("AACA", MakeSourceMapLine(0, 0, 1, 0))
a.Equal("AAEA", MakeSourceMapLine(0, 0, 2, 0))
a.Equal("AAgBA", MakeSourceMapLine(0, 0, 16, 0))
a.Equal("AAggBA", MakeSourceMapLine(0, 0, 512, 0))
a.Equal("ADggBD", MakeSourceMapLine(0, -1, 512, -1))
}