Skip to content

Commit

Permalink
gdb: change subfile::line_vector to an std::vector
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@simark
simark committed Apr 12, 2022
1 parent b08c778 commit 558802e
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 151 deletions.
94 changes: 34 additions & 60 deletions gdb/buildsym.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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_,
Expand Down Expand Up @@ -95,7 +88,6 @@ buildsym_compunit::~buildsym_compunit ()
subfile = nextsub)
{
nextsub = subfile->next;
xfree (subfile->line_vector);
delete subfile;
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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<int> 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;
}


Expand All @@ -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 ());
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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);
}

Expand All @@ -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);
Expand Down
6 changes: 2 additions & 4 deletions gdb/buildsym.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#define BUILDSYM_H 1

#include "gdbsupport/gdb_obstack.h"
#include "symtab.h"

struct objfile;
struct symbol;
Expand Down Expand Up @@ -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<linetable_entry> line_vector_entries;
enum language language = language_unknown;
struct symtab *symtab = nullptr;
};
Expand Down
Loading

0 comments on commit 558802e

Please sign in to comment.