public inbox for gdb-prs@sourceware.org
help / color / mirror / Atom feed
* [Bug gdb/27994] New: ASan crash when printing expression involving function call
@ 2021-06-18  4:17 simark at simark dot ca
  2021-06-19 12:34 ` [Bug gdb/27994] " ssbssa at sourceware dot org
                   ` (11 more replies)
  0 siblings, 12 replies; 13+ messages in thread
From: simark at simark dot ca @ 2021-06-18  4:17 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=27994

            Bug ID: 27994
           Summary: ASan crash when printing expression involving function
                    call
           Product: gdb
           Version: HEAD
            Status: NEW
          Severity: normal
          Priority: P2
         Component: gdb
          Assignee: unassigned at sourceware dot org
          Reporter: simark at simark dot ca
  Target Milestone: ---

I don't have a small reproducer yet, so here's a complex reproducer.  The
program to debug is a patched GDB, but the crashing GDB is from master.

Bisected to commit d182f2797922 ("Convert c-exp.y to use operations").

1) Build GDB at this commit: https://review.lttng.org/c/binutils-gdb/+/6066/12
2) Start debugging something with it
3) Build GDB at current master (7daf500de25c0e93bc70d593a7979657a2d4ceb5)
4) Debug GDB from step 1 using GDB from step 3:

$ ./gdb --data-directory=data-directory -p $(pidof gdb) -ex 'p
current_inferior_.m_obj.thread_list.m_head.next.next' -ex 'p
current_inferior_.m_obj.thread_list.as_value($1)'
<some text>
$1 = (struct intrusive_list_node<thread_info> *) 0x617000014298
=================================================================
==987643==ERROR: AddressSanitizer: heap-buffer-overflow on address
0x6020001df7a0 at pc 0x556f22e2cd84 bp 0x7ffd64b10d90 sp 0x7ffd64b10d80
READ of size 8 at 0x6020001df7a0 thread T0
    #0 0x556f22e2cd83 in typecmp /home/smarchi/src/wt/test/gdb/valops.c:1826
    #1 0x556f22e2f7d0 in search_struct_method
/home/smarchi/src/wt/test/gdb/valops.c:2220
    #2 0x556f22e30716 in value_struct_elt(value**, value**, char const*, int*,
char const*) /home/smarchi/src/wt/test/gdb/valops.c:2379
    #3 0x556f224c546b in expr::structop_base_operation::evaluate_funcall(type*,
expression*, noside, std::vector<std::unique_ptr<expr::operation,
std::default_delete<expr::operation> >,
std::allocator<std::unique_ptr<expr::operation,
std::default_delete<expr::operation> > > > const&)
/home/smarchi/src/wt/test/gdb/eval.c:944
    #4 0x556f22107e4c in expr::funcall_operation::evaluate(type*, expression*,
noside) /home/smarchi/src/wt/test/gdb/expop.h:2178
    #5 0x556f224c01a7 in expression::evaluate(type*, noside)
/home/smarchi/src/wt/test/gdb/eval.c:101
    #6 0x556f224c02f6 in evaluate_expression(expression*, type*)
/home/smarchi/src/wt/test/gdb/eval.c:115
    #7 0x556f228e59c2 in process_print_command_args
/home/smarchi/src/wt/test/gdb/printcmd.c:1305
    #8 0x556f228e5b9a in print_command_1
/home/smarchi/src/wt/test/gdb/printcmd.c:1318
    #9 0x556f228e65c7 in print_command
/home/smarchi/src/wt/test/gdb/printcmd.c:1435
    #10 0x556f2218bc27 in do_const_cfunc
/home/smarchi/src/wt/test/gdb/cli/cli-decode.c:102
    #11 0x556f22196f80 in cmd_func(cmd_list_element*, char const*, int)
/home/smarchi/src/wt/test/gdb/cli/cli-decode.c:2176
    #12 0x556f22cc43cf in execute_command(char const*, int)
/home/smarchi/src/wt/test/gdb/top.c:674
    #13 0x556f22783288 in catch_command_errors
/home/smarchi/src/wt/test/gdb/main.c:523
    #14 0x556f227838af in execute_cmdargs
/home/smarchi/src/wt/test/gdb/main.c:618
    #15 0x556f22786a2d in captured_main_1
