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

Compiler inlining fixes #1880

Merged
merged 2 commits into from
Apr 6, 2021
Merged

Compiler inlining fixes #1880

merged 2 commits into from
Apr 6, 2021

Conversation

roman-khimov
Copy link
Member

Problem

This simple code:

// OracleCallback is called by Oracle contract when the request is finished.
func OracleCallback(url []byte, data []byte, code int, res []byte) {
        if code != 0 {
                panic("oracle request failed for " + string(url) + " with " + std.Itoa(code, 10))
        }
        runtime.Log("oracle result for " + string(url) + ":" + string(res))
}

Fails with

error encountered at instruction 1 (SYSCALL): error during call from native: error encountered at instruction 205 (STLOC2): runtime error: index out of range [2] with length 2

Which means that the code generated is just plain wrong because it initializes two local slots and then uses all three of them.

Solution

#1878 would be a better one, but this one suffices for now.

Some arguments can be inlined functions themselves thus requiring additional
attention. Otherwise we can get less local variables than really used by
STLOCs (and subsequent program crash).
@roman-khimov roman-khimov added this to the v0.94.1 milestone Apr 5, 2021
@roman-khimov roman-khimov requested a review from fyrchik April 5, 2021 20:08
pkg/compiler/func_scope.go Outdated Show resolved Hide resolved
Selector here is either a struct field access or global package
variable/constant. While technically none of these require an additional
local, inlining actually uses one, so add a test for it.
@codecov
Copy link

codecov bot commented Apr 6, 2021

Codecov Report

Merging #1880 (7e9f8e0) into master (dbcd628) will increase coverage by 0.07%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1880      +/-   ##
==========================================
+ Coverage   83.02%   83.09%   +0.07%     
==========================================
  Files         286      286              
  Lines       22693    22692       -1     
==========================================
+ Hits        18840    18856      +16     
+ Misses       2664     2645      -19     
- Partials     1189     1191       +2     
Impacted Files Coverage Δ
pkg/compiler/func_scope.go 100.00% <ø> (ø)
pkg/consensus/consensus.go 70.21% <0.00%> (-0.61%) ⬇️
pkg/network/server.go 70.94% <0.00%> (+0.14%) ⬆️
pkg/services/oracle/request.go 55.64% <0.00%> (+4.83%) ⬆️
pkg/services/oracle/oracle.go 90.69% <0.00%> (+11.62%) ⬆️
pkg/network/metrics/metrics.go 81.81% <0.00%> (+18.18%) ⬆️

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 dbcd628...7e9f8e0. Read the comment docs.

@roman-khimov roman-khimov requested a review from fyrchik April 6, 2021 09:08
@roman-khimov roman-khimov merged commit a44cd71 into master Apr 6, 2021
@roman-khimov roman-khimov deleted the compiler-inlining-fixes branch April 6, 2021 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants