From 558802e4d1c5dcbd0df7d2c6ef62a6deac247a2f Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Thu, 7 Apr 2022 08:55:16 -0400 Subject: [PATCH] gdb: change subfile::line_vector to an std::vector Change this field to an std::vector to facilitate memory management. Since the linetable_entry array is copied into the symtab resulting from the subfile, it is possible to change it without changing how symtab stores the linetable entries (which would be a much larger change). There is a small change in buildsym_compunit::record_line to avoid accessing a now invalid linetable_entry. Before this patch, we keep a pointer to the last linetable entry, pop it from the vector, and then read last->line. It works with the manually-maintained array, but since we now use std::vector::pop_back, I am afraid that it could be flagged as an invalid access by the various static / dynamic analysis tools to access the linetable_entry object after popping it from the vector. Instead, record just the line number in an optional and use it. There are substantial changes in xcoffread.c that simplify the code, but I can't test them. I was hesitant to do this change because of that, but I decided to send it anyway. I don't think that an almost dead platform should hold back improving the code in the common parts of GDB. The changes in xcoffread.c are: - Make arrange_linetable "arrange" the linetable passed as a parameter, instead of returning possibly a new one, possibly the same one. - In the "Process main file's line numbers.", I'm not too sure what happens. We get the lintable from "main_subfile", "arrange" it, but then assign the result to the current subfile, obtained with get_current_subfile. I assume that the current subfile is also the main one, so now I just call arrange_linetable on the main subfile's line table. - Remove that weird "Useless if!!!" FIXME comment. It's been there forever, but the "if" is still there, so I guess the "if" can stay there. Change-Id: I11799006fd85189e8cf5bd3a168f8f38c2c27a80 --- gdb/buildsym.c | 94 ++++++++++++++------------------------ gdb/buildsym.h | 6 +-- gdb/xcoffread.c | 118 +++++++++++++----------------------------------- 3 files changed, 67 insertions(+), 151 deletions(-) diff --git a/gdb/buildsym.c b/gdb/buildsym.c index 034db598735..628903d674f 100644 --- a/gdb/buildsym.c +++ b/gdb/buildsym.c @@ -49,13 +49,6 @@ struct pending_block struct block *block; }; -/* Initial sizes of data structures. These are realloc'd larger if - needed, and realloc'd down to the size actually used, when - completed. */ - -#define INITIAL_LINE_VECTOR_LENGTH 1000 - - buildsym_compunit::buildsym_compunit (struct objfile *objfile_, const char *name, const char *comp_dir_, @@ -95,7 +88,6 @@ buildsym_compunit::~buildsym_compunit () subfile = nextsub) { nextsub = subfile->next; - xfree (subfile->line_vector); delete subfile; } @@ -536,9 +528,6 @@ buildsym_compunit::start_subfile (const char *name) m_current_subfile = subfile.get (); - /* Initialize line-number recording for this subfile. */ - subfile->line_vector = NULL; - /* Default the source language to whatever can be deduced from the filename. If nothing can be deduced (such as for a C/C++ include file with a ".h" extension), then inherit whatever language the @@ -657,28 +646,7 @@ void buildsym_compunit::record_line (struct subfile *subfile, int line, CORE_ADDR pc, linetable_entry_flags flags) { - struct linetable_entry *e; - - /* Make sure line vector exists and is big enough. */ - if (!subfile->line_vector) - { - subfile->line_vector_length = INITIAL_LINE_VECTOR_LENGTH; - subfile->line_vector = (struct linetable *) - xmalloc (sizeof (struct linetable) - + subfile->line_vector_length * sizeof (struct linetable_entry)); - subfile->line_vector->nitems = 0; - m_have_line_numbers = true; - } - - if (subfile->line_vector->nitems >= subfile->line_vector_length) - { - subfile->line_vector_length *= 2; - subfile->line_vector = (struct linetable *) - xrealloc ((char *) subfile->line_vector, - (sizeof (struct linetable) - + (subfile->line_vector_length - * sizeof (struct linetable_entry)))); - } + m_have_line_numbers = true; /* Normally, we treat lines as unsorted. But the end of sequence marker is special. We sort line markers at the same PC by line @@ -695,25 +663,30 @@ buildsym_compunit::record_line (struct subfile *subfile, int line, anyway. */ if (line == 0) { - struct linetable_entry *last = nullptr; - while (subfile->line_vector->nitems > 0) + gdb::optional last_line; + + while (!subfile->line_vector_entries.empty ()) { - last = subfile->line_vector->item + subfile->line_vector->nitems - 1; + linetable_entry *last = &subfile->line_vector_entries.back (); + last_line = last->line; + if (last->pc != pc) break; - subfile->line_vector->nitems--; + + subfile->line_vector_entries.pop_back (); } /* Ignore an end-of-sequence marker marking an empty sequence. */ - if (last == nullptr || last->line == 0) + if (!last_line.has_value () || *last_line == 0) return; } - e = subfile->line_vector->item + subfile->line_vector->nitems++; - e->line = line; - e->is_stmt = (flags & LEF_IS_STMT) != 0; - e->pc = pc; - e->prologue_end = (flags & LEF_PROLOGUE_END) != 0; + subfile->line_vector_entries.emplace_back (); + linetable_entry &e = subfile->line_vector_entries.back (); + e.line = line; + e.is_stmt = (flags & LEF_IS_STMT) != 0; + e.pc = pc; + e.prologue_end = (flags & LEF_PROLOGUE_END) != 0; } @@ -738,7 +711,7 @@ buildsym_compunit::watch_main_source_file_lossage () /* If the main source file doesn't have any line number or symbol info, look for an alias in another subfile. */ - if (mainsub->line_vector == NULL + if (mainsub->line_vector_entries.empty () && mainsub->symtab == NULL) { const char *mainbase = lbasename (mainsub->name.c_str ()); @@ -771,8 +744,8 @@ buildsym_compunit::watch_main_source_file_lossage () Copy its line_vector and symtab to the main subfile and then discard it. */ - mainsub->line_vector = mainsub_alias->line_vector; - mainsub->line_vector_length = mainsub_alias->line_vector_length; + mainsub->line_vector_entries + = std::move (mainsub_alias->line_vector_entries); mainsub->symtab = mainsub_alias->symtab; if (prev_mainsub_alias == NULL) @@ -929,13 +902,8 @@ buildsym_compunit::end_compunit_symtab_with_blockvector subfile != NULL; subfile = subfile->next) { - int linetablesize = 0; - - if (subfile->line_vector) + if (!subfile->line_vector_entries.empty ()) { - linetablesize = sizeof (struct linetable) + - subfile->line_vector->nitems * sizeof (struct linetable_entry); - const auto lte_is_less_than = [] (const linetable_entry &ln1, const linetable_entry &ln2) -> bool @@ -953,9 +921,8 @@ buildsym_compunit::end_compunit_symtab_with_blockvector address, as this maintains the inline function caller/callee relationships, this is why std::stable_sort is used. */ if (m_objfile->flags & OBJF_REORDERED) - std::stable_sort (subfile->line_vector->item, - subfile->line_vector->item - + subfile->line_vector->nitems, + std::stable_sort (subfile->line_vector_entries.begin (), + subfile->line_vector_entries.end (), lte_is_less_than); } @@ -967,13 +934,20 @@ buildsym_compunit::end_compunit_symtab_with_blockvector /* Fill in its components. */ - if (subfile->line_vector) + if (!subfile->line_vector_entries.empty ()) { - /* Reallocate the line table on the symbol obstack. */ + /* Reallocate the line table on the objfile obstack. */ + size_t n_entries = subfile->line_vector_entries.size (); + size_t entry_array_size = n_entries * sizeof (struct linetable_entry); + int linetablesize = sizeof (struct linetable) + entry_array_size; + symtab->set_linetable - ((struct linetable *) - obstack_alloc (&m_objfile->objfile_obstack, linetablesize)); - memcpy (symtab->linetable (), subfile->line_vector, linetablesize); + (XOBNEWVAR (&m_objfile->objfile_obstack, struct linetable, + linetablesize)); + + symtab->linetable ()->nitems = n_entries; + memcpy (symtab->linetable ()->item, + subfile->line_vector_entries.data (), entry_array_size); } else symtab->set_linetable (nullptr); diff --git a/gdb/buildsym.h b/gdb/buildsym.h index 6284aafc878..ee75e6fd95d 100644 --- a/gdb/buildsym.h +++ b/gdb/buildsym.h @@ -20,6 +20,7 @@ #define BUILDSYM_H 1 #include "gdbsupport/gdb_obstack.h" +#include "symtab.h" struct objfile; struct symbol; @@ -53,10 +54,7 @@ struct subfile struct subfile *next = nullptr; std::string name; - - /* Space for this is malloc'd. */ - struct linetable *line_vector = nullptr; - int line_vector_length = 0; + std::vector line_vector_entries; enum language language = language_unknown; struct symtab *symtab = nullptr; }; diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c index b89e98ae05f..566c0824cd5 100644 --- a/gdb/xcoffread.c +++ b/gdb/xcoffread.c @@ -239,8 +239,6 @@ static void read_xcoff_symtab (struct objfile *, legacy_psymtab *); static void add_stab_to_list (char *, struct pending_stabs **); #endif -static struct linetable *arrange_linetable (struct linetable *); - static void record_include_end (struct coff_symbol *); static void process_linenos (CORE_ADDR, CORE_ADDR); @@ -412,89 +410,77 @@ add_stab_to_list (char *stabname, struct pending_stabs **stabvector) /* Given a line table with function entries are marked, arrange its functions in ascending order and strip off function entry markers - and return it in a newly created table. If the old one is good - enough, return the old one. */ + and return it in a newly created table. */ + /* FIXME: I think all this stuff can be replaced by just passing sort_linevec = 1 to end_compunit_symtab. */ -static struct linetable * -arrange_linetable (struct linetable *oldLineTb) +static void +arrange_linetable (std::vector &old_linetable) { int extra_lines = 0; std::vector fentries; - for (int ii = 0; ii < oldLineTb->nitems; ++ii) + for (int ii = 0; ii < old_linetable.size (); ++ii) { - if (oldLineTb->item[ii].is_stmt == 0) + if (old_linetable[ii].is_stmt == 0) continue; - if (oldLineTb->item[ii].line == 0) + if (old_linetable[ii].line == 0) { /* Function entry found. */ fentries.emplace_back (); linetable_entry &e = fentries.back (); e.line = ii; e.is_stmt = 1; - e.pc = oldLineTb->item[ii].pc; + e.pc = old_linetable[ii].pc; /* If the function was compiled with XLC, we may have to add an extra line entry later. Reserve space for that. */ - if (ii + 1 < oldLineTb->nitems - && oldLineTb->item[ii].pc != oldLineTb->item[ii + 1].pc) + if (ii + 1 < old_linetable.size () + && old_linetable[ii].pc != old_linetable[ii + 1].pc) extra_lines++; } } if (fentries.empty ()) - return oldLineTb; + return; std::sort (fentries.begin (), fentries.end (), [] (const linetable_entry <e1, const linetable_entry& lte2) { return lte1.pc < lte2.pc; }); /* Allocate a new line table. */ - int new_linetable_nitems = oldLineTb->nitems - fentries.size () + extra_lines; - linetable *new_linetable - = XNEWVAR (linetable, - (sizeof (struct linetable) - + (new_linetable_nitems * sizeof (struct linetable_entry)))); + std::vector new_linetable; + new_linetable.reserve (old_linetable.size ()); /* If line table does not start with a function beginning, copy up until a function begin. */ - - int newline = 0; - if (oldLineTb->item[0].line != 0) - for (newline = 0; - newline < oldLineTb->nitems && oldLineTb->item[newline].line; - ++newline) - new_linetable->item[newline] = oldLineTb->item[newline]; + for (int i = 0; i < old_linetable.size () && old_linetable[i].line != 0; ++i) + new_linetable.push_back (old_linetable[i]); /* Now copy function lines one by one. */ - for (const linetable_entry &entry : fentries) { /* If the function was compiled with XLC, we may have to add an extra line to cover the function prologue. */ int jj = entry.line; - if (jj + 1 < oldLineTb->nitems - && oldLineTb->item[jj].pc != oldLineTb->item[jj + 1].pc) + if (jj + 1 < old_linetable.size () + && old_linetable[jj].pc != old_linetable[jj + 1].pc) { - new_linetable->item[newline] = oldLineTb->item[jj]; - new_linetable->item[newline].line = oldLineTb->item[jj + 1].line; - newline++; + new_linetable.push_back (old_linetable[jj]); + new_linetable.back ().line = old_linetable[jj + 1].line; } for (jj = entry.line + 1; - jj < oldLineTb->nitems && oldLineTb->item[jj].line != 0; - ++jj, ++newline) - new_linetable->item[newline] = oldLineTb->item[jj]; + jj < old_linetable.size () && old_linetable[jj].line != 0; + ++jj) + new_linetable.push_back (old_linetable[jj]); } - /* The number of items in the line table must include these - extra lines which were added in case of XLC compiled functions. */ - new_linetable->nitems = new_linetable_nitems; - return new_linetable; + new_linetable.shrink_to_fit (); + old_linetable = std::move (new_linetable); } /* include file support: C_BINCL/C_EINCL pairs will be kept in the @@ -595,7 +581,7 @@ static struct objfile *this_symtab_objfile; static void process_linenos (CORE_ADDR start, CORE_ADDR end) { - int offset, ii; + int offset; file_ptr max_offset = XCOFF_DATA (this_symtab_objfile)->max_lineno_offset; @@ -631,7 +617,7 @@ process_linenos (CORE_ADDR start, CORE_ADDR end) coff_data (this_symtab_objfile->obfd)->local_linesz; main_source_baseline = 0; - for (ii = 0; ii < inclIndx; ++ii) + for (int ii = 0; ii < inclIndx; ++ii) { /* If there is main file source before include file, enter it. */ if (offset < inclTable[ii].begin) @@ -678,49 +664,23 @@ process_linenos (CORE_ADDR start, CORE_ADDR end) } /* Process main file's line numbers. */ - if (main_subfile.line_vector) + if (!main_subfile.line_vector_entries.empty ()) { - struct linetable *lineTb, *lv; - - lv = main_subfile.line_vector; - /* Line numbers are not necessarily ordered. xlc compilation will put static function to the end. */ - - struct subfile *current_subfile = get_current_subfile (); - lineTb = arrange_linetable (lv); - if (lv == lineTb) - { - current_subfile->line_vector = (struct linetable *) - xrealloc (lv, (sizeof (struct linetable) - + lv->nitems * sizeof (struct linetable_entry))); - } - else - { - xfree (lv); - current_subfile->line_vector = lineTb; - } - - current_subfile->line_vector_length = - current_subfile->line_vector->nitems; + arrange_linetable (main_subfile.line_vector_entries); } /* Now, process included files' line numbers. */ - for (ii = 0; ii < inclIndx; ++ii) + for (int ii = 0; ii < inclIndx; ++ii) { if (inclTable[ii].subfile != ((struct subfile *) &main_subfile) - && (inclTable[ii].subfile)->line_vector) /* Useless if!!! - FIXMEmgo */ + && !inclTable[ii].subfile->line_vector_entries.empty ()) { - struct linetable *lineTb, *lv; - - lv = (inclTable[ii].subfile)->line_vector; - /* Line numbers are not necessarily ordered. xlc compilation will put static function to the end. */ - - lineTb = arrange_linetable (lv); + arrange_linetable (inclTable[ii].subfile->line_vector_entries); push_subfile (); @@ -755,22 +715,6 @@ process_linenos (CORE_ADDR start, CORE_ADDR end) current_subfile->name = inclTable[ii].name; #endif - if (lv == lineTb) - { - current_subfile->line_vector = - (struct linetable *) xrealloc - (lv, (sizeof (struct linetable) - + lv->nitems * sizeof (struct linetable_entry))); - - } - else - { - xfree (lv); - current_subfile->line_vector = lineTb; - } - - current_subfile->line_vector_length = - current_subfile->line_vector->nitems; start_subfile (pop_subfile ()); } } -- 2.30.2