/home/smarchi/src/wt/test/gdb/main.c:1322
    #16 0x556f22786fc1 in captured_main
/home/smarchi/src/wt/test/gdb/main.c:1343
    #17 0x556f22787062 in gdb_main(captured_main_args*)
/home/smarchi/src/wt/test/gdb/main.c:1368
    #18 0x556f21e7eb81 in main /home/smarchi/src/wt/test/gdb/gdb.c:32
    #19 0x7f51ce3ba0b2 in __libc_start_main
(/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
    #20 0x556f21e7e95d in _start (/home/smarchi/build/wt/test/gdb/gdb+0x42695d)

0x6020001df7a0 is located 0 bytes to the right of 16-byte region
[0x6020001df790,0x6020001df7a0)
allocated by thread T0 here:
    #0 0x7f51cf18b947 in operator new(unsigned long)
(/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10f947)
    #1 0x556f21f342dc in __gnu_cxx::new_allocator<value*>::allocate(unsigned
long, void const*) /usr/include/c++/9/ext/new_allocator.h:114
    #2 0x556f21f2ee1f in std::allocator_traits<std::allocator<value*>
>::allocate(std::allocator<value*>&, unsigned long)
/usr/include/c++/9/bits/alloc_traits.h:444
    #3 0x556f21f29c11 in std::_Vector_base<value*, std::allocator<value*>
>::_M_allocate(unsigned long) /usr/include/c++/9/bits/stl_vector.h:343
    #4 0x556f21f25736 in std::_Vector_base<value*, std::allocator<value*>
>::_M_create_storage(unsigned long) /usr/include/c++/9/bits/stl_vector.h:358
    #5 0x556f21f1fd9a in std::_Vector_base<value*, std::allocator<value*>
>::_Vector_base(unsigned long, std::allocator<value*> const&)
/usr/include/c++/9/bits/stl_vector.h:302
    #6 0x556f21f18f12 in std::vector<value*, std::allocator<value*>
>::vector(unsigned long, std::allocator<value*> const&)
/usr/include/c++/9/bits/stl_vector.h:508
    #7 0x556f224c4cac in expr::structop_base_operation::evaluate_funcall(type*,
expression*, noside, std::vector<std::unique_ptr<expr::operation,
std::default_delete<expr::operation> >,
std::allocator<std::unique_ptr<expr::operation,
std::default_delete<expr::operation> > > > const&)
/home/smarchi/src/wt/test/gdb/eval.c:875
    #8 0x556f22107e4c in expr::funcall_operation::evaluate(type*, expression*,
noside) /home/smarchi/src/wt/test/gdb/expop.h:2178
    #9 0x556f224c01a7 in expression::evaluate(type*, noside)
/home/smarchi/src/wt/test/gdb/eval.c:101
    #10 0x556f224c02f6 in evaluate_expression(expression*, type*)
/home/smarchi/src/wt/test/gdb/eval.c:115
    #11 0x556f228e59c2 in process_print_command_args
/home/smarchi/src/wt/test/gdb/printcmd.c:1305
    #12 0x556f228e5b9a in print_command_1
/home/smarchi/src/wt/test/gdb/printcmd.c:1318
    #13 0x556f228e65c7 in print_command
/home/smarchi/src/wt/test/gdb/printcmd.c:1435
    #14 0x556f2218bc27 in do_const_cfunc
/home/smarchi/src/wt/test/gdb/cli/cli-decode.c:102
    #15 0x556f22196f80 in cmd_func(cmd_list_element*, char const*, int)
/home/smarchi/src/wt/test/gdb/cli/cli-decode.c:2176
    #16 0x556f22cc43cf in execute_command(char const*, int)
/home/smarchi/src/wt/test/gdb/top.c:674
    #17 0x556f22783288 in catch_command_errors
/home/smarchi/src/wt/test/gdb/main.c:523
    #18 0x556f227838af in execute_cmdargs
/home/smarchi/src/wt/test/gdb/main.c:618
    #19 0x556f22786a2d in captured_main_1
/home/smarchi/src/wt/test/gdb/main.c:1322
    #20 0x556f22786fc1 in captured_main
/home/smarchi/src/wt/test/gdb/main.c:1343
    #21 0x556f22787062 in gdb_main(captured_main_args*)
