6l: what is the correct way to suppress or workaround warnings?

28 views
Skip to first unread message

j...@webmaster.ms

unread,
Aug 28, 2011, 10:54:12 PM8/28/11
to golang-dev
Hellu

I submitted a patch for windows/amd64 (http://codereview.appspot.com/
4958042/diff/9011/src/pkg/runtime/windows/amd64/sys.s)

This patch has assembler code which causes two 6l warnings.


First, my procedure has more POPs than PUSHes.
That's OK, it is not a procedure designed to be called from C code,
this code is to deal with callback thunks.

6l warns it as "unbalanced PUSH/POP"
It can be easily workaround as:
MOVQ 0(SP), DX // POPQ DX
MOVQ 8(SP), AX // POPQ AX
ADDQ $16, SP
But this causes reviewer's warnings "why not just POP?":
http://codereview.appspot.com/4958042/diff/9011/src/pkg/runtime/windows/amd64/sys.s
Also I do not like this kind of workaround as it makes code bigger.
Source code, binary code and comments to explain what is if for and
why not just POP)


Second, the procedure needs to save 8 64-byte registers to follow
windows/amd64 calling convention.
If I wrote 8 PUSHQ and 8 POPQ, 6l warns

runtime.callbackasm: nosplit stack overflow
120 assumed on entry to runtime.callbackasm
40 after runtime.callbackasm uses 80
32 on entry to runtime.cgocallback
0 after runtime.cgocallback uses 32
-8 on entry to runtime.cgocallbackg

The procedure is executed on OS thread stack, which is virtually
unlimited.
If I workaround if with MOVQ, ... then you know the story :)

So, how to deal with the 6l warnings?
Even if 6l warns, not fails, it causes building process to stop.
So I cannot just ignore them.

Russ Cox

unread,
Aug 28, 2011, 11:11:06 PM8/28/11
to j...@webmaster.ms, golang-dev
The correct way to avoid the errors is
to write code that doesn't contain the
errors 6l complains about. Yes, this is
a little limiting, but it is always possible.

Russ

j...@webmaster.ms

unread,
Aug 28, 2011, 11:57:09 PM8/28/11
to golang-dev
It is not only possible, but it is possible in at least three ways
But no one of them is good.

1. Write bigger code doing exactly the same.
Only difference that 6l does not complain.
Disadvantage A: it confuses people who read the code
Disadvantage B: it is slower and bigger code
Disadvantage C: improved 6l may warn on this code tomorrow, at the
code does the same thing, it allocates 64 bytes on stack. It is a
foxable 6l fault that it warns about PUSHQ but does not warn about SUB
$..., SP

2. Write same binary code using BYTE $0xopcodes
Disadvantage A: it confuses people very much.

3. Rethink code, change stack allocation to heap allocation, etc
Disadvantage A: it confuses people who read the code
Disadvantage B: it is slower and bigger code

Russ Cox

unread,
Aug 29, 2011, 10:31:31 AM8/29/11
to j...@webmaster.ms, golang-dev
6l diagnoses push/pop mismatches because they are
easy to get wrong. If you need to do something non-standard,
then the cost of the diagnosis is that you have to write an
extra instruction or two. For example,

// Pop arguments from callback;
// hide stack adjustment from 6l
MOVQ SP, AX
MOVQ $16(AX), SP
MOVQ 0(SP), DX // argsize
MOVQ 8(SP), AX // fn

instead of

POPQ DX
POPQ AX

In the grand scheme of things, that's not so bad.

Russ

Reply all
Reply to author
Forward
0 new messages