# [PATCH 2/2] Fix stack-use-after-scope error reported by ASAN by GCC 7. 4 messages from 2017-04-06 to 2017-04-06. Participants: Martin Liška, Jeff King, Johannes Schindelin. Thread: https://gitlist.dev/t/45617 ## Martin Liška, 2017-04-06 08:02 Subject: [PATCH 2/2] Fix stack-use-after-scope error reported by ASAN by GCC 7. Message-ID: <072afb58-6159-ddeb-b7dc-40a87e8c6ae7@suse.cz> URL: https://gitlist.dev/e/072afb58-6159-ddeb-b7dc-40a87e8c6ae7%40suse.cz ``` Hello. Following patch fixes issues that can be seen with -fsanitize=address on GCC 7. Patch was tested with make test. Thanks, Martin From 79dace4bdac4f571c14c7edb9b1007c155475c3f Mon Sep 17 00:00:00 2001 From: marxin Date: Wed, 5 Apr 2017 14:32:29 +0200 Subject: [PATCH 2/2] Fix stack-use-after-scope error reported by ASAN by GCC 7. The use-after-scope is triggered here: READ of size 8 at 0x7ffc4f674e20 thread T0 #0 0x6f0b69 in finish_command /home/marxin/Programming/git/run-command.c:570 #1 0x5b6101 in kill_multi_file_filter /home/marxin/Programming/git/convert.c:570 #2 0x5b798a in kill_multi_file_filter /home/marxin/Programming/git/convert.c:770 Signed-off-by: Martin Liska --- convert.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/convert.c b/convert.c index 8d652bf27..bf4eaf10e 100644 --- a/convert.c +++ b/convert.c @@ -568,6 +568,7 @@ static void kill_multi_file_filter(struct hashmap *hashmap, struct cmd2process * entry->process.clean_on_exit = 0; kill(entry->process.pid, SIGTERM); finish_command(&entry->process); + free(entry->process.argv); hashmap_remove(hashmap, entry, NULL); free(entry); @@ -582,6 +583,7 @@ static void stop_multi_file_filter(struct child_process *process) sigchain_pop(SIGPIPE); /* Finish command will wait until the shutdown is complete. */ finish_command(process); + free(process->argv); } static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, const char *cmd) @@ -589,7 +591,6 @@ static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, cons int err; struct cmd2process *entry; struct child_process *process; - const char *argv[] = { cmd, NULL }; struct string_list cap_list = STRING_LIST_INIT_NODUP; char *cap_buf; const char *cap_name; @@ -600,7 +601,8 @@ static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, cons process = &entry->process; child_process_init(process); - process->argv = argv; + process->argv = xcalloc(2, sizeof(const char *)); + process->argv[0] = cmd; process->use_shell = 1; process->in = -1; process->out = -1; -- 2.12.2 ``` ## Jeff King, 2017-04-06 08:33 Subject: Re: [PATCH 2/2] Fix stack-use-after-scope error reported by ASAN by GCC 7. Message-ID: <20170406083305.bs3hkxp7dagajg7m@sigill.intra.peff.net> URL: https://gitlist.dev/e/20170406083305.bs3hkxp7dagajg7m%40sigill.intra.peff.net In-Reply-To: <072afb58-6159-ddeb-b7dc-40a87e8c6ae7@suse.cz> ``` On Thu, Apr 06, 2017 at 10:02:45AM +0200, Martin Liška wrote: > Subject: [PATCH 2/2] Fix stack-use-after-scope error reported by ASAN by GCC > 7. > > The use-after-scope is triggered here: > READ of size 8 at 0x7ffc4f674e20 thread T0 > #0 0x6f0b69 in finish_command /home/marxin/Programming/git/run-command.c:570 > #1 0x5b6101 in kill_multi_file_filter /home/marxin/Programming/git/convert.c:570 > #2 0x5b798a in kill_multi_file_filter /home/marxin/Programming/git/convert.c:770 Yeah, this is definitely a problem. Your fix works, but... > @@ -600,7 +601,8 @@ static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, cons > process = &entry->process; > > child_process_init(process); > - process->argv = argv; > + process->argv = xcalloc(2, sizeof(const char *)); > + process->argv[0] = cmd; > process->use_shell = 1; > process->in = -1; > process->out = -1; We can just do: argv_array_push(&process->args, cmd); here. And then it is freed automatically when finish_command() is called (and also if start_command never starts the process, which I think your patch misses). -Peff ``` ## Johannes Schindelin, 2017-04-06 08:55 Subject: Re: [PATCH 2/2] Fix stack-use-after-scope error reported by ASAN by GCC 7. Message-ID: URL: https://gitlist.dev/e/alpine.DEB.2.20.1704061051130.4268%40virtualbox In-Reply-To: <072afb58-6159-ddeb-b7dc-40a87e8c6ae7@suse.cz> ``` Hi Martin, On Thu, 6 Apr 2017, Martin Liška wrote: > Following patch fixes issues that can be seen with -fsanitize=address on > GCC 7. Good catch. However, it may make more sense to switch to using the "args" field instead of the "argv" field: it is of type "struct argv_array" and is released automagically by finish_command(). In other words, you would use something like @@ -600,7 +601,8 @@ static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, cons process = &entry->process; child_process_init(process); - process->argv = argv; + argv_array_push(&process->args, cmd); instead, making even for a nice LOC reduction. Ciao, Johannes ``` ## Martin Liška, 2017-04-06 10:23 Subject: Re: [PATCH v2 2/2] Fix stack-use-after-scope error reported by ASAN by GCC 7. Message-ID: <28c22c5e-677b-1554-6156-4e31cc0f0913@suse.cz> URL: https://gitlist.dev/e/28c22c5e-677b-1554-6156-4e31cc0f0913%40suse.cz In-Reply-To: ``` On 04/06/2017 10:55 AM, Johannes Schindelin wrote: > Hi Martin, > > On Thu, 6 Apr 2017, Martin Liška wrote: > >> Following patch fixes issues that can be seen with -fsanitize=address on >> GCC 7. > > Good catch. Yep, actually it was me who wrote the new use-after-scope support in GCC 7. And I was bit pessimistic about real examples of such errors, but one popped up very soon. > > However, it may make more sense to switch to using the "args" field > instead of the "argv" field: it is of type "struct argv_array" and is > released automagically by finish_command(). > > In other words, you would use something like > > @@ -600,7 +601,8 @@ static struct cmd2process > *start_multi_file_filter(struct hashmap *hashmap, cons > process = &entry->process; > > child_process_init(process); > - process->argv = argv; > + argv_array_push(&process->args, cmd); > > instead, making even for a nice LOC reduction. Done that way, survives tests. Martin > > Ciao, > Johannes > From 4e80f9bddff3c29fefb3628e0e920ca265d55e65 Mon Sep 17 00:00:00 2001 From: marxin Date: Thu, 6 Apr 2017 11:40:24 +0200 Subject: [PATCH 2/2] Fix stack-use-after-scope error reported by ASAN by GCC 7. The use-after-scope is triggered here: READ of size 8 at 0x7ffc4f674e20 thread T0 #0 0x6f0b69 in finish_command /home/marxin/Programming/git/run-command.c:570 #1 0x5b6101 in kill_multi_file_filter /home/marxin/Programming/git/convert.c:570 #2 0x5b798a in kill_multi_file_filter /home/marxin/Programming/git/convert.c:770 --- convert.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/convert.c b/convert.c index 8d652bf27..73eb28eb8 100644 --- a/convert.c +++ b/convert.c @@ -589,7 +589,6 @@ static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, cons int err; struct cmd2process *entry; struct child_process *process; - const char *argv[] = { cmd, NULL }; struct string_list cap_list = STRING_LIST_INIT_NODUP; char *cap_buf; const char *cap_name; @@ -600,7 +599,7 @@ static struct cmd2process *start_multi_file_filter(struct hashmap *hashmap, cons process = &entry->process; child_process_init(process); - process->argv = argv; + argv_array_push(&process->args, cmd); process->use_shell = 1; process->in = -1; process->out = -1; -- 2.12.2 ```