/home/smarchi/src/wt/test/gdb/main.c:1368
    #22 0x556f21e7eb81 in main /home/smarchi/src/wt/test/gdb/gdb.c:32
    #23 0x7f51ce3ba0b2 in __libc_start_main
(/lib/x86_64-linux-gnu/libc.so.6+0x270b2)

SUMMARY: AddressSanitizer: heap-buffer-overflow
/home/smarchi/src/wt/test/gdb/valops.c:1826 in typecmp


I don't understand what happens yet, but here's what I found:

1) structop_base_operation::evaluate_funcall allocates vals, a vector of 2
elements
2) It calls value_struct_elt to find the "as_value" structure element passing
&vals[1] as args (args points to the vector's 2nd element)
3) It is then passed to search_struct_method and typecmd as argument `t2`.
4) typecmd accesses t2[1], which is out of bounds of the original vector (it
would be equivalent to doing vals[2]).

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug gdb/27994] ASan crash when printing expression involving function call
  2021-06-18  4:17 [Bug gdb/27994] New: ASan crash when printing expression involving function call simark at simark dot ca
@ 2021-06-19 12:34 ` ssbssa at sourceware dot org
  2021-06-21 16:41 ` simark at simark dot ca
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: ssbssa at sourceware dot org @ 2021-06-19 12:34 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=27994

Hannes Domani <ssbssa at sourceware dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ssbssa at sourceware dot org

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug gdb/27994] ASan crash when printing expression involving function call
  2021-06-18  4:17 [Bug gdb/27994] New: ASan crash when printing expression involving function call simark at simark dot ca
  2021-06-19 12:34 ` [Bug gdb/27994] " ssbssa at sourceware dot org
@ 2021-06-21 16:41 ` simark at simark dot ca
  2021-06-21 21:49 ` andrew.burgess at embecosm dot com
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: simark at simark dot ca @ 2021-06-21 16:41 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=27994

Simon Marchi <simark at simark dot ca> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |11.1

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug gdb/27994] ASan crash when printing expression involving function call
  2021-06-18  4:17 [Bug gdb/27994] New: ASan crash when printing expression involving function call simark at simark dot ca
  2021-06-19 12:34 ` [Bug gdb/27994] " ssbssa at sourceware dot org
  2021-06-21 16:41 ` simark at simark dot ca
@ 2021-06-21 21:49 ` andrew.burgess at embecosm dot com
  2021-06-21 22:40 ` andrew.burgess at embecosm dot com
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: andrew.burgess at embecosm dot com @ 2021-06-21 21:49 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=27994

Andrew Burgess <andrew.burgess at embecosm dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |andrew.burgess at embecosm dot com

--- Comment #1 from Andrew Burgess <andrew.burgess at embecosm dot com> ---
I suspect that the problem is how we populate the vals array in
structop_base_operation::evaluate_funcall.

If in structop_base_operation::evaluate_funcall, just before the call to
value_struct_elt we examine vals, it looks like this:

  { 0x6110011631c0, 0x611001163080 }

In contrast, if I roll back to commit c1299a23448, and find the equivalent code
in evaluate_funcall, and this time print argvec (the equivalent to vals), I now
see:

  { 0x4, 0x6110011d7200, 0x6110011d70c0, (nil) }

I believe the leading '0x4' is just uninitialised junk corresponding to where
the 'this' argument will be placed, so this looks like the first possible
problem, then, as Simon points out, the call stack is:

  value_struct_elt -> search_struct_method -> typecmp

and vals ends up as argument t2 in 'typecmp'.  The comment on 'typecmp' says:

 "... and T2 is a NULL-terminated vector."

So the missing nullptr at the end of vals also seems important, so this looks
like a possible second problem.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug gdb/27994] ASan crash when printing expression involving function call
  2021-06-18  4:17 [Bug gdb/27994] New: ASan crash when printing expression involving function call simark at simark dot ca
                   ` (2 preceding siblings ...)
  2021-06-21 21:49 ` andrew.burgess at embecosm dot com
@ 2021-06-21 22:40 ` andrew.burgess at embecosm dot com
  2021-06-21 23:00 ` andrew.burgess at embecosm dot com
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: andrew.burgess at embecosm dot com @ 2021-06-21 22:40 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=27994

--- Comment #2 from Andrew Burgess <andrew.burgess at embecosm dot com> ---
Created attachment 13509
  --> https://sourceware.org/bugzilla/attachment.cgi?id=13509&action=edit
Possible fix.

Here's a possible fix.  Following on from my previous comment, in the "old"
world, the leading '0x4' which I thought was where the `this' pointer was going
to be placed is actually where we used to place the value representing the
callee.  In the new world we store this outside the arguments (vals) array.

