Goal: Source mapping PC to TEAL #3726

algochoi · 2022-03-08T03:24:21Z

Summary

This is a PR generates a PC to TEAL source map file when goal clerk compile with the -m option is invoked.

Mostly shamelessly taken from this PR.

The source map follows this spec used in JS. The mappings are encoded using base64 VLQ and should be compatible with most browsers, including the tealdbg on CDT.

Example

myprog.teal

#pragma version 5

int 3
byte 0x03
btoi
==
assert

int 10
byte 0x10
btoi
==
assert

myprog.teal.map

{
  "version":3,
  "file":"",
  "sourceRoot":"",
  "sources":["myprog.teal"],
  "names":[],
  "mapping":";AAEA;;AAGA;;;AAIA;AAKA;AAMA;AAQA;;AASA;;;AAUA;AAWA;AAYA"
}

Example decoder here: https://github.com/algochoi/teal-sourcemap-decoder

myprog_annotated.teal

#pragma version 5

int 3            // PC: 1
byte 0x03        // PC: 3
btoi             // PC: 6
==               // PC: 7
assert           // PC: 8

int 10           // PC: 9
byte 0x10        // PC: 11
btoi             // PC: 14
==               // PC: 15
assert           // PC: 16

Test Plan

This also changes the goal clerk compile command, so any manual changes to documentation needs to be done as well.

codecov-commenter · 2022-03-17T19:36:18Z

Codecov Report

Merging #3726 (21aa4a1) into master (3dd1b6f) will increase coverage by 0.02%.
The diff coverage is 70.45%.

@@            Coverage Diff             @@
##           master    #3726      +/-   ##
==========================================
+ Coverage   50.10%   50.12%   +0.02%     
==========================================
  Files         393      394       +1     
  Lines       68382    68409      +27     
==========================================
+ Hits        34263    34292      +29     
+ Misses      30420    30413       -7     
- Partials     3699     3704       +5

Impacted Files	Coverage Δ
cmd/tealdbg/util.go	`84.61% <ø> (-5.13%)`	⬇️
cmd/goal/clerk.go	`9.01% <7.14%> (-0.03%)`	⬇️
cmd/tealdbg/debugger.go	`72.69% <100.00%> (-0.81%)`	⬇️
data/transactions/logic/sourcemap.go	`100.00% <100.00%> (ø)`
ledger/blockqueue.go	`82.18% <0.00%> (-2.88%)`	⬇️
data/transactions/verify/txn.go	`44.15% <0.00%> (-0.87%)`	⬇️
ledger/acctupdates.go	`68.51% <0.00%> (-0.66%)`	⬇️
catchup/service.go	`69.38% <0.00%> (-0.25%)`	⬇️
network/wsPeer.go	`68.05% <0.00%> (ø)`
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3dd1b6f...21aa4a1. Read the comment docs.

data/transactions/logic/sourcemap.go

algochoi · 2022-04-14T20:23:20Z

data/transactions/logic/sourcemap.go

@@ -0,0 +1,94 @@
+// Copyright (C) 2019-2022 Algorand, Inc.


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

cmd/tealdbg/util.go

data/transactions/logic/sourcemap_test.go

cmd/goal/clerk.go

algoidurovic

Looks good! As you can see, all my suggestions are nits so feel free to resolve anything you don't think is worthwhile to address.

algoidurovic · 2022-04-14T20:52:19Z

data/transactions/logic/sourcemap.go

+// 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 {


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."

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

algoidurovic · 2022-04-14T20:54:05Z

data/transactions/logic/sourcemap.go

+type SourceMap struct {
+	Version    int      `json:"version"`
+	File       string   `json:"file"`
+	SourceRoot string   `json:"sourceRoot"`


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.

Good point, added omitempty field for File and SourceRoot

algoidurovic · 2022-04-14T21:00:04Z

cmd/goal/clerk.go

@@ -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")


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.

algoidurovic · 2022-04-14T21:31:37Z

data/transactions/logic/sourcemap_test.go

+	"github.com/stretchr/testify/require"
+)
+
+func TestGetSourceMap(t *testing.T) {


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

cmd/goal/clerk.go

barnjamin · 2022-04-15T16:44:23Z

Looks pretty good to me, I'd be interested in a follow up pr to have the compile -D accept a source map as input as well as the binary to reproduce the original. That might require saving original labels and comments though which I guess would be part of the Extensions bit of the spec.

test/scripts/e2e_subs/e2e-teal.sh

jasonpaulos · 2022-04-20T21:37:10Z

cmd/goal/clerk.go

@@ -1056,6 +1069,17 @@ var compileCmd = &cobra.Command{
 					reportErrorf("%s: %s", outname, err)
 				}
 			}
+			if writeSourceMap {
+				mapname := fname + ".map"


@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?).

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.

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.

Start prototype of PC source map

d2dd036

algochoi mentioned this pull request Mar 8, 2022

Source mapping: PC->TEAL map #3725

Closed

michaeldiamant added the Team Scytale label Mar 8, 2022

Refactor source map struct

f00862f

Add some tests for source maps

9279f9c

