[Bug gdb/23838] New: 8.2 regression for invalid -data-directory [Re: [RESEND][PATCH 01/40] Convert struct target_ops to C++]

[jan.kratochvil at redhat dot com]

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

Bug ID: 23838
Summary: 8.2 regression for invalid -data-directory [Re:
[RESEND][PATCH 01/40] Convert struct target_ops to
C++]
Product: gdb
Version: 8.2
Status: NEW
Severity: normal
Priority: P2
Component: gdb
Assignee: unassigned at sourceware dot org
Reporter: jan.kratochvil at redhat dot com
Target Milestone: ---

https://sourceware.org/ml/gdb-patches/2018-10/msg00659.html

f6ac5f3d63e03a81c4ff3749aba234961cc9090e is the first bad commit
commit f6ac5f3d63e03a81c4ff3749aba234961cc9090e
Author: Pedro Alves <***@redhat.com>
Date: Thu May 3 00:37:22 2018 +0100
Convert struct target_ops to C++

$ ./gdb -data-directory gdb
Segmentation fault

Program received signal SIGSEGV, Segmentation fault.
target_supports_terminal_ours () at target.c:590
590 return current_top_target ()->supports_terminal_ours ();
(gdb) bt
#0 target_supports_terminal_ours () at target.c:590
#1 0x0000000000c0c0d0 in vwarning (string=0x14bc4ca "%s is not a directory.",
args=0x7fffffffc8e0) at utils.c:173
#2 0x00000000005af49d in warning (fmt=0x14bc4ca "%s is not a directory.") at
common/errors.c:31
#3 0x0000000000873848 in set_gdb_data_directory (new_datadir=0x7fffffffd1f4
"gdb") at main.c:121
#4 0x000000000087465a in captured_main_1 (context=0x7fffffffcce0) at
main.c:763
#5 0x00000000008753e1 in captured_main (data=0x7fffffffcce0) at main.c:1163
#6 0x00000000008754bd in gdb_main (args=0x7fffffffcce0) at main.c:1189
#7 0x00000000004129dc in main (argc=3, argv=0x7fffffffcde8) at gdb.c:32
(gdb) _

[jan.kratochvil at redhat dot com]

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

Jan Kratochvil <jan.kratochvil at redhat dot com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |jan.kratochvil at redhat dot com
Target Milestone|--- |8.2.1

[simon.marchi at ericsson dot com]

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

Simon Marchi <simon.marchi at ericsson dot com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |simon.marchi at ericsson dot com

--- Comment #1 from Simon Marchi <simon.marchi at ericsson dot com> ---
It happens when what you pass to --data-directory is a regular file.

$ touch a-regular-file
$ ./gdb --data-directory a-regular-file
AddressSanitizer:DEADLYSIGNAL
=================================================================
==23181==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc
0x564528c37e20 bp 0x7ffe4ce4beb0 sp 0x7ffe4ce4beb0 T0)
==23181==The signal is caused by a READ memory access.
==23181==Hint: address points to the zero page.
#0 0x564528c37e1f in target_supports_terminal_ours()
/home/simark/src/binutils-gdb/gdb/target.c:590
#1 0x564528de01fb in vwarning(char const*, __va_list_tag*)
/home/simark/src/binutils-gdb/gdb/utils.c:173
#2 0x564527df0985 in warning(char const*, ...)
/home/simark/src/binutils-gdb/gdb/common/errors.c:31
#3 0x56452855d9a8 in set_gdb_data_directory(char const*)
/home/simark/src/binutils-gdb/gdb/main.c:121
#4 0x56452855fcb9 in captured_main_1
/home/simark/src/binutils-gdb/gdb/main.c:763
#5 0x56452856188e in captured_main
/home/simark/src/binutils-gdb/gdb/main.c:1163
#6 0x564528561a79 in gdb_main(captured_main_args*)
/home/simark/src/binutils-gdb/gdb/main.c:1189
#7 0x5645279f4569 in main /home/simark/src/binutils-gdb/gdb/gdb.c:32
#8 0x7fbbe8cc7222 in __libc_start_main (/usr/lib/libc.so.6+0x24222)
#9 0x5645279f434d in _start
(/home/simark/build/binutils-gdb/gdb/gdb+0x18c734d)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /home/simark/src/binutils-gdb/gdb/target.c:590
in target_supports_terminal_ours()
==23181==ABORTING

