Bug 23838 - 8.2 regression for invalid -data-directory [Re: [RESEND][PATCH 01/40] Convert struct target_ops to C++]
Summary: 8.2 regression for invalid -data-directory [Re: [RESEND][PATCH 01/40] Convert...
Status: RESOLVED FIXED
Alias: None
Product: gdb
Classification: Unclassified
Component: gdb (show other bugs)
Version: 8.2
: P2 normal
Target Milestone: 8.2.1
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-10-27 21:59 UTC by Jan Kratochvil
Modified: 2018-11-19 18:35 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Kratochvil 2018-10-27 21:59:40 UTC
https://sourceware.org/ml/gdb-patches/2018-10/msg00659.html

f6ac5f3d63e03a81c4ff3749aba234961cc9090e is the first bad commit
commit f6ac5f3d63e03a81c4ff3749aba234961cc9090e
Author: Pedro Alves <palves@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) _
Comment 1 Simon Marchi 2018-10-28 14:54:05 UTC
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
Comment 2 cvs-commit@gcc.gnu.org 2018-11-08 23:16:54 UTC
The master branch has been updated by Tom Tromey <tromey@sourceware.org>:

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

commit 20f0d60db4fb5083779c4c9182bbc692f7d2bac5
Author: Tom Tromey <tom@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  <tom@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  <tom@tromey.com>
    
    	PR gdb/23555:
    	PR gdb/23838:
    	* gdb.base/warning.exp: New file.
Comment 3 Tom Tromey 2018-11-12 18:16:22 UTC
Fixed.
Comment 4 Joel Brobecker 2018-11-14 18:25:30 UTC
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!
Comment 5 Tom Tromey 2018-11-15 16:05:22 UTC
(In reply to Joel Brobecker from comment #4)

> 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.
Comment 6 cvs-commit@gcc.gnu.org 2018-11-19 18:34:13 UTC
The gdb-8.2-branch branch has been updated by Tom Tromey <tromey@sourceware.org>:

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

commit 9ec912f34410c33de581b5af4a0773c12b4fe20a
Author: Tom Tromey <tom@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  <tom@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  <tom@tromey.com>
    
    	PR gdb/23555:
    	PR gdb/23838:
    	* gdb.base/warning.exp: New file.
Comment 7 Tom Tromey 2018-11-19 18:35:08 UTC
The fix was backported for 8.2.1.