Thus, where we previous passed argvec[1] which became vals[1], this should have
been vals[0] - as the 0 index is now the `this' pointer, not the callee.

I have also added a trailing nullptr as required by typecmp.

This certainly stops the Asan failure that was originally reported.  I'm
currently running the full test set to see if there are any problems thrown up.

Any initial thoughts would be great.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug gdb/27994] ASan crash when printing expression involving function call
  2021-06-18  4:17 [Bug gdb/27994] New: ASan crash when printing expression involving function call simark at simark dot ca
                   ` (3 preceding siblings ...)
  2021-06-21 22:40 ` andrew.burgess at embecosm dot com
@ 2021-06-21 23:00 ` andrew.burgess at embecosm dot com
  2021-06-21 23:40 ` andrew.burgess at embecosm dot com
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: andrew.burgess at embecosm dot com @ 2021-06-21 23:00 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=27994

--- Comment #3 from Andrew Burgess <andrew.burgess at embecosm dot com> ---
Interesting little aside, in this bug we are running GDB_1 and attach to it
with GDB_2.  We then evaluate some expressions in GDB_2, and it is in GDB_2
that we see the bugs.

One o the expressions we evaluate is:

  p current_inferior_.m_obj.thread_list.as_value($1)

So, calling the 'as_value' method on 'thread_list', a member of 'm_obj', a
member of ... etc

What language is this expression evaluated as?

Did you guess 'C'?  I didn't.  I assumed 'C++'.

What happens is, when we attach to GDB_1 its call stack is:

  #0  0x00007ff8bcedca5f in poll () from /lib64/libc.so.6
  #1  0x000000000153029e in gdb_wait_for_event (block=1) at
../../src/gdbsupport/event-loop.cc:613
  #2  0x000000000152f69c in gdb_do_one_event () at
../../src/gdbsupport/event-loop.cc:237
  #3  0x00000000009aa737 in start_event_loop () at ../../src/gdb/main.c:421
  #4  0x00000000009aa857 in captured_command_loop () at
../../src/gdb/main.c:481
  #5  0x00000000009ac18c in captured_main (data=0x7ffe054f8b10) at
../../src/gdb/main.c:1353
  #6  0x00000000009ac1f2 in gdb_main (args=0x7ffe054f8b10) at
../../src/gdb/main.c:1368
  #7  0x000000000041754d in main (argc=4, argv=0x7ffe054f8c18) at
../../src/gdb/gdb.c:32

And 'libc.so.6' is obviously written in C, hence, when the user inputs the
expression at the command line we pick up the current language, which is C. 
Makes sense, but I guess I hadn't really thought about this case.

Would be weirder if the expression had depended upon overload resolution as
this would not have occurred.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug gdb/27994] ASan crash when printing expression involving function call
  2021-06-18  4:17 [Bug gdb/27994] New: ASan crash when printing expression involving function call simark at simark dot ca
                   ` (4 preceding siblings ...)
  2021-06-21 23:00 ` andrew.burgess at embecosm dot com
@ 2021-06-21 23:40 ` andrew.burgess at embecosm dot com
  2021-06-22  0:13 ` simark at simark dot ca
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: andrew.burgess at embecosm dot com @ 2021-06-21 23:40 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=27994

--- Comment #4 from Andrew Burgess <andrew.burgess at embecosm dot com> ---
Posted the patch to the m/l:

  https://sourceware.org/pipermail/gdb-patches/2021-June/180215.html

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug gdb/27994] ASan crash when printing expression involving function call
  2021-06-18  4:17 [Bug gdb/27994] New: ASan crash when printing expression involving function call simark at simark dot ca
                   ` (5 preceding siblings ...)
  2021-06-21 23:40 ` andrew.burgess at embecosm dot com
@ 2021-06-22  0:13 ` simark at simark dot ca
  2021-06-22  8:29 ` andrew.burgess at embecosm dot com
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: simark at simark dot ca @ 2021-06-22  0:13 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=27994

--- Comment #5 from Simon Marchi <simark at simark dot ca> ---
(In reply to Andrew Burgess from comment #2)
> Created attachment 13509 [details]
> Possible fix.
> 
> Here's a possible fix.  Following on from my previous comment, in the "old"
> world, the leading '0x4' which I thought was where the `this' pointer was
> going to be placed is actually where we used to place the value representing
> the callee.  In the new world we store this outside the arguments (vals)
> array.
> 
> Thus, where we previous passed argvec[1] which became vals[1], this should
> have been vals[0] - as the 0 index is now the `this' pointer, not the callee.
> 
> I have also added a trailing nullptr as required by typecmp.
> 
> This certainly stops the Asan failure that was originally reported.  I'm
> currently running the full test set to see if there are any problems thrown
> up.
> 
> Any initial thoughts would be great.

What you said makes sense to me.  I tried to debug it when I reported it, but I
didn't have much time and it was all a bit blurry.  But what you said matches
what I remember.  I didn't have time to compare the behavior after with the
behavior before, thanks for doing this.

I'd be tempted to make what we pass down a gdb::array_view, instead of a
NULL-terminated array, but I haven't actually tried it.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug gdb/27994] ASan crash when printing expression involving function call
  2021-06-18  4:17 [Bug gdb/27994] New: ASan crash when printing expression involving function call simark at simark dot ca
                   ` (6 preceding siblings ...)
  2021-06-22  0:13 ` simark at simark dot ca
@ 2021-06-22  8:29 ` andrew.burgess at embecosm dot com
  2021-06-22 13:48 ` simark at simark dot ca
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: andrew.burgess at embecosm dot com @ 2021-06-22  8:29 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=27994

--- Comment #6 from Andrew Burgess <andrew.burgess at embecosm dot com> ---
I agree that passing an array_view around would be better than a NULL
terminated array.

But, given multiple functions would need to change, and fixing the bug using
the existing API is pretty simple, I'd prefer to fix things using the existing
API first, then we can update the API later as a refactoring task.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug gdb/27994] ASan crash when printing expression involving function call
  2021-06-18  4:17 [Bug gdb/27994] New: ASan crash when printing expression involving function call simark at simark dot ca
                   ` (7 preceding siblings ...)
  2021-06-22  8:29 ` andrew.burgess at embecosm dot com
@ 2021-06-22 13:48 ` simark at simark dot ca
  2021-06-25 19:51 ` cvs-commit at gcc dot gnu.org
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: simark at simark dot ca @ 2021-06-22 13:48 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=27994

--- Comment #7 from Simon Marchi <simark at simark dot ca> ---
(In reply to Andrew Burgess from comment #6)
> I agree that passing an array_view around would be better than a NULL
> terminated array.
> 
> But, given multiple functions would need to change, and fixing the bug using
> the existing API is pretty simple, I'd prefer to fix things using the
> existing API first, then we can update the API later as a refactoring task.

Agreed.  I gave this a try yesterday and stumbled on

https://sourceware.org/bugzilla/show_bug.cgi?id=28004

which completely distracted me from this :)

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug gdb/27994] ASan crash when printing expression involving function call
  2021-06-18  4:17 [Bug gdb/27994] New: ASan crash when printing expression involving function call simark at simark dot ca
                   ` (8 preceding siblings ...)
  2021-06-22 13:48 ` simark at simark dot ca
@ 2021-06-25 19:51 ` cvs-commit at gcc dot gnu.org
  2021-06-25 19:51 ` cvs-commit at gcc dot gnu.org
  2021-06-25 19:52 ` andrew.burgess at embecosm dot com
  11 siblings, 0 replies; 13+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-06-25 19:51 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=27994

