* [PATCH 2/2] Fix stack-use-after-scope error reported by ASAN by GCC 7.
@ 2017-04-06 8:02 Martin Liška
2017-04-06 8:33 ` Jeff King
2017-04-06 8:55 ` Johannes Schindelin
0 siblings, 2 replies; 4+ messages in thread
From: Martin Liška @ 2017-04-06 8:02 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 138 bytes --]
Hello.
Following patch fixes issues that can be seen with -fsanitize=address on GCC 7.
Patch was tested with make test.
Thanks,
Martin
[-- Attachment #2: 0002-Fix-stack-use-after-scope-error-reported-by-ASAN-by-.patch --]
[-- Type: text/x-patch, Size: 2049 bytes --]
From 79dace4bdac4f571c14c7edb9b1007c155475c3f Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
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 <mliska@suse.cz>
---
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] Fix stack-use-after-scope error reported by ASAN by GCC 7.
2017-04-06 8:02 [PATCH 2/2] Fix stack-use-after-scope error reported by ASAN by GCC 7 Martin Liška
@ 2017-04-06 8:33 ` Jeff King
2017-04-06 8:55 ` Johannes Schindelin
1 sibling, 0 replies; 4+ messages in thread
From: Jeff King @ 2017-04-06 8:33 UTC (permalink / raw)
To: Martin Liška; +Cc: git
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] Fix stack-use-after-scope error reported by ASAN by GCC 7.
2017-04-06 8:02 [PATCH 2/2] Fix stack-use-after-scope error reported by ASAN by GCC 7 Martin Liška
2017-04-06 8:33 ` Jeff King
@ 2017-04-06 8:55 ` Johannes Schindelin
2017-04-06 10:23 ` [PATCH v2 " Martin Liška
1 sibling, 1 reply; 4+ messages in thread
From: Johannes Schindelin @ 2017-04-06 8:55 UTC (permalink / raw)
To: Martin Liška; +Cc: git
[-- Attachment #1: Type: text/plain, Size: 714 bytes --]
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 2/2] Fix stack-use-after-scope error reported by ASAN by GCC 7.
2017-04-06 8:55 ` Johannes Schindelin
@ 2017-04-06 10:23 ` Martin Liška
0 siblings, 0 replies; 4+ messages in thread
From: Martin Liška @ 2017-04-06 10:23 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
[-- Attachment #1: Type: text/plain, Size: 1006 bytes --]
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
>
[-- Attachment #2: 0002-Fix-stack-use-after-scope-error-reported-by-ASAN-by-.patch --]
[-- Type: text/x-patch, Size: 1341 bytes --]
From 4e80f9bddff3c29fefb3628e0e920ca265d55e65 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-04-06 10:24 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-06 8:02 [PATCH 2/2] Fix stack-use-after-scope error reported by ASAN by GCC 7 Martin Liška
2017-04-06 8:33 ` Jeff King
2017-04-06 8:55 ` Johannes Schindelin
2017-04-06 10:23 ` [PATCH v2 " Martin Liška
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.