FAQ
Some low-level comments below. I apologize that we don't have gofmt for
C code.

I would like to understand a bit better what the model is that we want
to provide when invoking Go code from a shared library. Can you write a
paragraph or two (in email) about what users should expect?

For example:
- It appears that os.Args will not be initialized. What other changes
are there?
- Can you load multiple Go shared objects or just one?
- When does package initialization happen?



https://codereview.appspot.com/6822078/diff/15001/src/pkg/runtime/asm_amd64.s
File src/pkg/runtime/asm_amd64.s (right):

https://codereview.appspot.com/6822078/diff/15001/src/pkg/runtime/asm_amd64.s#newcode84
src/pkg/runtime/asm_amd64.s:84: libret:
elfret:
// ELF shared library return

extra points for another comment with a link to the spec for this
procedure

https://codereview.appspot.com/6822078/diff/15001/src/pkg/runtime/asm_amd64.s#newcode85
src/pkg/runtime/asm_amd64.s:85: ADDQ $(4*8 + 16), SP // 2args 2auto
delete comment

https://codereview.appspot.com/6822078/diff/15001/src/pkg/runtime/asm_amd64.s#newcode177
src/pkg/runtime/asm_amd64.s:177: CMPB runtime·islibrary(SB), $1
I don't understand this. mcall is not supposed to return in most of the
contexts where it is called. Being in a library does not change all
those contexts. Which context is supposed to expect the return? Perhaps
we can put a flag in the m struct or something like that.

https://codereview.appspot.com/6822078/diff/15001/src/pkg/runtime/os_linux.h
File src/pkg/runtime/os_linux.h (right):

https://codereview.appspot.com/6822078/diff/15001/src/pkg/runtime/os_linux.h#newcode11
src/pkg/runtime/os_linux.h:11: int32 runtime·open(uint8*, int32, int32);
tabs, like the surrounding lines

https://codereview.appspot.com/6822078/diff/15001/src/pkg/runtime/proc.c
File src/pkg/runtime/proc.c (right):

https://codereview.appspot.com/6822078/diff/15001/src/pkg/runtime/proc.c#newcode15
src/pkg/runtime/proc.c:15: bool runtime·islibrary = 0;
= 0 is redundant. remove.

https://codereview.appspot.com/6822078/diff/15001/src/pkg/runtime/proc.c#newcode250
src/pkg/runtime/proc.c:250: if (!runtime·islibrary) {
no space
But really this should be

if(runtime.islibrary)
return;

and then the rest of the function can be normal.

https://codereview.appspot.com/6822078/diff/15001/src/pkg/runtime/proc.c#newcode625
src/pkg/runtime/proc.c:625: if (&m->g0->sched.pc == (void *)-1)
No space between if and (.
Also I think the new condition would be clearer as

if(!runtime.islibrary)

https://codereview.appspot.com/6822078/diff/15001/src/pkg/runtime/proc.c#newcode768
src/pkg/runtime/proc.c:768: static bool completed = 0;
s/ = 0//

https://codereview.appspot.com/6822078/diff/15001/src/pkg/runtime/proc.c#newcode768
src/pkg/runtime/proc.c:768: static bool completed = 0;
s/ = 0//
s/completed/m0done/

https://codereview.appspot.com/6822078/diff/15001/src/pkg/runtime/proc.c#newcode786
src/pkg/runtime/proc.c:786: if (completed && m == &runtime·m0)
no space

https://codereview.appspot.com/6822078/diff/15001/src/pkg/runtime/proc.c#newcode788
src/pkg/runtime/proc.c:788: if (!runtime·islibrary)
no space

https://codereview.appspot.com/6822078/diff/15001/src/pkg/runtime/rt0_linux_amd64.s
File src/pkg/runtime/rt0_linux_amd64.s (right):

https://codereview.appspot.com/6822078/diff/15001/src/pkg/runtime/rt0_linux_amd64.s#newcode13
src/pkg/runtime/rt0_linux_amd64.s:13: SUBQ $0x58, SP /* keeps stack
pointer 32-byte aligned */
This comment is unclear. (0x58 > 32)

Perhaps instead of that new code in rt0_amd64, the JMP below can become
a call, and then the restoring code can be done here instead of in
rt0_amd64?

https://codereview.appspot.com/6822078/diff/15001/src/pkg/runtime/runtime.c
File src/pkg/runtime/runtime.c (right):

https://codereview.appspot.com/6822078/diff/15001/src/pkg/runtime/runtime.c#newcode100
src/pkg/runtime/runtime.c:100: int32 i, n = 0;
no initialization in variable declarations.

https://codereview.appspot.com/6822078/diff/15001/src/pkg/runtime/runtime.c#newcode102
src/pkg/runtime/runtime.c:102: if (argc > 0)
no space

n = 0;
if(argc > 0)
while(argv[argc+1+n] != 0)
n++;

https://codereview.appspot.com/6822078/diff/15001/src/pkg/runtime/symtab.c
File src/pkg/runtime/symtab.c (right):

https://codereview.appspot.com/6822078/diff/15001/src/pkg/runtime/symtab.c#newcode105
src/pkg/runtime/symtab.c:105: if (runtime·strcmp((byte
*)"runtime.findfunc", sym->name) == 0)
no space between if and (
no space between (byte and *)

Perhaps the linker could define a variable at shared library 0 instead
of needing to compute it this way?

xdefine("reloffset", SRODATA, 0);

seems like it would work.

https://codereview.appspot.com/6822078/diff/15001/src/pkg/runtime/vdso_linux_amd64.c
File src/pkg/runtime/vdso_linux_amd64.c (right):

https://codereview.appspot.com/6822078/diff/15001/src/pkg/runtime/vdso_linux_amd64.c#newcode296
src/pkg/runtime/vdso_linux_amd64.c:296: static uint64
runtime·atolhex(byte *s, uint32 count) {
\n before runtime
\n before {

https://codereview.appspot.com/6822078/diff/15001/src/pkg/runtime/vdso_linux_amd64.c#newcode302
src/pkg/runtime/vdso_linux_amd64.c:302: if(*s >= '0' && *s <= '9')
insert n *= 16;
and then you can use n += below

https://codereview.appspot.com/6822078/diff/15001/src/pkg/runtime/vdso_linux_amd64.c#newcode345
src/pkg/runtime/vdso_linux_amd64.c:345: byte vdso_name[] = {'[', 'v',
'd', 's', 'o', ']'};
make this a global

https://codereview.appspot.com/6822078/diff/15001/src/pkg/runtime/vdso_linux_amd64.c#newcode346
src/pkg/runtime/vdso_linux_amd64.c:346: int32 vdso_name_idx = 0,
start_idx = 0;
no initializations in declarations please

https://codereview.appspot.com/6822078/diff/15001/src/pkg/runtime/vdso_linux_amd64.c#newcode351
src/pkg/runtime/vdso_linux_amd64.c:351: if (maps_fd < 0)
no space. etc.

https://codereview.appspot.com/6822078/diff/15001/src/pkg/runtime/vdso_linux_amd64.c#newcode360
src/pkg/runtime/vdso_linux_amd64.c:360: case '\n':
do not indent case statements. the words switch and case should be
aligned.

https://codereview.appspot.com/6822078/

Search Discussions

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedJan 2, '13 at 9:36p
activeJan 6, '13 at 5:24a
posts2
users2
websitegolang.org

2 users in discussion

Rsc: 1 post Elias Naur: 1 post

People

Translate

site design / logo © 2022 Grokbase