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

proposal: runtime: make the ABI undefined as a step toward changing it #27539

Closed
aclements opened this issue Sep 6, 2018 · 62 comments

Comments

Projects
None yet
@aclements
Copy link
Member

commented Sep 6, 2018

The are various changes we'd like to make (or explore making) to the Go ABI, such as changing to a register-based ABI (#18597), propagating register clobbers up the call graph to avoid unnecessary spills, and passing dynamic information about allocation scopes. However, every attempt we've made to modify the calling convention has encountered major hurdles with calling back and forth between Go and assembly code.

This proposal is not about a specific change to the ABI, but about how to get to a place where we have the freedom to change the ABI. It was in part inspired by how Rust handles this issue.

I propose:

  1. We make the default Go function calling convention officially unspecified.
  2. We provide a pragma for marking the rare functions that need a stable ABI (specifically, the current stack-based ABI). We officially document the stack ABI (including the need to participate in stack growth).
  3. For a release or two, we keep the unspecified ABI the same as the stack ABI.
  4. We define the CALL assembly instruction to call via the stack ABI and modify vet to check for ABI violations, where assembly code is CALLing a function using the undefined ABI. (We may also need a way to call via the unspecified ABI in the runtime for closure calls, though ideally I'd like to minimize this even in the runtime.)
  5. After a few releases, we have the freedom to modify the unspecified ABI. We move the ABI check into the linker itself.

This would involve supporting potentially two ABIs for the rest of time, but I believe that's actually a strength. This approach provides a stable ABI for the rare cases that need it, while simultaneously giving us as compiler writers the freedom to evolve the ABI in the vast majority of cases that don't need a stable ABI.

/cc @dr2chase @randall77 @ianlancetaylor @josharian

@aclements aclements added the Proposal label Sep 6, 2018

@aclements aclements added this to the Proposal milestone Sep 6, 2018

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Sep 6, 2018

Can we also write pseudo-ops for assembly code so that assembly code can become ABI agnostic?

E.g., ENTER could look at the Go function declaration to move all parameters into appropriate registers, or leave them on the stack with a register pointing to them. It would also save all callee-saved registers. EXIT would move appropriate registers into the correct return location, and restore registers saved by ENTER

@aclements

This comment has been minimized.

Copy link
Member Author

commented Sep 6, 2018

Can we also write pseudo-ops for assembly code so that assembly code can become ABI agnostic?

Potentially. I worry that, while this could certainly be more flexible than what we have now, and is probably powerful enough to adapt to a register-based ABI, it may still tie us to specific design decisions. Some of this, I think, is just fundamental to assembly (or pseudo-assembly) language; for example, if a calling convention change requires more stack space (like passing dynamic allocation scope information), what do we do with the function's declared stack size?

And usually assembly is written for performance reasons, but if we're trying to transparently adapt ABIs, that gives up some of the performance.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Sep 6, 2018

Do we ever want to have more than two ABIs?

(By comparison, GCC on X86 has a large number of ABIs, through use of function attributes like regparm, sseregparm, force_align_arg_pointer, naked, stdcall.)

@aclements

This comment has been minimized.

Copy link
Member Author

commented Sep 6, 2018

Do we ever want to have more than two ABIs?

I'm not sure. I don't think we should preclude that possibility (let's not call the pragma //go:onetruestableabi), but I definitely don't think we should start there.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Sep 6, 2018

What I'm wondering is, once we have three ABIs, how do we know which one assembler code uses?

@cherrymui

This comment has been minimized.

Copy link
Contributor

commented Sep 6, 2018

I imagine we should have two ABIs, one "stable", and one "unspecified". The assembly code can only use the "stable" one, except for a small number of special functions like in the runtime. There could be as many variants of the "unspecified" ABI as we want, which the compiler can choose.

If we really have more than two, I think the assembly code can only call functions with specified ABI. The specified ABIs should all be stable.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Sep 7, 2018

I'm less interested in assembly code calling other functions, which is less common, than in how assembly functions themselves are called. Where we have cases where assembly code is justified, perhaps it is also worth using the faster calling convention to call that code. (Or maybe it doesn't matter, but it seems worth considering.)

@dr2chase

This comment has been minimized.

Copy link
Contributor

commented Sep 7, 2018

The other side of this question is what could we convince gollvm/gccgo to do. If Rust works with bespoke calling conventions, presumably there's a way to do this; should we give assembly the option of specifying precisely which registers to use, and declaring which ones it trashes?

@aclements

This comment has been minimized.

Copy link
Member Author

commented Sep 7, 2018

That's fair, and I think having faster calls from Go to assembly is another thing that would be infeasible now, but this proposal could enable. I think we still want the stack and unstable ABIs for legacy and optimization reasons, but this would make it possible to introduce other stable ABIs.

Implementation-wise, I imagine we would mark the assembly function's ABI in the symbol flags as well as on its Go declaration. To make ABI checking possible, we would introduce other variants of the CALL instruction that still assemble to a CALL, but mark the intended ABI of the call (perhaps via a relocation). These are probably the exact some mechanisms the compiler itself would use to implement multiple ABIs.

@thanm

This comment has been minimized.

Copy link
Member

commented Sep 7, 2018

With respect to gollvm: LLVM definitely supports multiple calling conventions and has provisions for tagging each call and each function with the expected calling convention. With that said, there is (currently) a fixed set of conventions (e.g. you have to update LLVM if you want to add another one). Also worth noting that most of the aspects of carrying out the calling convention (for example, figuring out which params go in memory and which in registers) are expected to be handled by the front end (there is code in the gollvm bridge that handles these details).

@paulzhol

This comment has been minimized.

Copy link
Member

commented Sep 7, 2018

Can this proposal also include that (gc supported) .syso object files be required to use the "stable stack" ABI as well?
I'm not referring to the .syso used by the race detector but the general mechanism in gc for linking external "assembly" code.

@aclements

This comment has been minimized.

Copy link
Member Author

commented Sep 7, 2018

Can this proposal also include that (gc supported) .syso object files be required to use the "stable stack" ABI as well?

I guess I don't see how this could work in general. Part of the Go stack ABI is participating in Go's stack growth protocol. Other than functions that don't use any stack space, I don't see how you can write a function in "external assembly" code that correctly follows the stack ABI. If anything, it seems like functions in syso files should be required to use the system ABI (and cgo is the bridge to the system ABI).

@paulzhol

This comment has been minimized.

Copy link
Member

commented Sep 7, 2018

@aclements I'm referring to the first part of https://github.com/golang/go/wiki/GcToolchainTricks

.. it shouldn't use too much stack .. For compute kernels, this requirement isn't too restricting.

Using a stable stack ABI to unpack arguments and set return variables, without the cgo overhead.

@aclements

This comment has been minimized.

Copy link
Member Author

commented Sep 7, 2018

I see.

Since syso files are also used for code using the system ABI (for example, I believe Go's boringcrypto works this way), I don't think we can force ABI constraints one way or the other directly on syso files. However, for functions with Go declarations, the declaration itself could certainly be marked with an ABI and the compiler will generate the correct calling sequence, assuming such calls are allowed (e.g., if we had a marker for "system ABI", we probably wouldn't allow direct Go -> system ABI calls because those are complicated to bridge; that's what cgo is for). We wouldn't be able to check ABI compatibility between the caller and the callee in this case. We'd just have to take it on faith that the declaration was marked correctly.

@paulzhol

This comment has been minimized.

Copy link
Member

commented Sep 7, 2018

@aclements If I understand you correctly then adding a function deceleration like this:

//go:stack_abi_call
func Func(uint64 arg)

for a compute .syso symbol would also save one writing the stubs for it

TEXT ·Func(SB),$0-8 // please set the correct parameter size (8) here
    JMP Func(SB)

otherwise when the declaration appearing without an annotation or used with //go:onetruestableabi then the (linker?) will report an error?

(I thought Go boringcrypto was using regular cgo, with the C symbols resolved by the linker in the .syso object, while the race detector using a custom cgo mechanism with //go:cgo_import_static directives).

@aclements

This comment has been minimized.

Copy link
Member Author

commented Sep 7, 2018

The ABI declaration would not save you from writing the stub for it, since stub isn't serving to adapt the calling convention. But the stub would default to using the stack ABI, so it would continue to work as written, with perhaps the additional annotation on the Go declaration.

(I thought Go boringcrypto was using regular cgo, with the C symbols resolved by the linker in the .syso object, while the race detector using a custom cgo mechanism with //go:cgo_import_static directives).

I'm not sure what point you're trying to make here. I was just saying that the function entry points in the boringcrypto syso file (and the race detector syso file) are using the system ABI. We can't just declare that all functions in syso files use the stack ABI. We don't know a priori what ABI they're using.

@paulzhol

This comment has been minimized.

Copy link
Member

commented Sep 7, 2018

ok thanks for clarifying @aclements!

@TocarIP

This comment has been minimized.

Copy link
Contributor

commented Sep 7, 2018

Is marking function as requiring stable ABI is better than marking it as requiring new ABI (when it is introduced)? For unmaintained projects there is no difference, because they won't have pragmas and wont update ABI, and eventually stop working, once ABI is changed. Maintained projects will have to change code anyway (at least by adding ABI pragma), so there is little reason not to switch to the new ABI, which presumably has some benefits.

@aclements

This comment has been minimized.

Copy link
Member Author

commented Sep 7, 2018

For Go functions, we should definitely default to the unstable ABI and require marking those that require a stable ABI. We want to move everyone to the unstable ABI because that's what gets us un-stuck and the vast majority of code won't be able to tell the difference.

For assembly functions, we have to default to the stack ABI since the vast majority of existing code will be able to tell the difference and currently assumes the stack ABI. I don't think we should even make it possible to declare assembly functions that use the unstable ABI or call functions from assembly that use the unstable ABI (though the runtime might need an escape hatch). We could perhaps migrate toward requiring all assembly functions to have an explicit ABI marking, but that seems like unnecessary churn.

For Go function stubs, I'm not sure. Since those are usually for assembly functions, we could say that those default to the stack ABI. But that seems inconsistent with Go functions defaulting to the unstable ABI, and would require an explicit pragma to select the unstable ABI for cases that need that (such as linkname'd Go functions). So I think Go function stubs should default to the unstable ABI, but I'm less certain about that.

@cherrymui

This comment has been minimized.

Copy link
Contributor

commented Sep 7, 2018

For Go calling assembly, maybe we can have function declarations (without body) default to stable ABI, unless otherwise specified. If the external function doesn't use the stack ABI, it doesn't work today.

For assembly calling Go, maybe we can scan the assembly code, find all the func symbols, and assume them to have the stack ABI. Maybe we can have a tool to do that and insert the pragma on the Go side. (Assembly code calling Go function defined in another package? Oh no, don't do it...)

@TocarIP

This comment has been minimized.

Copy link
Contributor

commented Sep 7, 2018

Defaulting to stack ABI in asm functions forever is extremely unfortunate. It encourages using more asm than necessary to avoid call overhead. It also causes weird situation were you know that better ABI is implemented, but you cannot use it in function that is so performance critical, that it is written in assembly. I'd personally prefer some option to use opt-in for new, versioned ABI, that may break in future releases.

@cherrymui

This comment has been minimized.

Copy link
Contributor

commented Sep 8, 2018

To support existing code, in particular as you mentioned unmaintained code, we probably have to default to the stack ABI. But it is just default. Once a new ABI is specified, the assembly code will be able to use it. You just need to change the assembly code and add the pragma at same time.

@aclements

This comment has been minimized.

Copy link
Member Author

commented Sep 8, 2018

For Go calling assembly, maybe we can have function declarations (without body) default to stable ABI, unless otherwise specified.

My concern with this is mostly that there are some uses for function declarations that aren't for calling assembly, such as linkname'ing Go functions across packages. In the case we'd still want to use the unstable ABI, so we'd need an annotation to opt-in to the unstable ABI. I'd rather not have such an annotation at all.

For assembly calling Go, maybe we can scan the assembly code, find all the func symbols, and assume them to have the stack ABI. Maybe we can have a tool to do that and insert the pragma on the Go side.

Yes, definitely. I think at first the vet tool should do this (checking ABI compatibility both ways), and once we're ready to change the unstable ABI, the linker should take responsibility for checking this. The vet tool can suggest exactly what to do to resolve the problem ("add pragma X here").

@rsc rsc changed the title proposal: make the ABI undefined as a step toward changing it proposal: runtime: make the ABI undefined as a step toward changing it Sep 19, 2018

@laboger

This comment has been minimized.

Copy link
Contributor

commented Sep 25, 2018

The other side of this question is what could we convince gollvm/gccgo to do. If Rust works with bespoke calling conventions, presumably there's a way to do this; should we give assembly the option of specifying precisely which registers to use, and declaring which ones it trashes?

On Power gccgo follows our normal ELF v2 PPC64 ABI, which means arguments and return values are passed in registers. gccgo doesn't recognize or know how to assemble Go assembler so calling them is not an issue.

@aclements

This comment has been minimized.

Copy link
Member Author

commented Oct 15, 2018

Another motivation for un-defining the default ABI: we could free up a fixed register for storing the G or possibly even the stack bound, which would speed up function prologues and reduce function call overhead.

@aclements

This comment has been minimized.

Copy link
Member Author

commented Nov 1, 2018

Here's a sketch of a new proposal that @rsc and I came up with for how to separate the stable and internal ABIs. This doesn't require any magic comments or user intervention whatsoever except in extremely unusual cases of breaking package boundaries with linkname or assembly (and even those mostly work). I have a mostly complete implementation of this (just ironing out some cgo weirdness because there's always cgo weirdness).

The core idea is to use ABI wrappers to translate between ABIs where necessary. These wrappers live alongside the "native" implementation of a function, so that function can be called via any available ABI rather than being constrained to a single ABI. We repurpose the existing symbol version mechanism so these symbols can all have the same name without colliding, and the ABI is encoded into every symbol reference.

The Go compiler always operates in terms of the internal ABI. Go functions use the internal ABI and calls from Go code use the internal ABI. Function values and method tables always use the internal ABI. If the Go compiler knows that the "native" implementation of a function uses another ABI, it can (and probably should) optimize call sites, but this is just an optimization.

The Go assembler always generates stable ABI (we'll call this ABI0) function symbols and all symbol references are to ABI0. There's simply no way to write an internal ABI symbol. If we do add another stable ABI in the future, we can extend the symbol syntax to have a way to specify the ABI (probably put something in angle brackets).

We add a mode to the Go assembler that scans the assembly and produces a "symbol ABIs" file containing the names of symbols defined and referenced via ABI0. This turns out to be pretty easy to do robustly, even accounting for go_defs.h weirdness. The build system feeds this file to the compiler and the compiler produces ABI0 -> internal wrappers for functions defined in assembly and internal -> ABI0 wrappers for functions called from assembly when it encounters the corresponding Go definitions (since it needs the type information).

Altogether, this keeps virtually all assembly code and linknamed symbols working as-is, which was a problem with my earlier proposal. I think there are only two cases that require user intervention:

  1. If assembly in package A defines a symbol in package B, then package A must provide a body-less Go function with a linkname. This happens a few times in std, and I really hope it never happens outside of std!

  2. If assembly in package A refers to a Go symbol S in package B, then package B must also have an assembly reference to S in order to produce the ABI0 -> internal wrapper. I've found exactly two instances of this: syscall and x/sys reference runtime.entersyscall and runtime.exitsyscall. We could potentially loosen this as long as package A imports package B and S is exported.

gopherbot pushed a commit that referenced this issue Nov 12, 2018

cmd/compile: create "init" symbol earlier
We create the "init" symbol and mark it as a function before compiling
to SSA because SSA can initialize this symbol, but it turns out we do
it slightly too late. peekitabs, at least, can also create the "init"
LSym. Move this initialization to just after type-checking.

Fixes the linux-amd64-ssacheck and the android-arm64-wiko-fever
builders.

Updates #27539.

Change-Id: If145952c79d39f75c93b24e35e67fe026dd08329
Reviewed-on: https://go-review.googlesource.com/c/149137
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>

gopherbot pushed a commit that referenced this issue Nov 12, 2018

cmd/compile: fix race on initializing Sym symFunc flag
SSA lowering can create PFUNC ONAME nodes when compiling method calls.
Since we generally initialize the node's Sym to a func when we set its
class to PFUNC, we did this here, too. Unfortunately, since SSA
compilation is concurrent, this can cause a race if two function
compilations try to initialize the same symbol.

Luckily, we don't need to do this at all, since we're actually just
wrapping an ONAME node around an existing Sym that's already marked as
a function symbol.

Fixes the linux-amd64-racecompile builder, which was broken by CL
147158.

Updates #27539.

Change-Id: I8ddfce6e66a08ce53998c5bfa6f5a423c1ffc1eb
Reviewed-on: https://go-review.googlesource.com/c/149158
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
@gopherbot

This comment has been minimized.

Copy link

commented Nov 13, 2018

Change https://golang.org/cl/149483 mentions this issue: test: fix ABI mismatch in fixedbugs/issue19507

gopherbot pushed a commit that referenced this issue Nov 13, 2018

test: fix ABI mismatch in fixedbugs/issue19507
Because run.go doesn't pass the package being compiled to the compiler
via the -p flag, it can't match up the main·f symbol from the
assembler with the "func f" stub in Go, so it doesn't produce the
correct assembly stub.

Fix this by removing the package prefix from the assembly definition.

Alternatively, we could make run.go pass -p to the compiler, but it's
nicer to remove these package prefixes anyway.

Should fix the linux-arm builder, which was broken by the introduction
of function ABIs in CL 147160.

Updates #27539.

Change-Id: Id62b7701e1108a21a5ad48ffdb5dad4356c273a6
Reviewed-on: https://go-review.googlesource.com/c/149483
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
@gopherbot

This comment has been minimized.

Copy link

commented Nov 14, 2018

Change https://golang.org/cl/149605 mentions this issue: cmd/link: fix isStmt DWARF info

gopherbot pushed a commit that referenced this issue Nov 14, 2018

cmd/link: fix isStmt DWARF info
When CL 147160 introduced function ABIs encoded as symbol versions in
the linker, it became slightly more complicated to look up derived
DWARF symbols. It fixed this by introducing a dwarfFuncSym function to
hide this logic, but missed one derived lookup that was done in the
object reader itself. As a result, we lost the isStmt tables from the
compiler, so every PC was marked as a statement in the DWARF info.

Fix this by moving this derived lookup out of the object reader and
into the DWARF code and calling dwarfFuncSym to get the correctly
versioned symbol.

Should fix the linux-amd64-longtest builder.

Updates #27539.

Change-Id: If40d5ba28bab1918ac4ad18fbb5103666b6d978b
Reviewed-on: https://go-review.googlesource.com/c/149605
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
@gopherbot

This comment has been minimized.

Copy link

commented Nov 15, 2018

Change https://golang.org/cl/149817 mentions this issue: cmd/go: more cross-package references from internal/syscall/unix

@gopherbot

This comment has been minimized.

Copy link

commented Nov 15, 2018

Change https://golang.org/cl/149818 mentions this issue: cmd/asm: rename -symabis to -gensymabis

@gopherbot

This comment has been minimized.

Copy link

commented Nov 16, 2018

Change https://golang.org/cl/150077 mentions this issue: design: add internal ABI design doc

@josharian

This comment has been minimized.

Copy link
Contributor

commented Nov 16, 2018

I can't seem to find the design CL in question, even manually entering a gerrit url. (See #28836 and #28839.) I'm sure I'm doing something stupid, but...if anyone has a link handy, that'd be nice. :)

@sbinet

This comment has been minimized.

Copy link
Member

commented Nov 16, 2018

gopherbot pushed a commit that referenced this issue Nov 16, 2018

cmd/asm: rename -symabis to -gensymabis
Currently, both asm and compile have a -symabis flag, but in asm it's
a boolean flag that means to generate a symbol ABIs file and in the
compiler its a string flag giving the path of the symbol ABIs file to
consume. I'm worried about this false symmetry biting us in the
future, so rename asm's flag to -gensymabis.

Updates #27539.

Change-Id: I8b9c18a852d2838099718f8989813f19d82e7434
Reviewed-on: https://go-review.googlesource.com/c/149818
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

bradfitz pushed a commit that referenced this issue Nov 21, 2018

cmd/link: fix isStmt DWARF info
When CL 147160 introduced function ABIs encoded as symbol versions in
the linker, it became slightly more complicated to look up derived
DWARF symbols. It fixed this by introducing a dwarfFuncSym function to
hide this logic, but missed one derived lookup that was done in the
object reader itself. As a result, we lost the isStmt tables from the
compiler, so every PC was marked as a statement in the DWARF info.

Fix this by moving this derived lookup out of the object reader and
into the DWARF code and calling dwarfFuncSym to get the correctly
versioned symbol.

Should fix the linux-amd64-longtest builder.

Updates #27539.

Change-Id: If40d5ba28bab1918ac4ad18fbb5103666b6d978b
Reviewed-on: https://go-review.googlesource.com/c/149605
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>

bradfitz pushed a commit that referenced this issue Nov 21, 2018

cmd/asm: rename -symabis to -gensymabis
Currently, both asm and compile have a -symabis flag, but in asm it's
a boolean flag that means to generate a symbol ABIs file and in the
compiler its a string flag giving the path of the symbol ABIs file to
consume. I'm worried about this false symmetry biting us in the
future, so rename asm's flag to -gensymabis.

Updates #27539.

Change-Id: I8b9c18a852d2838099718f8989813f19d82e7434
Reviewed-on: https://go-review.googlesource.com/c/149818
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

gopherbot pushed a commit that referenced this issue Nov 23, 2018

cmd/go: more cross-package references from internal/syscall/unix
On some platforms, assembly in internal/syscall/unix references
unexported runtime symbols. Catch these references so the compiler can
generate the necessary ABI wrappers.

Fixes #28769.
Updates #27539.

Change-Id: I118eebfb8b3d907b4c3562198e6afb49854f5827
Reviewed-on: https://go-review.googlesource.com/c/149817
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Clément Chigot <clement.chigot@atos.net>
Reviewed-by: Keith Randall <khr@golang.org>
@gopherbot

This comment has been minimized.

Copy link

commented Jan 9, 2019

Change https://golang.org/cl/157017 mentions this issue: cmd/compile: separate data and function LSyms

gopherbot pushed a commit that referenced this issue Jan 11, 2019

cmd/compile: separate data and function LSyms
Currently, obj.Ctxt's symbol table does not distinguish between ABI0
and ABIInternal symbols. This is *almost* okay, since a given symbol
name in the final object file is only going to belong to one ABI or
the other, but it requires that the compiler mark a Sym as being a
function symbol before it retrieves its LSym. If it retrieves the LSym
first, that LSym will be created as ABI0, and later marking the Sym as
a function symbol won't change the LSym's ABI.

Marking a Sym as a function symbol before looking up its LSym sounds
easy, except Syms have a dual purpose: they are used just as interned
strings (every function, variable, parameter, etc with the same
textual name shares a Sym), and *also* to store state for whatever
package global has that name. As a result, it's easy to slip up and
look up an LSym when a Sym is serving as the name of a local variable,
and then later mark it as a function when it's serving as the global
with the name.

In general, we were careful to avoid this, but #29610 demonstrates one
case where we messed up. Because of on-demand importing from indexed
export data, it's possible to compile a method wrapper for a type
imported from another package before importing an init function from
that package. If the argument of the method is named "init", the
"init" LSym will be created as a data symbol when compiling the
wrapper, before it gets marked as a function symbol.

To fix this, we separate obj.Ctxt's symbol tables for ABI0 and
ABIInternal symbols. This way, the compiler will simply get a
different LSym once the Sym takes on its package-global meaning as a
function.

This fixes the above ordering issue, and means we no longer need to go
out of our way to create the "init" function early and mark it as a
function symbol.

Fixes #29610.
Updates #27539.

Change-Id: Id9458b40017893d46ef9e4a3f9b47fc49e1ce8df
Reviewed-on: https://go-review.googlesource.com/c/157017
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>

gopherbot pushed a commit to golang/proposal that referenced this issue Jan 16, 2019

design: add internal ABI design doc
Updates golang/go#27539.

Change-Id: I40e51d59847cdb2df9ad6e3b402c619329172f47
Reviewed-on: https://go-review.googlesource.com/c/150077
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@neclepsio

This comment has been minimized.

Copy link

commented Feb 26, 2019

I think this change broke go-qml, in particular the cdata package. The error is: cdata.Ref: relocation target runtime.acquirem not defined for ABI0 (but is defined for ABIInternal).

The proposal "compatibility" section suggests some work-arounds, but I was not able to fix the error.

I opened the issue go-qml/qml#190, but the package is abandoned so I'm not so confident someone will help there. Can someone please help me fix?

@aclements

This comment has been minimized.

Copy link
Member Author

commented Feb 26, 2019

@neclepsio, thanks for the report. Unfortunately, reaching into the runtime implementation like this is really, really not supported. Given that go-qml is officially dead and there are other (maintained) Qt packages for Go, can you expand a bit on what you're trying to accomplish so I can better point you in the right direction?

For example, on Linux it would be better to use syscall.Gettid.

@neclepsio

This comment has been minimized.

Copy link

commented Feb 26, 2019

@aclements thank you for your reply. I'm just trying to upgrade to go 1.12 my package using go-qml. I don't understand what the assembly code does and what the cdata package is for. Unfortunately using other qt bindings would require to rewrite the ui part of my application from the ground up. Moreover, there are licensing issues with therecipe/qt. So it would be great to be able to keep go-qml on life support.

@neclepsio

This comment has been minimized.

Copy link

commented Feb 27, 2019

@aclements thank you for your suggestion to use syscall.Gettid. After having understood what the function did, at least under Windows I got it working.

@aclements

This comment has been minimized.

Copy link
Member Author

commented Feb 27, 2019

Glad you were able to get it working, @neclepsio.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.