From 83418a44912855d03b4117c7ff4ee31c362df9e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Gl=C3=A4=C3=9Fle?= Date: Thu, 15 Mar 2018 23:09:47 +0100 Subject: [PATCH 1/4] Fix duplicate sequence deletion after it has already been destroyed Fixes #602 --- src/mad_exec.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/mad_exec.c b/src/mad_exec.c index 5caac7725..2ef8107b0 100644 --- a/src/mad_exec.c +++ b/src/mad_exec.c @@ -13,7 +13,6 @@ exec_delete_sequ(const char* name) current_sequ->orbits = delete_vector_list(current_sequ->orbits); } sequences->sequs[spos] = delete_sequence(current_sequ); - remove_from_sequ_list(current_sequ, sequences); current_sequ = keep; } else warning("sequence to be deleted does not exist:", name); From 98949d4c219ee26691c19f40274e09594dc64ba3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Gl=C3=A4=C3=9Fle?= Date: Thu, 15 Mar 2018 23:16:10 +0100 Subject: [PATCH 2/4] Fix crash due to deleting another sequence after delete_sequence() `delete_sequence` already calls `remove_from_sequ_list` which fills `sequences->list[pos]` with the last sequence. Example: s1: SEQUENCE, l = 1; ENDSEQUENCE; s2: SEQUENCE, l = 1; ENDSEQUENCE; DELETE, SEQUENCE=s1; beam; use, sequence=s1; -> +++ memory access outside program range, fatal +++ --- src/mad_exec.c | 2 +- src/mad_seq.c | 7 +++---- src/mad_seq.h | 2 +- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/mad_exec.c b/src/mad_exec.c index 2ef8107b0..206e9185a 100644 --- a/src/mad_exec.c +++ b/src/mad_exec.c @@ -12,7 +12,7 @@ exec_delete_sequ(const char* name) current_sequ->ex_start = delete_node_ring(current_sequ->ex_start); current_sequ->orbits = delete_vector_list(current_sequ->orbits); } - sequences->sequs[spos] = delete_sequence(current_sequ); + delete_sequence(current_sequ); current_sequ = keep; } else warning("sequence to be deleted does not exist:", name); diff --git a/src/mad_seq.c b/src/mad_seq.c index fc1834121..cfe0f3b01 100644 --- a/src/mad_seq.c +++ b/src/mad_seq.c @@ -634,7 +634,7 @@ make_sequ_from_line(char* name) current_sequ = new_sequence(name, 0); /* node positions = centre */ old_sequ = find_sequence(name, sequences); add_to_sequ_list(current_sequ, sequences); - if (old_sequ) old_sequ = delete_sequence(old_sequ); + if (old_sequ) delete_sequence(old_sequ); if (current_sequ->cavities != NULL) current_sequ->cavities->curr = 0; else current_sequ->cavities = new_el_list(100); if (occ_list == NULL) @@ -1487,7 +1487,7 @@ new_sequ_node(struct sequence* sequ, int occ_cnt) return p; } -struct sequence* +void delete_sequence(struct sequence* sequ) { const char *rout_name = "delete_sequence"; @@ -1506,7 +1506,6 @@ delete_sequence(struct sequence* sequ) sequ->start = delete_node_ring(sequ->start); if (sequ->cavities) sequ->cavities = delete_el_list(sequ->cavities); myfree(rout_name, sequ); - return NULL; } void @@ -1572,7 +1571,7 @@ enter_sequence(struct in_cmd* cmd) // sequence exists already; delete the old one /*printf("enter_sequence: removing %s\n", sequences->sequs[pos]->name);*/ remove_from_sequ_list(sequences->sequs[pos], sequences); - sequences->sequs[pos] = delete_sequence(sequences->sequs[pos]); + delete_sequence(sequences->sequs[pos]); } current_sequ = new_sequence(toks[aux_pos], k); diff --git a/src/mad_seq.h b/src/mad_seq.h index e4b59822b..d87270fb8 100644 --- a/src/mad_seq.h +++ b/src/mad_seq.h @@ -68,7 +68,7 @@ struct sequence_list /* contains list of sequence pointers sorted by name */ struct node* new_sequ_node(struct sequence*, int occ_cnt); struct sequence* new_sequence(const char* name, int ref); -struct sequence* delete_sequence(struct sequence*); +void delete_sequence(struct sequence*); struct sequence_list* new_sequence_list(int length); struct sequence* find_sequence(const char* name, struct sequence_list*); From e404823f89dee6f8f25292a37e1c0b63a29da470 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Gl=C3=A4=C3=9Fle?= Date: Thu, 15 Mar 2018 23:42:22 +0100 Subject: [PATCH 3/4] Avoid a few more duplicate deletion statements --- src/mad_exec.c | 5 ----- src/mad_seq.c | 1 - 2 files changed, 6 deletions(-) diff --git a/src/mad_exec.c b/src/mad_exec.c index 206e9185a..9ed90f5dd 100644 --- a/src/mad_exec.c +++ b/src/mad_exec.c @@ -7,11 +7,6 @@ exec_delete_sequ(const char* name) int spos; if ((spos = name_list_pos(name, sequences->list)) >= 0) { current_sequ = sequences->sequs[spos]; - if (current_sequ->ex_start != NULL) { /* delete expanded */ - current_sequ->ex_nodes = delete_node_list(current_sequ->ex_nodes); - current_sequ->ex_start = delete_node_ring(current_sequ->ex_start); - current_sequ->orbits = delete_vector_list(current_sequ->orbits); - } delete_sequence(current_sequ); current_sequ = keep; } diff --git a/src/mad_seq.c b/src/mad_seq.c index cfe0f3b01..dd0ac8b76 100644 --- a/src/mad_seq.c +++ b/src/mad_seq.c @@ -1570,7 +1570,6 @@ enter_sequence(struct in_cmd* cmd) if ((pos = name_list_pos(toks[aux_pos], sequences->list)) >= 0) { // sequence exists already; delete the old one /*printf("enter_sequence: removing %s\n", sequences->sequs[pos]->name);*/ - remove_from_sequ_list(sequences->sequs[pos], sequences); delete_sequence(sequences->sequs[pos]); } From accce03091326cc580adda64da502864ddb23bf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Gl=C3=A4=C3=9Fle?= Date: Thu, 15 Mar 2018 23:37:02 +0100 Subject: [PATCH 4/4] Some cleanup --- src/mad_exec.c | 10 +++------- src/mad_seq.c | 16 +++++----------- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/src/mad_exec.c b/src/mad_exec.c index 9ed90f5dd..c6ba5f912 100644 --- a/src/mad_exec.c +++ b/src/mad_exec.c @@ -3,15 +3,11 @@ static void exec_delete_sequ(const char* name) { - struct sequence* keep = current_sequ; - int spos; - if ((spos = name_list_pos(name, sequences->list)) >= 0) { - current_sequ = sequences->sequs[spos]; - delete_sequence(current_sequ); - current_sequ = keep; + struct sequence* sequ = find_sequence(name, sequences); + if (sequ) { + delete_sequence(sequ); } else warning("sequence to be deleted does not exist:", name); - return; } void diff --git a/src/mad_seq.c b/src/mad_seq.c index dd0ac8b76..1854a7860 100644 --- a/src/mad_seq.c +++ b/src/mad_seq.c @@ -618,7 +618,6 @@ make_sequ_from_line(char* name) { char** tmp = NULL; int pos = name_list_pos(name, line_list->list); - struct sequence* old_sequ = NULL; struct macro* line; int mpos = name_list_pos("marker", defined_commands->list); struct command* clone = clone_command(defined_commands->commands[mpos]); @@ -632,9 +631,8 @@ make_sequ_from_line(char* name) expand_line(line_buffer); /* act on '-' and rep. count */ current_sequ = new_sequence(name, 0); /* node positions = centre */ - old_sequ = find_sequence(name, sequences); + delete_sequence(find_sequence(name, sequences)); add_to_sequ_list(current_sequ, sequences); - if (old_sequ) delete_sequence(old_sequ); if (current_sequ->cavities != NULL) current_sequ->cavities->curr = 0; else current_sequ->cavities = new_el_list(100); if (occ_list == NULL) @@ -1491,7 +1489,7 @@ void delete_sequence(struct sequence* sequ) { const char *rout_name = "delete_sequence"; - struct sequence* seq; + if (!sequ) return; if (sequ->ex_start != NULL) { sequ->ex_nodes = delete_node_list(sequ->ex_nodes); @@ -1499,8 +1497,8 @@ delete_sequence(struct sequence* sequ) sequ->orbits = delete_vector_list(sequ->orbits); myfree(rout_name, sequ->all_nodes); } - if ((seq = find_sequence(sequ->name, sequences))) - remove_from_sequ_list(seq, sequences); + if (sequ == find_sequence(sequ->name, sequences)) + remove_from_sequ_list(sequ, sequences); if (sequ->l_expr) sequ->l_expr = delete_expression(sequ->l_expr); sequ->nodes = delete_node_list(sequ->nodes); sequ->start = delete_node_ring(sequ->start); @@ -1567,11 +1565,7 @@ enter_sequence(struct in_cmd* cmd) } } - if ((pos = name_list_pos(toks[aux_pos], sequences->list)) >= 0) { - // sequence exists already; delete the old one - /*printf("enter_sequence: removing %s\n", sequences->sequs[pos]->name);*/ - delete_sequence(sequences->sequs[pos]); - } + delete_sequence(find_sequence(toks[aux_pos], sequences)); current_sequ = new_sequence(toks[aux_pos], k); add_to_sequ_list(current_sequ, sequences);