code review 7058054: lib9: declare __fixargv0 before use in flag.c

lib9: declare __fixargv0 before use in flag.c

The Plan 9 compilers complain about not
having type information for the function,
which sets off type signature problems
during the linking stage.

[akumar, 2013-01-08 02:58:45]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go

[lucio, 2013-01-08 03:59:29]

I was going to include a declaration of __fixargv0()  in
"include/plan9/libc.h", then thought that it seemed unnecessary to
advertise it outside osx.  Now it seems plan 9 also needs to be
alerted to it.

Lucio.

[akumar, 2013-01-08 04:11:55]

It seems too specific. We're only building one file from lib9
that needs the function -- and only peripherally so. Nor
does it seem like this will propagate to other files we'll build,
hence a local change here seems sufficient.


On 8 January 2013 03:59, Lucio De Re <lucio.dere@gmail.com> wrote:
> I was going to include a declaration of __fixargv0()  in
> "include/plan9/libc.h", then thought that it seemed unnecessary to
> advertise it outside osx.  Now it seems plan 9 also needs to be
> alerted to it.
>
> Lucio.

[lucio, 2013-01-08 04:14:04]

Good enough for me, then.

Lucio.

On 1/8/13, Akshat Kumar <seed@mail.nanosouffle.net> wrote:
> It seems too specific. We're only building one file from lib9
> that needs the function -- and only peripherally so. Nor
> does it seem like this will propagate to other files we'll build,
> hence a local change here seems sufficient.
>
>
> On 8 January 2013 03:59, Lucio De Re <lucio.dere@gmail.com> wrote:
>> I was going to include a declaration of __fixargv0()  in
>> "include/plan9/libc.h", then thought that it seemed unnecessary to
>> advertise it outside osx.  Now it seems plan 9 also needs to be
>> alerted to it.
>>
>> Lucio.
>

[rsc, 2013-01-09 19:03:20]

Copy and paste the char *argv0; and fixargv0 definitions into flag.c
and delete argv0.c entirely.
Then flag.c doesn't need to call fixargv0 either.

[akumar, 2013-01-09 20:25:42]

sysfatal.c uses argv0/fixargv0 as well, iirc. Should perhaps argv0 go into a
header file that ought to be included by the sources that need it?

On 9 January 2013 11:03, Russ Cox <rsc@golang.org> wrote:
> Copy and paste the char *argv0; and fixargv0 definitions into flag.c
> and delete argv0.c entirely.
> Then flag.c doesn't need to call fixargv0 either.

[rsc, 2013-01-09 20:43:25]

sysfatal was already working. This is only about the complexities
added by flag.c. They go away if we just put argv0.c into flag.c.

[ality, 2013-01-09 23:19:21]

Akshat Kumar <seed@mail.nanosouffle.net> once said:
> On 9 January 2013 11:03, Russ Cox <rsc@golang.org> wrote:
> > Copy and paste the char *argv0; and fixargv0 definitions into flag.c
> > and delete argv0.c entirely.
> > Then flag.c doesn't need to call fixargv0 either.
>
> sysfatal.c uses argv0/fixargv0 as well, iirc. Should perhaps argv0 go into a
> header file that ought to be included by the sources that need it?

argv0 is already defined (with extern) in libc.h

Just remove argv0.c and both references to __fixargv0,
in sysfatal.c and flag.c.

  Anthony

[akumar, 2013-01-10 02:58:01]

PTAL.

I've just added __fixargv0 into flag.c and removed argv0.c for now. I can also
remove all mentions of __fixargv0 from lib9 if that's preferred.

Please let me know.

[rsc, 2013-01-10 15:33:28]

Yes, please do NOT remove __fixargv0 from lib9. It's there for a
reason (there's a comment).

[rsc, 2013-01-10 15:34:14]

https://codereview.appspot.com/7058054/diff/2002/src/lib9/argv0.c
File src/lib9/argv0.c (left):

https://codereview.appspot.com/7058054/diff/2002/src/lib9/argv0.c#oldcode32
src/lib9/argv0.c:32: * Mac OS can't deal with files that only declare data.
Please copy this comment to the new location. Thank you.

[ality, 2013-01-11 05:06:44]

Russ Cox <rsc@golang.org> once said:
> Yes, please do NOT remove __fixargv0 from lib9. It's there for a
> reason (there's a comment).

I read the comment.

How is it relevant now that the argv0 declaration is
in a file that declares more than data?

  Anthony

[rsc, 2013-01-18 20:18:20]

> How is it relevant now that the argv0 declaration is
> in a file that declares more than data?

The problem is that old linkers (such as Darwin's) do not bring in an
object file from an archive just to satisfy a data reference. It only
does it to satisfy function references. Referring to __fixargv0 is a
way to bring in the object file containing argv0. ARGBEGIN does this;
otherwise the link will fail. There is no need for a use of __fixargv0
in flag.c because argv0 is now in the same file. But the definition
needs to be there, so that ARGBEGIN can continue to use it.

Russ

[akumar, 2013-01-18 23:15:50]

PTAL.

Removed the call to __fixargv0 in flag.c .

https://codereview.appspot.com/7058054/diff/2002/src/lib9/argv0.c
File src/lib9/argv0.c (left):

https://codereview.appspot.com/7058054/diff/2002/src/lib9/argv0.c#oldcode32
src/lib9/argv0.c:32: * Mac OS can't deal with files that only declare data.
On 2013/01/10 15:34:14, rsc wrote:
> Please copy this comment to the new location. Thank you.

Done.

[iant2, 2013-01-19 00:12:46]

On Fri, Jan 18, 2013 at 12:18 PM, Russ Cox <rsc@golang.org> wrote:
>> How is it relevant now that the argv0 declaration is
>> in a file that declares more than data?
>
> The problem is that old linkers (such as Darwin's) do not bring in an
> object file from an archive just to satisfy a data reference. It only
> does it to satisfy function references. Referring to __fixargv0 is a
> way to bring in the object file containing argv0. ARGBEGIN does this;
> otherwise the link will fail. There is no need for a use of __fixargv0
> in flag.c because argv0 is now in the same file. But the definition
> needs to be there, so that ARGBEGIN can continue to use it.

That doesn't sound right.  If your object file says "int i;" and the
archive says "int i = 2;" then I believe that the Darwin linker
doesn't bring in the object from the archive.  But if your object file
says "extern int i;" then I would expect the linker to pull in the
object from the archive.  That said, if your object says "extern int
i;" and the archive says "int i;", then the object might not be pulled
in; different systems have different behaviour for that.

Ian

[rsc, 2013-01-22 18:30:04]

On Fri, Jan 18, 2013 at 7:12 PM, Ian Lance Taylor <iant@google.com> wrote:

> That doesn't sound right.  If your object file says "int i;" and the
> archive says "int i = 2;" then I believe that the Darwin linker
> doesn't bring in the object from the archive.  But if your object file
> says "extern int i;" then I would expect the linker to pull in the
> object from the archive.  That said, if your object says "extern int
> i;" and the archive says "int i;", then the object might not be pulled
> in; different systems have different behaviour for that.
>

Yes, the bss case is the problem I ran into.

Russ

[rsc, 2013-01-22 22:23:06]

LGTM

[rsc, 2013-01-22 22:23:40]

*** Submitted as https://code.google.com/p/go/source/detail?r=4adf815d56df ***

lib9: declare __fixargv0 before use in flag.c

The Plan 9 compilers complain about not
having type information for the function,
which sets off type signature problems
during the linking stage.

R=rsc, ality, iant
CC=golang-dev
https://codereview.appspot.com/7058054

Committer: Russ Cox <rsc@golang.org>