-
Notifications
You must be signed in to change notification settings - Fork 91
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
pass: ensure frame pointer register is saved #174
Conversation
@@ -145,6 +145,7 @@ type Info uint8 | |||
const ( | |||
None Info = 0 | |||
Restricted Info = 1 << iota | |||
BasePointer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this is the right property name. An alternative is to call it something like AutoSaveRestore
. The distinction is naming it for what it is vs. what the Go assembler does with it.
} | ||
|
||
if fn.LocalSize == 0 { | ||
fn.AllocLocal(int(gotypes.PointerSize)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason adding pointer size seemed like the right idea. But should this just be fn.AllocLocal(1)
instead?
1f7a734
to
48aa0b3
Compare
48aa0b3
to
af77c25
Compare
Codecov Report
@@ Coverage Diff @@
## master #174 +/- ##
=======================================
Coverage 79.49% 79.50%
=======================================
Files 56 56
Lines 30907 30919 +12
=======================================
+ Hits 24569 24581 +12
Misses 6192 6192
Partials 146 146
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Currently
avo
usesBP
as a standard general purpose register. However,BP
is used for the frame pointer and should be callee-save. Under some circumstances, the Go assembler will do this automatically, but not always. At the momentavo
can produce code that clobbers theBP
register. Since Go 1.16 this code will also fail a newgo vet
check.This PR provides a (currently sub-optimal) fix for the issue. It introduces an
EnsureBasePointerCalleeSaved
pass which will check if the base pointer is written to by a function, and if so will artificially ensure that the function has a non-zero frame size. This will trigger the Go assembler to automatically save and restore the BP register.In addition, we update the
asmdecl
tool toasmvet
, which includes theframepointer
vet check.Updates #156