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

flag: index out of range when argc is 0 #32775

Closed
chirkink opened this issue Jun 25, 2019 · 13 comments
Closed

flag: index out of range when argc is 0 #32775

chirkink opened this issue Jun 25, 2019 · 13 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@chirkink
Copy link

What version of Go are you using (go version)?

$ go version
go version go1.12.6 linux/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE=""
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH=""
GOPROXY=""
GORACE=""
GOROOT="/usr/lib/go-1.12"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go-1.12/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"

What did you do?

When golang compiled binary invoked by execve with 0 arguments like this:

test.cpp code
#include 

int main() {
execve("test.go.out", NULL, NULL);
}

https://play.golang.org/p/zo3YJZTkBu_A

What did you expect to see?

nothing

What did you see instead?

panic: runtime error: index out of range

goroutine 1 [running]:
flag.init.ializers()
/usr/lib/go-1.12/src/flag/flag.go:1009 +0x172

As for documentation, it's sort of guaranteed that os.Args[0] is a program name:
https://golang.org/pkg/os/#pkg-variables

When argc is 0, at least for Linux platform program name can be fetched from procfs(/proc/self/comm), probably here:

argslice = make([]string, argc)
for i := int32(0); i < argc; i++ {
argslice[i] = gostringnocopy(argv_index(argv, i))
}

@ianlancetaylor ianlancetaylor added help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Jun 25, 2019
@ianlancetaylor ianlancetaylor added this to the Go1.14 milestone Jun 25, 2019
@robpike
Copy link
Contributor

robpike commented Jun 25, 2019

Not sure there's anything to fix. That binary compiles and runs fine; it's the caller that's making a mistake: argv should never be empty.

@gregory-m
Copy link
Contributor

gregory-m commented Jun 26, 2019

From execve(2) man page:

argv is an array of argument strings passed to the new program. By convention, the first of these strings should contain the filename associated with the file being executed

On Linux, argv can be specified as NULL, which has the same effect as
specifying this argument as a pointer to a list containing a single NULL pointer.
Do not take advantage of this misfeature!
It is nonstandard and nonportable: on most other UNIX systems doing this
will result in an error (EFAULT).

So, I'am also not sure if we need to fix something.

@chirkink
Copy link
Author

chirkink commented Jul 1, 2019

I've spotted this issue when my binary was ran by rsyslog.
Documentation looks inconsistent, as argv[0] doesn't have a program name, but whatever is provided to argv[0].
Module flag apparently is not only in charge of parsing args, but also flags when there is no argv[0] by throwing runtime error.
I'm new to golang and don't really have horse in this race, but from what I see at least len(argv) in flag module needs to be checked.

@gregory-m
Copy link
Contributor

What config was used for rsyslog?

Something like this:

module(load="omprog")
action(type="omprog"
       binary="/pathto/omprog.py --parm1=\"value 1\" --parm2=\"value2\""
       template="RSYSLOG_TraditionalFileFormat")

@chirkink
Copy link
Author

chirkink commented Jul 1, 2019

yep, binary was invoked by omprog with no parameters.
like binary="/pathto/omprog.py"

@chirkink
Copy link
Author

chirkink commented Jul 1, 2019

It was fixed in latest versions of rsyslog apparently:
rsyslog/rsyslog@8d807e3

mmexternal bugfix: potentially missing argv[0]
This is incorrect and can cause problems with some language, namely go.

@gregory-m
Copy link
Contributor

I searched Github and found another examples of execve("cmd", NULL, NULL); usage. It defentely not best practice but maybe it good idea to not panic in such case? @ianlancetaylor @robpike

@ianlancetaylor
Copy link
Contributor

It seems to me that the C getopt function will also crash in this case. It really doesn't seem worth worrying about.

But I guess if someone sends a patch for 1.14 we can take a look.

@chirkink
Copy link
Author

chirkink commented Jul 1, 2019

getopt(0,NULL,NULL) will work as argc is 0 obviously,
I can try to work on patch, but I would need some guidance on where this issue needs to be fixed.

@ianlancetaylor
Copy link
Contributor

In the initialization of CommandLine in the flag package.

@gopherbot
Copy link

Change https://golang.org/cl/184637 mentions this issue: flag: handle nil os.Args on CommandLine initialization

@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@smasher164 smasher164 modified the milestones: Backlog, Go1.14 Oct 11, 2019
@odeke-em
Copy link
Member

odeke-em commented Nov 8, 2019

Thank you for filing this issue @Kneht and for the patience and everyone for chiming in. Welcome to the Go project!

I agree with what @robpike and @ianlancetaylor say: I don't think we should do anything here.

You are incorrectly using execve and yet execve asks that you pass at least one argument to char *const argv[] with the various standards and OSes stating in e.g.

Posix

The all encompassing standard says
https://pubs.opengroup.org/onlinepubs/9699919799/functions/exec.html
Screen Shot 2019-11-07 at 10 06 16 PM

Linux

http://man7.org/linux/man-pages/man2/execve.2.html
Screen Shot 2019-11-07 at 10 02 36 PM

Windows

https://docs.microsoft.com/en-us/cpp/c-runtime-library/exec-wexec-functions?view=vs-2019
Screen Shot 2019-11-07 at 10 05 48 PM

Darwin

https://man.openbsd.org/execve.2
Screen Shot 2019-11-07 at 10 13 08 PM

FreeBSD

https://www.freebsd.org/cgi/man.cgi?query=execve&sektion=2
Screen Shot 2019-11-07 at 10 12 16 PM

and running a C variant on my MacbookPro computer

#include <unistd.h>

int main(int argc, char **argv) {
    getopt(0, NULL, NULL);
}

produces a SEGMENTATION FAULT as per

$ gcc call.c -o call && ./call
Segmentation fault: 11

Thus I shall close this issue, but nonetheless thank you for filing it, for the discourse, patience and for the CL. Looking forward to continuing to interact more with you on Go.

@odeke-em odeke-em closed this as completed Nov 8, 2019
@chirkink
Copy link
Author

chirkink commented Nov 8, 2019

Hi @odeke-em, thank you for complete reply.

Based on links you've posted, standard and Linux documentation says that argv[0] should contain the filename, which sounds like recommendation to me.

Obviously you can check in evecve documentation for every OS and run any tests on personal computer of your choice, but it doesn't matter much as more then 50% of golang users are on Linux, at least based on survey 2018.

Given that fix has been implemented by Max and pre approved by Ian I guess it's up to them if this change needs to be merged.

Thanks for your contributions @odeke-em .

@golang golang locked and limited conversation to collaborators Nov 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

8 participants