diff mbox

Fix use-after-move in compile/compile-cplus-types.c

Message ID 20180917135203.10368-1-simon.marchi@ericsson.com
State New, archived
Headers

Commit Message

Simon Marchi Sept. 17, 2018, 1:52 p.m. UTC 
  Patch

  d82b3862f12 ("compile: Remove non-const reference parameters")

introduced a regression in compile/compile-cplus-types.c.  The new_scope
variable in compile_cplus_instance::enter_scope is used after it was
std::moved.  This patch fixes it by referring to the back of the vector
where it was moved instead.

gdb/ChangeLog:

	* compile/compile-cplus-types.c
	(compile_cplus_instance::enter_scope): Don't use new_scope after
	std::move.
---
 gdb/compile/compile-cplus-types.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Keith Seitz Sept. 17, 2018, 3:25 p.m. UTC | #1 
On 09/17/2018 06:52 AM, Simon Marchi wrote:
> Patch
> 
>   d82b3862f12 ("compile: Remove non-const reference parameters")
> 
> introduced a regression in compile/compile-cplus-types.c.  The new_scope
> variable in compile_cplus_instance::enter_scope is used after it was
> std::moved.  This patch fixes it by referring to the back of the vector
> where it was moved instead.
> 
> gdb/ChangeLog:
> 
> 	* compile/compile-cplus-types.c
> 	(compile_cplus_instance::enter_scope): Don't use new_scope after
> 	std::move.

That LGTM. [Although I would have used a const reference to it everywhere, but
six of one, ...]

Keith

PS. Reminder: IANAM, but you are. So please approve your patch. :-)
Simon Marchi Sept. 17, 2018, 5:10 p.m. UTC | #2 
On 2018-09-17 11:25, Keith Seitz wrote:
> On 09/17/2018 06:52 AM, Simon Marchi wrote:
>> Patch
>> 
>>   d82b3862f12 ("compile: Remove non-const reference parameters")
>> 
>> introduced a regression in compile/compile-cplus-types.c.  The 
>> new_scope
>> variable in compile_cplus_instance::enter_scope is used after it was
>> std::moved.  This patch fixes it by referring to the back of the 
>> vector
>> where it was moved instead.
>> 
>> gdb/ChangeLog:
>> 
>> 	* compile/compile-cplus-types.c
>> 	(compile_cplus_instance::enter_scope): Don't use new_scope after
>> 	std::move.
> 
> That LGTM. [Although I would have used a const reference to it 
> everywhere, but
> six of one, ...]

Yeah, I thought about that too, but then I didn't know what to name the 
new variable, since "new_scope" is already taken.  So in the end I chose 
the solution where I didn't have to choose a name :).

> PS. Reminder: IANAM, but you are. So please approve your patch. :-)

Thanks for taking a look, I am pushing it.

Simon
diff mbox

Patch

diff --git a/gdb/compile/compile-cplus-types.c b/gdb/compile/compile-cplus-types.c
index 75193d2e75b..996fea56986 100644
--- a/gdb/compile/compile-cplus-types.c
+++ b/gdb/compile/compile-cplus-types.c
@@ -261,7 +261,7 @@  compile_cplus_instance::enter_scope (compile_scope &&new_scope)
       if (debug_compile_cplus_scopes)
 	{
 	  fprintf_unfiltered (gdb_stdlog, "entering new scope %s\n",
-			      host_address_to_string (&new_scope));
+			      host_address_to_string (&m_scopes.back ()));
 	}
 
       /* Push the global namespace. */
@@ -270,7 +270,7 @@  compile_cplus_instance::enter_scope (compile_scope &&new_scope)
       /* Push all other namespaces.  Note that we do not push the last
 	 scope_component -- that's the actual type we are converting.  */
       std::for_each
-	(new_scope.begin (), new_scope.end () - 1,
+	(m_scopes.back ().begin (), m_scopes.back ().end () - 1,
 	 [this] (const scope_component &comp)
 	 {
 	  gdb_assert (TYPE_CODE (SYMBOL_TYPE (comp.bsymbol.symbol))