michaeldiamant reviewed Apr 11, 2022

View reviewed changes

data/transactions/logic/sourcemap.go Show resolved Hide resolved

Merge branch 'master' into assemble-sourcemap

b1ee70f

algochoi added the New Feature label Apr 14, 2022

algochoi changed the title ~~Source mapping: PC to TEAL Draft~~ Source mapping: PC to TEAL Apr 14, 2022

algochoi commented Apr 14, 2022

View reviewed changes

algochoi marked this pull request as ready for review April 14, 2022 20:24

algochoi requested review from algoidurovic and barnjamin April 14, 2022 20:25

Move VLQ encoding into logic package and delete in tealdbg dir

bf9fc02

michaeldiamant reviewed Apr 14, 2022

View reviewed changes

cmd/tealdbg/util.go Outdated Show resolved Hide resolved

Remove redundant function in tealdbg

6ddc4d1

michaeldiamant reviewed Apr 14, 2022

View reviewed changes

data/transactions/logic/sourcemap_test.go Show resolved Hide resolved

michaeldiamant reviewed Apr 14, 2022

View reviewed changes

cmd/goal/clerk.go Show resolved Hide resolved

algoidurovic reviewed Apr 14, 2022

View reviewed changes

michaeldiamant self-requested a review April 14, 2022 21:40

michaeldiamant reviewed Apr 14, 2022

View reviewed changes

cmd/goal/clerk.go Outdated Show resolved Hide resolved

barnjamin reviewed Apr 15, 2022

View reviewed changes

cmd/goal/clerk.go Outdated Show resolved Hide resolved

Fix some tests and mark some fields as optional

6645cb5

barnjamin previously approved these changes Apr 15, 2022

View reviewed changes

Add e2e shell test and fix typo in existing test

17d9b9c

algochoi dismissed barnjamin’s stale review via 17d9b9c April 19, 2022 22:08

michaeldiamant reviewed Apr 20, 2022

View reviewed changes

test/scripts/e2e_subs/e2e-teal.sh Outdated Show resolved Hide resolved

Merge branch 'master' into assemble-sourcemap

3615d00

Retain diff and use trap to clean up test files

21aa4a1

michaeldiamant approved these changes Apr 20, 2022

View reviewed changes

michaeldiamant merged commit 0b59e09 into algorand:master Apr 20, 2022

jasonpaulos reviewed Apr 20, 2022

View reviewed changes

algochoi mentioned this pull request Apr 21, 2022

goal: Rename source map output file #3905

Merged

egieseke mentioned this pull request Apr 22, 2022

go-algorand 3.6.0-beta Release PR #3909

Closed

Algo-devops-service mentioned this pull request Apr 26, 2022

go-algorand 0.0.0-beta Release PR #3922

Closed

onetechnical mentioned this pull request Apr 27, 2022

go-algorand 3.6.0-beta Release PR #3923

Merged

onetechnical changed the title ~~Source mapping: PC to TEAL~~ Goal: Source mapping PC to TEAL Apr 27, 2022

barnjamin mentioned this pull request Apr 28, 2022

TEAL - Produce Assembly Map During Compilation algorand-devrel/community#17

Closed

onetechnical mentioned this pull request Apr 28, 2022

go-algorand 3.6.0-beta Release PR #3934

Merged

This was referenced May 2, 2022

go-algorand 3.6.1-beta Release PR #3943

Merged

go-algorand 3.6.2-beta Release PR #3947

Merged

Algo-devops-service mentioned this pull request May 7, 2022

go-algorand 3.6.2-stable Release PR #3960

Merged

This was referenced Jun 27, 2022

Dev Tools: Source map decoder algorand/py-algorand-sdk#353

Merged

Sourcemap: Support relative segment field values #4191

Closed

Sourcemap: Expose mappings field #4192

Closed

algochoi deleted the assemble-sourcemap branch November 9, 2022 19:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Goal: Source mapping PC to TEAL #3726

Goal: Source mapping PC to TEAL #3726

algochoi commented Mar 8, 2022 •

edited

Loading

codecov-commenter commented Mar 17, 2022 •

edited

Loading

algochoi Apr 14, 2022

algoidurovic left a comment

algoidurovic Apr 14, 2022

algochoi Apr 15, 2022

algoidurovic Apr 14, 2022

algochoi Apr 15, 2022

algoidurovic Apr 14, 2022 •

edited

Loading

algoidurovic Apr 14, 2022

barnjamin commented Apr 15, 2022

jasonpaulos Apr 20, 2022

algochoi Apr 20, 2022

jasonpaulos Apr 20, 2022

Goal: Source mapping PC to TEAL #3726

Goal: Source mapping PC to TEAL #3726

Conversation

algochoi commented Mar 8, 2022 • edited Loading

Summary

Example

Test Plan

codecov-commenter commented Mar 17, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

algoidurovic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

algoidurovic Apr 14, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

barnjamin commented Apr 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

algochoi commented Mar 8, 2022 •

edited

Loading

codecov-commenter commented Mar 17, 2022 •

edited

Loading

algoidurovic Apr 14, 2022 •

edited

Loading