--- Comment #8 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Andrew Burgess <aburgess@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=79bd4d34f0583f1c1cea60fa94986e222ade33b8

commit 79bd4d34f0583f1c1cea60fa94986e222ade33b8
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date:   Mon Jun 21 23:33:11 2021 +0100

    gdb: fix regression in evaluate_funcall for non C++ like cases

    This regression, as it is exposed by the test added in this commit,
    first became noticable with this commit:

      commit d182f2797922a305fbd1ef6a483cc39a56b43e02
      Date:   Mon Mar 8 07:27:57 2021 -0700

          Convert c-exp.y to use operations

    But, this commit only added converted the C expression parser to make
    use of code that was added in this commit:

      commit a00b7254fb614af557de7ae7cc0eb39a0ce0e408
      Date:   Mon Mar 8 07:27:57 2021 -0700

          Implement function call operations

    And it was this second commit that actually introduced the bugs (there
    are two).

    In structop_base_operation::evaluate_funcall we build up an argument
    list in the vector vals.  Later in this function the argument list
    might be passed to value_struct_elt.

    Prior to commit a00b7254fb614 the vals vector (or argvec as it used to
    be called) stored the value for the function callee in the argvec at
    index 0.  This 'callee' value is what ends up being passed to
    evaluate_subexp_do_call, and represents the function to be called, the
    value contents are the address of the function, and the value type is
    the function signature.  The remaining items held in the argvec were
    the values to pass to the function.  For a non-static member function
    the `this' pointer would be at index 1 in the array.

    After commit a00b7254fb614 this callee value is now held in a separate
    variable, not the vals array.  So, for non-static member functions,
    the `this' pointer is now at index 0, with any other arguments after
    that.

    What this means is that previous, when we called value_struct_elt we
    would pass the address of argvec[1] as this was the first argument.
    But now we should be passing the address of vals[0].  Unfortunately,
    we are still passing vals[1], effectively skipping the first
    argument.

    The second issue is that, prior to commit a00b7254fb614, the argvec
    array was NULL terminated.  This is required as value_struct_elt
    calls search_struct_method, which calls typecmp, and typecmp requires
    that the array have a NULL at the end.

    After commit a00b7254fb614 this NULL has been lost, and we are
    therefore violating the API requirements of typecmp.

    This commit fixes both of these regressions.  I also extended the
    header comments on search_struct_method and value_struct_elt to make
    it clearer that the array required a NULL marker at the end.

    You will notice in the test attached to this commit that I test
    calling a non-static member function, but not calling a static member
    function.  The reason for this is that calling static member functions
    is currently broken due to a different bug.  That will be fixed in a
    later patch in this series, at which time I'll add a test for calling
    a static member function.

    gdb/ChangeLog:

            PR gdb/27994
            * eval.c (structop_base_operation::evaluate_funcall): Add a
            nullptr to the end of the args array, which should not be included
            in the argument array_view.  Pass all the arguments through to
            value_struct_elt.
            * valops.c (search_struct_method): Update header comment.
            (value_struct_elt): Likewise.

    gdb/testsuite/ChangeLog:

            PR gdb/27994
            * gdb.cp/method-call-in-c.cc: New file.
            * gdb.cp/method-call-in-c.exp: New file.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug gdb/27994] ASan crash when printing expression involving function call
  2021-06-18  4:17 [Bug gdb/27994] New: ASan crash when printing expression involving function call simark at simark dot ca
                   ` (9 preceding siblings ...)
  2021-06-25 19:51 ` cvs-commit at gcc dot gnu.org
