From 55089490f79ce1ddb9610fd6abeeaf896825fb71 Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Sat, 24 Feb 2018 09:55:30 -0700 Subject: [PATCH] Change target_write_memory_blocks to use std::vector This changes target_write_memory_blocks to use std::vector, rather than VEC. This allows the removal of some cleanups. This version incorporates the additions that Simon made. Regression tested by the buildbot. ChangeLog 2018-02-27 Simon Marchi Tom Tromey * target.h (memory_write_request_s): Remove typedef. Don't define VEC. (target_write_memory_blocks): Change argument to std::vector. (struct memory_write_request): Add constructor. * target-memory.c (compare_block_starting_address): Return bool. Change argument types. (claim_memory): Change arguments to use std::vector. (split_regular_and_flash_blocks, blocks_to_erase) (compute_garbled_blocks): Likewise. (cleanup_request_data, cleanup_write_requests_vector): Remove. (target_write_memory_blocks): Change argument to std::vector. * symfile.c (struct load_section_data): Add constructor and destructor. Use std::vector for "requests". (struct load_progress_data): Add initializers. (load_section_callback): Update. Use "new". (clear_memory_write_data): Remove. (generic_load): Update. --- gdb/ChangeLog | 21 +++++ gdb/symfile.c | 109 ++++++++++--------------- gdb/target-memory.c | 231 +++++++++++++++++++--------------------------------- gdb/target.h | 34 ++++---- 4 files changed, 167 insertions(+), 228 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index dc9ce42..33fbaac 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,24 @@ +2018-02-27 Simon Marchi + Tom Tromey + + * target.h (memory_write_request_s): Remove typedef. Don't define + VEC. + (target_write_memory_blocks): Change argument to std::vector. + (struct memory_write_request): Add constructor. + * target-memory.c (compare_block_starting_address): Return bool. + Change argument types. + (claim_memory): Change arguments to use std::vector. + (split_regular_and_flash_blocks, blocks_to_erase) + (compute_garbled_blocks): Likewise. + (cleanup_request_data, cleanup_write_requests_vector): Remove. + (target_write_memory_blocks): Change argument to std::vector. + * symfile.c (struct load_section_data): Add constructor and + destructor. Use std::vector for "requests". + (struct load_progress_data): Add initializers. + (load_section_callback): Update. Use "new". + (clear_memory_write_data): Remove. + (generic_load): Update. + 2018-02-27 Alan Hayward * arch/aarch64.h: Use common/tdesc.h. diff --git a/gdb/symfile.c b/gdb/symfile.c index 699d9e6..2a1428b 100644 --- a/gdb/symfile.c +++ b/gdb/symfile.c @@ -1892,33 +1892,56 @@ add_section_size_callback (bfd *abfd, asection *asec, void *data) *sum += bfd_get_section_size (asec); } -/* Opaque data for load_section_callback. */ -struct load_section_data { - CORE_ADDR load_offset; - struct load_progress_data *progress_data; - VEC(memory_write_request_s) *requests; -}; - /* Opaque data for load_progress. */ -struct load_progress_data { +struct load_progress_data +{ /* Cumulative data. */ - unsigned long write_count; - unsigned long data_count; - bfd_size_type total_size; + unsigned long write_count = 0; + unsigned long data_count = 0; + bfd_size_type total_size = 0; }; /* Opaque data for load_progress for a single section. */ -struct load_progress_section_data { +struct load_progress_section_data +{ + load_progress_section_data (load_progress_data *cumulative_, + const char *section_name_, ULONGEST section_size_, + CORE_ADDR lma_, gdb_byte *buffer_) + : cumulative (cumulative_), section_name (section_name_), + section_size (section_size_), lma (lma_), buffer (buffer_) + {} + struct load_progress_data *cumulative; /* Per-section data. */ const char *section_name; - ULONGEST section_sent; + ULONGEST section_sent = 0; ULONGEST section_size; CORE_ADDR lma; gdb_byte *buffer; }; +/* Opaque data for load_section_callback. */ +struct load_section_data +{ + load_section_data (load_progress_data *progress_data_) + : progress_data (progress_data_) + {} + + ~load_section_data () + { + for (auto &&request : requests) + { + xfree (request.data); + delete ((load_progress_section_data *) request.baton); + } + } + + CORE_ADDR load_offset = 0; + struct load_progress_data *progress_data; + std::vector requests; +}; + /* Target write callback routine for progress reporting. */ static void @@ -1988,11 +2011,8 @@ load_progress (ULONGEST bytes, void *untyped_arg) static void load_section_callback (bfd *abfd, asection *asec, void *data) { - struct memory_write_request *new_request; struct load_section_data *args = (struct load_section_data *) data; - struct load_progress_section_data *section_data; bfd_size_type size = bfd_get_section_size (asec); - gdb_byte *buffer; const char *sect_name = bfd_get_section_name (abfd, asec); if ((bfd_get_section_flags (abfd, asec) & SEC_LOAD) == 0) @@ -2001,44 +2021,16 @@ load_section_callback (bfd *abfd, asection *asec, void *data) if (size == 0) return; - new_request = VEC_safe_push (memory_write_request_s, - args->requests, NULL); - memset (new_request, 0, sizeof (struct memory_write_request)); - section_data = XCNEW (struct load_progress_section_data); - new_request->begin = bfd_section_lma (abfd, asec) + args->load_offset; - new_request->end = new_request->begin + size; /* FIXME Should size - be in instead? */ - new_request->data = (gdb_byte *) xmalloc (size); - new_request->baton = section_data; - - buffer = new_request->data; - - section_data->cumulative = args->progress_data; - section_data->section_name = sect_name; - section_data->section_size = size; - section_data->lma = new_request->begin; - section_data->buffer = buffer; - + ULONGEST begin = bfd_section_lma (abfd, asec) + args->load_offset; + ULONGEST end = begin + size; + gdb_byte *buffer = (gdb_byte *) xmalloc (size); bfd_get_section_contents (abfd, asec, buffer, 0, size); -} -/* Clean up an entire memory request vector, including load - data and progress records. */ + load_progress_section_data *section_data + = new load_progress_section_data (args->progress_data, sect_name, size, + begin, buffer); -static void -clear_memory_write_data (void *arg) -{ - VEC(memory_write_request_s) **vec_p = (VEC(memory_write_request_s) **) arg; - VEC(memory_write_request_s) *vec = *vec_p; - int i; - struct memory_write_request *mr; - - for (i = 0; VEC_iterate (memory_write_request_s, vec, i, mr); ++i) - { - xfree (mr->data); - xfree (mr->baton); - } - VEC_free (memory_write_request_s, vec); + args->requests.emplace_back (begin, end, buffer, section_data); } static void print_transfer_performance (struct ui_file *stream, @@ -2049,19 +2041,10 @@ static void print_transfer_performance (struct ui_file *stream, void generic_load (const char *args, int from_tty) { - struct cleanup *old_cleanups; - struct load_section_data cbdata; struct load_progress_data total_progress; + struct load_section_data cbdata (&total_progress); struct ui_out *uiout = current_uiout; - CORE_ADDR entry; - - memset (&cbdata, 0, sizeof (cbdata)); - memset (&total_progress, 0, sizeof (total_progress)); - cbdata.progress_data = &total_progress; - - old_cleanups = make_cleanup (clear_memory_write_data, &cbdata.requests); - if (args == NULL) error_no_arg (_("file to load")); @@ -2110,7 +2093,7 @@ generic_load (const char *args, int from_tty) steady_clock::time_point end_time = steady_clock::now (); - entry = bfd_get_start_address (loadfile_bfd.get ()); + CORE_ADDR entry = bfd_get_start_address (loadfile_bfd.get ()); entry = gdbarch_addr_bits_remove (target_gdbarch (), entry); uiout->text ("Start address "); uiout->field_fmt ("address", "%s", paddress (target_gdbarch (), entry)); @@ -2132,8 +2115,6 @@ generic_load (const char *args, int from_tty) print_transfer_performance (gdb_stdout, total_progress.data_count, total_progress.write_count, end_time - start_time); - - do_cleanups (old_cleanups); } /* Report on STREAM the performance of a memory transfer operation, diff --git a/gdb/target-memory.c b/gdb/target-memory.c index 81faa22..395bf0b 100644 --- a/gdb/target-memory.c +++ b/gdb/target-memory.c @@ -26,20 +26,11 @@ #include "gdb_sys_time.h" #include -static int -compare_block_starting_address (const void *a, const void *b) +static bool +compare_block_starting_address (const memory_write_request &a_req, + const memory_write_request &b_req) { - const struct memory_write_request *a_req - = (const struct memory_write_request *) a; - const struct memory_write_request *b_req - = (const struct memory_write_request *) b; - - if (a_req->begin < b_req->begin) - return -1; - else if (a_req->begin == b_req->begin) - return 0; - else - return 1; + return a_req.begin < b_req.begin; } /* Adds to RESULT all memory write requests from BLOCK that are @@ -49,17 +40,15 @@ compare_block_starting_address (const void *a, const void *b) that part of the memory request will be added. */ static void -claim_memory (VEC(memory_write_request_s) *blocks, - VEC(memory_write_request_s) **result, +claim_memory (const std::vector &blocks, + std::vector *result, ULONGEST begin, ULONGEST end) { - int i; ULONGEST claimed_begin; ULONGEST claimed_end; - struct memory_write_request *r; - for (i = 0; VEC_iterate (memory_write_request_s, blocks, i, r); ++i) + for (const memory_write_request &r : blocks) { /* If the request doesn't overlap [BEGIN, END), skip it. We must handle END == 0 meaning the top of memory; we don't yet @@ -67,28 +56,28 @@ claim_memory (VEC(memory_write_request_s) *blocks, memory, but there's an assertion in target_write_memory_blocks which checks for that. */ - if (begin >= r->end) + if (begin >= r.end) continue; - if (end != 0 && end <= r->begin) + if (end != 0 && end <= r.begin) continue; - claimed_begin = std::max (begin, r->begin); + claimed_begin = std::max (begin, r.begin); if (end == 0) - claimed_end = r->end; + claimed_end = r.end; else - claimed_end = std::min (end, r->end); + claimed_end = std::min (end, r.end); - if (claimed_begin == r->begin && claimed_end == r->end) - VEC_safe_push (memory_write_request_s, *result, r); + if (claimed_begin == r.begin && claimed_end == r.end) + result->push_back (r); else { - struct memory_write_request *n = - VEC_safe_push (memory_write_request_s, *result, NULL); + struct memory_write_request n = r; + + n.begin = claimed_begin; + n.end = claimed_end; + n.data += claimed_begin - r.begin; - *n = *r; - n->begin = claimed_begin; - n->end = claimed_end; - n->data += claimed_begin - r->begin; + result->push_back (n); } } } @@ -98,9 +87,9 @@ claim_memory (VEC(memory_write_request_s) *blocks, regular memory to REGULAR_BLOCKS. */ static void -split_regular_and_flash_blocks (VEC(memory_write_request_s) *blocks, - VEC(memory_write_request_s) **regular_blocks, - VEC(memory_write_request_s) **flash_blocks) +split_regular_and_flash_blocks (const std::vector &blocks, + std::vector *regular_blocks, + std::vector *flash_blocks) { struct mem_region *region; CORE_ADDR cur_address; @@ -116,7 +105,7 @@ split_regular_and_flash_blocks (VEC(memory_write_request_s) *blocks, cur_address = 0; while (1) { - VEC(memory_write_request_s) **r; + std::vector *r; region = lookup_mem_region (cur_address); r = region->attrib.mode == MEM_FLASH ? flash_blocks : regular_blocks; @@ -156,35 +145,22 @@ block_boundaries (CORE_ADDR address, CORE_ADDR *begin, CORE_ADDR *end) returns write requests covering each group of flash blocks which must be erased. */ -static VEC(memory_write_request_s) * -blocks_to_erase (VEC(memory_write_request_s) *written) +static std::vector +blocks_to_erase (const std::vector &written) { - unsigned i; - struct memory_write_request *ptr; - - VEC(memory_write_request_s) *result = NULL; + std::vector result; - for (i = 0; VEC_iterate (memory_write_request_s, written, i, ptr); ++i) + for (const memory_write_request &request : written) { CORE_ADDR begin, end; - block_boundaries (ptr->begin, &begin, 0); - block_boundaries (ptr->end - 1, 0, &end); + block_boundaries (request.begin, &begin, 0); + block_boundaries (request.end - 1, 0, &end); - if (!VEC_empty (memory_write_request_s, result) - && VEC_last (memory_write_request_s, result)->end >= begin) - { - VEC_last (memory_write_request_s, result)->end = end; - } + if (!result.empty () && result.back ().end >= begin) + result.back ().end = end; else - { - struct memory_write_request *n = - VEC_safe_push (memory_write_request_s, result, NULL); - - memset (n, 0, sizeof (struct memory_write_request)); - n->begin = begin; - n->end = end; - } + result.emplace_back (begin, end); } return result; @@ -196,14 +172,14 @@ blocks_to_erase (VEC(memory_write_request_s) *written) that will be erased but not rewritten (e.g. padding within a block which is only partially filled by "load"). */ -static VEC(memory_write_request_s) * -compute_garbled_blocks (VEC(memory_write_request_s) *erased_blocks, - VEC(memory_write_request_s) *written_blocks) +static std::vector +compute_garbled_blocks (const std::vector &erased_blocks, + const std::vector &written_blocks) { - VEC(memory_write_request_s) *result = NULL; + std::vector result; - unsigned i, j; - unsigned je = VEC_length (memory_write_request_s, written_blocks); + unsigned j; + unsigned je = written_blocks.size (); struct memory_write_request *erased_p; /* Look at each erased memory_write_request in turn, and @@ -213,19 +189,15 @@ compute_garbled_blocks (VEC(memory_write_request_s) *erased_blocks, the lists are sorted at this point it could be rewritten more efficiently, but the complexity is not generally worthwhile. */ - for (i = 0; - VEC_iterate (memory_write_request_s, erased_blocks, i, erased_p); - ++i) + for (const memory_write_request &erased_iter : erased_blocks) { /* Make a deep copy -- it will be modified inside the loop, but we don't want to modify original vector. */ - struct memory_write_request erased = *erased_p; + struct memory_write_request erased = erased_iter; for (j = 0; j != je;) { - struct memory_write_request *written - = VEC_index (memory_write_request_s, - written_blocks, j); + const memory_write_request *written = &written_blocks[j]; /* Now try various cases. */ @@ -242,7 +214,7 @@ compute_garbled_blocks (VEC(memory_write_request_s) *erased_blocks, blocks. */ if (written->begin >= erased.end) { - VEC_safe_push (memory_write_request_s, result, &erased); + result.push_back (erased); goto next_erased; } @@ -259,12 +231,7 @@ compute_garbled_blocks (VEC(memory_write_request_s) *erased_blocks, again for the remainder. */ if (written->begin > erased.begin) { - struct memory_write_request *n = - VEC_safe_push (memory_write_request_s, result, NULL); - - memset (n, 0, sizeof (struct memory_write_request)); - n->begin = erased.begin; - n->end = written->begin; + result.emplace_back (erased.begin, written->begin); erased.begin = written->begin; continue; } @@ -283,7 +250,7 @@ compute_garbled_blocks (VEC(memory_write_request_s) *erased_blocks, /* If we ran out of write requests without doing anything about ERASED, then that means it's really erased. */ - VEC_safe_push (memory_write_request_s, result, &erased); + result.push_back (erased); next_erased: ; @@ -292,59 +259,30 @@ compute_garbled_blocks (VEC(memory_write_request_s) *erased_blocks, return result; } -static void -cleanup_request_data (void *p) -{ - VEC(memory_write_request_s) **v = (VEC(memory_write_request_s) **) p; - struct memory_write_request *r; - int i; - - for (i = 0; VEC_iterate (memory_write_request_s, *v, i, r); ++i) - xfree (r->data); -} - -static void -cleanup_write_requests_vector (void *p) -{ - VEC(memory_write_request_s) **v = (VEC(memory_write_request_s) **) p; - - VEC_free (memory_write_request_s, *v); -} - int -target_write_memory_blocks (VEC(memory_write_request_s) *requests, +target_write_memory_blocks (const std::vector &requests, enum flash_preserve_mode preserve_flash_p, void (*progress_cb) (ULONGEST, void *)) { - struct cleanup *back_to = make_cleanup (null_cleanup, NULL); - VEC(memory_write_request_s) *blocks = VEC_copy (memory_write_request_s, - requests); - unsigned i; - int err = 0; + std::vector blocks = requests; struct memory_write_request *r; - VEC(memory_write_request_s) *regular = NULL; - VEC(memory_write_request_s) *flash = NULL; - VEC(memory_write_request_s) *erased, *garbled; + std::vector regular; + std::vector flash; + std::vector erased, garbled; /* END == 0 would represent wraparound: a write to the very last byte of the address space. This file was not written with that possibility in mind. This is fixable, but a lot of work for a rare problem; so for now, fail noisily here instead of obscurely later. */ - for (i = 0; VEC_iterate (memory_write_request_s, requests, i, r); ++i) - gdb_assert (r->end != 0); - - make_cleanup (cleanup_write_requests_vector, &blocks); + for (const memory_write_request &iter : requests) + gdb_assert (iter.end != 0); /* Sort the blocks by their start address. */ - qsort (VEC_address (memory_write_request_s, blocks), - VEC_length (memory_write_request_s, blocks), - sizeof (struct memory_write_request), compare_block_starting_address); + std::sort (blocks.begin (), blocks.end (), compare_block_starting_address); /* Split blocks into list of regular memory blocks, and list of flash memory blocks. */ - make_cleanup (cleanup_write_requests_vector, ®ular); - make_cleanup (cleanup_write_requests_vector, &flash); split_regular_and_flash_blocks (blocks, ®ular, &flash); /* If a variable is added to forbid flash write, even during "load", @@ -354,37 +292,35 @@ target_write_memory_blocks (VEC(memory_write_request_s) *requests, /* Find flash blocks to erase. */ erased = blocks_to_erase (flash); - make_cleanup (cleanup_write_requests_vector, &erased); /* Find what flash regions will be erased, and not overwritten; then either preserve or discard the old contents. */ garbled = compute_garbled_blocks (erased, flash); - make_cleanup (cleanup_request_data, &garbled); - make_cleanup (cleanup_write_requests_vector, &garbled); - if (!VEC_empty (memory_write_request_s, garbled)) + std::vector> mem_holders; + if (!garbled.empty ()) { if (preserve_flash_p == flash_preserve) { - struct memory_write_request *r; - /* Read in regions that must be preserved and add them to the list of blocks we read. */ - for (i = 0; VEC_iterate (memory_write_request_s, garbled, i, r); ++i) + for (memory_write_request &iter : garbled) { - gdb_assert (r->data == NULL); - r->data = (gdb_byte *) xmalloc (r->end - r->begin); - err = target_read_memory (r->begin, r->data, r->end - r->begin); + gdb_assert (iter.data == NULL); + gdb::unique_xmalloc_ptr holder + ((gdb_byte *) xmalloc (iter.end - iter.begin)); + iter.data = holder.get (); + mem_holders.push_back (std::move (holder)); + int err = target_read_memory (iter.begin, iter.data, + iter.end - iter.begin); if (err != 0) - goto out; + return err; - VEC_safe_push (memory_write_request_s, flash, r); + flash.push_back (iter); } - qsort (VEC_address (memory_write_request_s, flash), - VEC_length (memory_write_request_s, flash), - sizeof (struct memory_write_request), - compare_block_starting_address); + std::sort (flash.begin (), flash.end (), + compare_block_starting_address); } } @@ -398,47 +334,44 @@ target_write_memory_blocks (VEC(memory_write_request_s) *requests, have the opportunity to batch flash requests. */ /* Write regular blocks. */ - for (i = 0; VEC_iterate (memory_write_request_s, regular, i, r); ++i) + for (const memory_write_request &iter : regular) { LONGEST len; len = target_write_with_progress (current_target.beneath, TARGET_OBJECT_MEMORY, NULL, - r->data, r->begin, r->end - r->begin, - progress_cb, r->baton); - if (len < (LONGEST) (r->end - r->begin)) + iter.data, iter.begin, + iter.end - iter.begin, + progress_cb, iter.baton); + if (len < (LONGEST) (iter.end - iter.begin)) { /* Call error? */ - err = -1; - goto out; + return -1; } } - if (!VEC_empty (memory_write_request_s, erased)) + if (!erased.empty ()) { /* Erase all pages. */ - for (i = 0; VEC_iterate (memory_write_request_s, erased, i, r); ++i) - target_flash_erase (r->begin, r->end - r->begin); + for (const memory_write_request &iter : erased) + target_flash_erase (iter.begin, iter.end - iter.begin); /* Write flash data. */ - for (i = 0; VEC_iterate (memory_write_request_s, flash, i, r); ++i) + for (const memory_write_request &iter : flash) { LONGEST len; len = target_write_with_progress (¤t_target, TARGET_OBJECT_FLASH, NULL, - r->data, r->begin, - r->end - r->begin, - progress_cb, r->baton); - if (len < (LONGEST) (r->end - r->begin)) + iter.data, iter.begin, + iter.end - iter.begin, + progress_cb, iter.baton); + if (len < (LONGEST) (iter.end - iter.begin)) error (_("Error writing data to flash")); } target_flash_done (); } - out: - do_cleanups (back_to); - - return err; + return 0; } diff --git a/gdb/target.h b/gdb/target.h index 83cf485..e8972ca 100644 --- a/gdb/target.h +++ b/gdb/target.h @@ -1463,18 +1463,21 @@ void target_flash_done (void); /* Describes a request for a memory write operation. */ struct memory_write_request - { - /* Begining address that must be written. */ - ULONGEST begin; - /* Past-the-end address. */ - ULONGEST end; - /* The data to write. */ - gdb_byte *data; - /* A callback baton for progress reporting for this request. */ - void *baton; - }; -typedef struct memory_write_request memory_write_request_s; -DEF_VEC_O(memory_write_request_s); +{ + memory_write_request (ULONGEST begin_, ULONGEST end_, + gdb_byte *data_ = nullptr, void *baton_ = nullptr) + : begin (begin_), end (end_), data (data_), baton (baton_) + {} + + /* Begining address that must be written. */ + ULONGEST begin; + /* Past-the-end address. */ + ULONGEST end; + /* The data to write. */ + gdb_byte *data; + /* A callback baton for progress reporting for this request. */ + void *baton; +}; /* Enumeration specifying different flash preservation behaviour. */ enum flash_preserve_mode @@ -1500,9 +1503,10 @@ enum flash_preserve_mode with a NULL baton, when preserved flash sectors are being rewritten. The function returns 0 on success, and error otherwise. */ -int target_write_memory_blocks (VEC(memory_write_request_s) *requests, - enum flash_preserve_mode preserve_flash_p, - void (*progress_cb) (ULONGEST, void *)); +int target_write_memory_blocks + (const std::vector &requests, + enum flash_preserve_mode preserve_flash_p, + void (*progress_cb) (ULONGEST, void *)); /* Print a line about the current target. */ -- 2.7.4