[cvs-commit at gcc dot gnu.org]

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

--- Comment #2 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tom Tromey <***@sourceware.org>:

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

commit 20f0d60db4fb5083779c4c9182bbc692f7d2bac5
Author: Tom Tromey <***@tromey.com>
Date: Fri Oct 5 14:54:35 2018 -0600

Avoid crash when calling warning too early

I noticed that if you pass the name of an existing file (not a
directory) as the argument to --data-directory, gdb will crash:

$ ./gdb -nx --data-directory ./gdb
../../binutils-gdb/gdb/target.c:590:56: runtime error: member call on
null pointer of type 'struct target_ops'

This was later reported as PR gdb/23838.

This happens because warning ends up calling
target_supports_terminal_ours, which calls current_top_target, which
returns nullptr this early.

This fixes the problem by handling this case specially in
target_supports_terminal_ours. I also changed
target_supports_terminal_ours to return bool.

gdb/ChangeLog
2018-11-08 Tom Tromey <***@tromey.com>

PR gdb/23555:
PR gdb/23838:
* target.h (target_supports_terminal_ours): Return bool.
* target.c (target_supports_terminal_ours): Handle case where
current_top_target returns nullptr. Return bool.

gdb/testsuite/ChangeLog
2018-11-08 Tom Tromey <***@tromey.com>

PR gdb/23555:
PR gdb/23838:
* gdb.base/warning.exp: New file.

[tromey at sourceware dot org]

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

Tom Tromey <tromey at sourceware dot org> changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
CC| |tromey at sourceware dot org
Resolution|--- |FIXED
Target Milestone|8.2.1 |8.3

--- Comment #3 from Tom Tromey <tromey at sourceware dot org> ---
Fixed.

[brobecker at gnat dot com]

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

Joel Brobecker <brobecker at gnat dot com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |brobecker at gnat dot com

--- Comment #4 from Joel Brobecker <brobecker at gnat dot com> ---
Hey Tom,

The patch looks safe to me, so I was wondering about backporting it 8.2.1. If
you agree with the assessment, I can do the backport.

Thanks!

[tromey at sourceware dot org]

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

--- Comment #5 from Tom Tromey <tromey at sourceware dot org> ---
(In reply to Joel Brobecker from comment #4)

Post by brobecker at gnat dot com
The patch looks safe to me, so I was wondering about backporting it 8.2.1.
If you agree with the assessment, I can do the backport.

Pedro asked for some changes to the test case.
I can do the backport but I want to get to those first.

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The gdb-8.2-branch branch has been updated by Tom Tromey
<***@sourceware.org>:

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

commit 9ec912f34410c33de581b5af4a0773c12b4fe20a
Author: Tom Tromey <***@tromey.com>
Date: Fri Oct 5 14:54:35 2018 -0600

Avoid crash when calling warning too early

I noticed that if you pass the name of an existing file (not a
directory) as the argument to --data-directory, gdb will crash:

$ ./gdb -nx --data-directory ./gdb
../../binutils-gdb/gdb/target.c:590:56: runtime error: member call on
null pointer of type 'struct target_ops'

This was later reported as PR gdb/23838.

This happens because warning ends up calling
target_supports_terminal_ours, which calls current_top_target, which
returns nullptr this early.

This fixes the problem by handling this case specially in
target_supports_terminal_ours. I also changed
target_supports_terminal_ours to return bool.

2018-11-08 Tom Tromey <***@tromey.com>

PR gdb/23555:
PR gdb/23838:
* target.h (target_supports_terminal_ours): Return bool.
* target.c (target_supports_terminal_ours): Handle case where
current_top_target returns nullptr. Return bool.

gdb/testsuite/ChangeLog
2018-11-08 Tom Tromey <***@tromey.com>

PR gdb/23555:
PR gdb/23838:
* gdb.base/warning.exp: New file.

[tromey at sourceware dot org]

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

Tom Tromey <tromey at sourceware dot org> changed:

What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|8.3 |8.2.1

--- Comment #7 from Tom Tromey <tromey at sourceware dot org> ---
The fix was backported for 8.2.1.