@ 2021-06-25 19:51 ` cvs-commit at gcc dot gnu.org
  2021-06-25 19:52 ` andrew.burgess at embecosm dot com
  11 siblings, 0 replies; 13+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-06-25 19:51 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=27994

--- Comment #9 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Andrew Burgess <aburgess@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=13221aec0d87c701c463d4fa54aa70096d0f43a7

commit 13221aec0d87c701c463d4fa54aa70096d0f43a7
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date:   Tue Jun 22 10:17:53 2021 +0100

    gdb: replace NULL terminated array with array_view

    After the previous commit, this commit updates the value_struct_elt
    function to take an array_view rather than a NULL terminated array of
    values.

    The requirement for a NULL terminated array of values actually stems
    from typecmp, so the change from an array to array_view needs to be
    propagated through to this function.

    While making this change I noticed that this fixes another bug, in
    value_x_binop and value_x_unop GDB creates an array of values which
    doesn't have a NULL at the end.  An array_view of this array is passed
    to value_user_defined_op, which then unpacks the array_view and passed
    the raw array to value_struct_elt, but only if the language is not
    C++.

    As value_x_binop and value_x_unop can only request member functions
    with the names of C++ operators, then most of the time, assuming the
    inferior is not a C++ program, then GDB will not find a matching
    member function with the call to value_struct_elt, and so typecmp will
    never be called, and so, GDB will avoid undefined behaviour.

    However, it is worth remembering that, when GDB's language is set to
    "auto", the current language is selected based on the language of the
    current compilation unit.  As C++ programs usually link against libc,
    which is written in C, then, if the inferior is stopped in libc GDB
    will set the language to C.  And so, it is possible that we will end
    up using value_struct_elt to try and lookup, and match, a C++
    operator.  If this occurs then GDB will experience undefined
    behaviour.

    I have extended the test added in the previous commit to also cover
    this case.

    Finally, this commit changes the API from passing around a pointer to
    an array to passing around a pointer to an array_view.  The reason for
    this is that we need to be able to distinguish between the cases where
    we call value_struct_elt with no arguments, i.e. we are looking up a
    struct member, but we either don't have the arguments we want to pass
    yet, or we don't expect there to be any need for GDB to use the
    argument types to resolve any overloading; and the second case where
    we call value_struct_elt looking for a function that takes no
    arguments, that is, the argument list is empty.

    NOTE: While writing this I realise that if we pass an array_view at
    all then it will always have at least one item in it, the `this'
    pointer for the object we are planning to call the method on.  So we
    could, I guess, pass an empty array_view to indicate the case where we
    don't know anything about the arguments, and when the array_view is
    length 1 or more, it means we do have the arguments.  However, though
    we could do this, I don't think this would be better, the length 0 vs
    length 1 difference seems a little too subtle, I think that there's a
    better solution...

    I think a better solution would be to wrap the array_view in a
    gdb::optional, this would make the whole, do we have an array view or
    not question explicit.

    I haven't done this as part of this commit as making that change is
    much more extensive, every user of value_struct_elt will need to be
    updated, and as this commit already contains a bug fix, I wanted to
    keep the large refactoring in a separate commit, so, check out the
    next commit for the use of gdb::optional.

    gdb/ChangeLog:

            PR gdb/27994
            * eval.c (structop_base_operation::evaluate_funcall): Pass
            array_view instead of array to value_struct_elt.
            * valarith.c (value_user_defined_op): Likewise.
            * valops.c (typecmp): Change parameter type from array pointer to
            array_view.  Update header comment, and update body accordingly.
            (search_struct_method): Likewise.
            (value_struct_elt): Likewise.
            * value.h (value_struct_elt): Update declaration.

    gdb/testsuite/ChangeLog:

            PR gdb/27994
            * gdb.cp/method-call-in-c.cc (struct foo_type): Add operator+=,
            change initial value of var member variable.
            (main): Make use of foo_type's operator+=.
            * gdb.cp/method-call-in-c.exp: Test use of operator+=.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug gdb/27994] ASan crash when printing expression involving function call
  2021-06-18  4:17 [Bug gdb/27994] New: ASan crash when printing expression involving function call simark at simark dot ca
                   ` (10 preceding siblings ...)
  2021-06-25 19:51 ` cvs-commit at gcc dot gnu.org
@ 2021-06-25 19:52 ` andrew.burgess at embecosm dot com
  11 siblings, 0 replies; 13+ messages in thread
From: andrew.burgess at embecosm dot com @ 2021-06-25 19:52 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=27994

Andrew Burgess <andrew.burgess at embecosm dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #10 from Andrew Burgess <andrew.burgess at embecosm dot com> ---
Should be all fixed now.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-06-25 19:52 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-18  4:17 [Bug gdb/27994] New: ASan crash when printing expression involving function call simark at simark dot ca
2021-06-19 12:34 ` [Bug gdb/27994] " ssbssa at sourceware dot org
2021-06-21 16:41 ` simark at simark dot ca
2021-06-21 21:49 ` andrew.burgess at embecosm dot com
2021-06-21 22:40 ` andrew.burgess at embecosm dot com
2021-06-21 23:00 ` andrew.burgess at embecosm dot com
2021-06-21 23:40 ` andrew.burgess at embecosm dot com
2021-06-22  0:13 ` simark at simark dot ca
2021-06-22  8:29 ` andrew.burgess at embecosm dot com
2021-06-22 13:48 ` simark at simark dot ca
2021-06-25 19:51 ` cvs-commit at gcc dot gnu.org
2021-06-25 19:51 ` cvs-commit at gcc dot gnu.org
2021-06-25 19:52 ` andrew.burgess at embecosm dot com

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).