Files
binutils-gdb/gdb/testsuite/gdb.base/internal-string-values.exp
Andrew Burgess baab375361 gdb: building inferior strings from within GDB
History Of This Patch
=====================

This commit aims to address PR gdb/21699.  There have now been a
couple of attempts to fix this issue.  Simon originally posted two
patches back in 2021:

  https://sourceware.org/pipermail/gdb-patches/2021-July/180894.html
  https://sourceware.org/pipermail/gdb-patches/2021-July/180896.html

Before Pedro then posted a version of his own:

  https://sourceware.org/pipermail/gdb-patches/2021-July/180970.html

After this the conversation halted.  Then in 2023 I (Andrew) also took
a look at this bug and posted two versions:

  https://sourceware.org/pipermail/gdb-patches/2023-April/198570.html
  https://sourceware.org/pipermail/gdb-patches/2023-April/198680.html

The approach taken in my first patch was pretty similar to what Simon
originally posted back in 2021.  My second attempt was only a slight
variation on the first.

Pedro then pointed out his older patch, and so we arrive at this
patch.  The GDB changes here are mostly Pedro's work, but updated by
me (Andrew), any mistakes are mine.

The tests here are a combinations of everyone's work, and the commit
message is new, but copies bits from everyone's earlier work.

Problem Description
===================

Bug PR gdb/21699 makes the observation that using $_as_string with
GDB's printf can cause GDB to print unexpected data from the
inferior.  The reproducer is pretty simple:

  #include <stddef.h>
  static char arena[100];

  /* Override malloc() so value_coerce_to_target() gets a known
     pointer, and we know we"ll see an error if $_as_string() gives
     a string that isn't null terminated. */
  void
  *malloc (size_t size)
  {
      memset (arena, 'x', sizeof (arena));
      if (size > sizeof (arena))
          return NULL;
      return arena;
  }

  int
  main ()
  {
    return 0;
  }

And then in a GDB session:

  $ gdb -q test
  Reading symbols from /tmp/test...
  (gdb) start
  Temporary breakpoint 1 at 0x4004c8: file test.c, line 17.
  Starting program: /tmp/test

  Temporary breakpoint 1, main () at test.c:17
  17        return 0;
  (gdb) printf "%s\n", $_as_string("hello")
  "hello"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
  (gdb) quit

The problem above is caused by how value_cstring is used within
py-value.c, but once we understand the issue then it turns out that
value_cstring is used in an unexpected way in many places within GDB.

Within py-value.c we have a null-terminated C-style string.  We then
pass a pointer to this string, along with the length of this
string (so not including the null-character) to value_cstring.

In value_cstring GDB allocates an array value of the given character
type, and copies in requested number of characters.  However
value_cstring does not add a null-character of its own.  This means
that the value created by calling value_cstring is only
null-terminated if the null-character is included in the passed in
length.  In py-value.c this is not the case, and indeed, in most uses
of value_cstring, this is not the case.

When GDB tries to print one of these strings the value contents are
pushed to the inferior, and then read back as a C-style string, that
is, GDB reads inferior memory until it finds a null-terminator.  For
the py-value.c case, no null-terminator is pushed into the inferior,
so GDB will continue reading inferior memory until a null-terminator
is found, with unpredictable results.

Patch Description
=================

The first thing this patch does is better define what the arguments
for the two function value_cstring and value_string should represent.
The comments in the header file are updated to describe whether the
length argument should, or should not, include a null-character.
Also, the data argument is changed to type gdb_byte.  The functions as
they currently exist will handle wide-characters, in which case more
than one 'char' would be needed for each character.  As such using
gdb_byte seems to make more sense.

To avoid adding casts throughout GDB, I've also added an overload that
still takes a 'char *', but asserts that the character type being used
is of size '1'.

The value_cstring function is now responsible for adding a null
character at the end of the string value it creates.

However, once we start looking at how value_cstring is used, we
realise there's another, related, problem.  Not every language's
strings are null terminated.  Fortran and Ada strings, for example,
are just an array of characters, GDB already has the function
value_string which can be used to create such values.

Consider this example using current GDB:

  (gdb) set language ada
  (gdb) p $_gdb_setting("arch")
  $1 = (97, 117, 116, 111)
  (gdb) ptype $
  type = array (1 .. 4) of char
  (gdb) p $_gdb_maint_setting("test-settings string")
  $2 = (0)
  (gdb) ptype $
  type = array (1 .. 1) of char

This shows two problems, first, the $_gdb_setting and
$_gdb_maint_setting functions are calling value_cstring using the
builtin_char character, rather than a language appropriate type.  In
the first call, the 'arch' case, the value_cstring call doesn't
include the null character, so the returned array only contains the
expected characters.  But, in the $_gdb_maint_setting example we do
end up including the null-character, even though this is not expected
for Ada strings.

This commit adds a new language method language_defn::value_string,
this function takes a pointer and length and creates a language
appropriate value that represents the string.  For C, C++, etc this
will be a null-terminated string (by calling value_cstring), and for
Fortran and Ada this can be a bounded array of characters with no null
terminator.  Additionally, this new language_defn::value_string
function is responsible for selecting a language appropriate character
type.

After this commit the only calls to value_cstring are from the C
expression evaluator and from the default language_defn::value_string.

And the only calls to value_string are from Fortan, Ada, and ObjectC
related code.

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

Co-Authored-By: Simon Marchi <simon.marchi@efficios.com>
Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
Co-Authored-By: Pedro Alves <pedro@palves.net>
Approved-By: Simon Marchi <simon.marchi@efficios.com>
2023-06-05 13:25:08 +01:00

280 lines
8.1 KiB
Plaintext

# Copyright 2021-2023 Free Software Foundation, Inc.
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
# Test that string values are correctly allocated inside GDB when doing
# various operations that yield strings.
#
# The issue that lead to this test was a missing NULL terminator in the
# C-string values. We verify that we can print the null terminator of these
# strings.
load_lib "trace-support.exp"
load_lib "gdb-guile.exp"
standard_testfile
if {[build_executable "failed to prepare" $testfile $srcfile ]} {
return
}
set user_conv_funcs {$_gdb_setting $_gdb_setting_str}
set maint_conv_funcs {$_gdb_maint_setting $_gdb_maint_setting_str}
# Add language (LANG) appropriate quotation marks around string STR.
proc quote_for_lang {lang str} {
if {$lang == "fortran"} {
return "'$str'"
} else {
return "\"$str\""
}
}
# Check that the string contained in the convenienced variable $v is
# EXPECTED_STR.
#
# In particular, check that the null terminator is there and that we can't
# access a character past the end of the string.
proc check_v_string { expected_str } {
set len [string length $expected_str]
for { set i 0 } { $i < $len } { incr i } {
set c [string index $expected_str $i]
gdb_test "print \$v\[$i\]" "= $::decimal '$c'"
}
# Check that the string ends with a null terminator.
gdb_test "print \$v\[$i\]" {= 0 '\\000'}
# Check that we can't access a character after the end of the string.
incr i
gdb_test "print \$v\[$i\]" "no such vector element"
}
# Test with string values made by $_gdb_setting & co.
proc_with_prefix test_setting { } {
clean_restart
# This is an internal GDB implementation detail, but the variable backing
# a string setting starts as nullptr (unless explicitly initialized at
# startup). When assigning an empty value, the variable then points to an
# empty string. Test both cases, as it triggers different code paths (in
# addition to a non-empty value).
#
# Use "set trace-user" and "maintenance set test-settings string" as they
# are both not initialized at startup.
with_test_prefix "user setting" {
with_test_prefix "not set" {
foreach_with_prefix conv_func $::user_conv_funcs {
gdb_test_no_output "set \$v = ${conv_func}(\"trace-user\")"
check_v_string ""
}
}
with_test_prefix "set to empty" {
gdb_test "set trace-user"
foreach_with_prefix conv_func $::user_conv_funcs {
gdb_test_no_output "set \$v = ${conv_func}(\"trace-user\")"
check_v_string ""
}
}
with_test_prefix "set" {
gdb_test "set trace-user poulet"
foreach_with_prefix conv_func $::user_conv_funcs {
gdb_test_no_output {set $v = $_gdb_setting("trace-user")}
check_v_string "poulet"
}
}
}
with_test_prefix "maintenance setting" {
with_test_prefix "not set" {
foreach_with_prefix conv_func $::maint_conv_funcs {
gdb_test_no_output \
"set \$v = ${conv_func}(\"test-settings string\")"
check_v_string ""
}
}
with_test_prefix "set to empty" {
gdb_test "maintenance set test-settings string"
foreach_with_prefix conv_func $::maint_conv_funcs {
gdb_test_no_output \
"set \$v = ${conv_func}(\"test-settings string\")"
check_v_string ""
}
}
with_test_prefix "set" {
gdb_test "maintenance set test-settings string perchaude"
foreach_with_prefix conv_func $::maint_conv_funcs {
gdb_test_no_output \
"set \$v = ${conv_func}(\"test-settings string\")"
check_v_string "perchaude"
}
}
}
# Test with a non-string setting, this tests yet another code path.
with_test_prefix "integer setting" {
gdb_test_no_output {set $v = $_gdb_setting_str("remotetimeout")}
check_v_string "2"
}
# Test string values made by $_gdb_setting & co. in all languages.
with_test_prefix "all langs" {
# Get list of supported languages.
set langs [gdb_supported_languages]
gdb_test "maintenance set test-settings string foo"
foreach_with_prefix lang $langs {
gdb_test_no_output "set language $lang"
if {$lang == "modula-2"} {
# The Modula-2 parser doesn't know how to build a
# suitable string expression.
gdb_test "print \"foo\"" "strings are not implemented"
continue
}
if {$lang == "rust"} {
# Rust strings are actually structs, without a running
# inferior into which the string data can be pushed
# GDB can't print anything.
gdb_test "print \"foo\"" \
"evaluation of this expression requires the target program to be active"
gdb_test "print \$_gdb_maint_setting(\"test-settings string\")" \
"evaluation of this expression requires the target program to be active"
continue
}
if {$lang == "unknown"} {
# Skipped because expression parsing is not supported
# for the "unknown" language. See gdb/28093 for more
# details.
continue
}
set print_output ""
set ptype_output ""
set foo_str [quote_for_lang $lang foo]
gdb_test_multiple "print $foo_str" "" {
-wrap -re " = (.*)" {
set print_output $expect_out(1,string)
pass $gdb_test_name
}
}
gdb_test_multiple "ptype $foo_str" "" {
-wrap -re " = (.*)" {
set ptype_output $expect_out(1,string)
pass $gdb_test_name
}
}
set cmd_str [quote_for_lang $lang "test-settings string"]
set ptype_output_re [string_to_regexp $ptype_output]
set print_output_re [string_to_regexp $print_output]
foreach_with_prefix conv_func $::maint_conv_funcs {
gdb_test "print ${conv_func}($cmd_str)" \
" = $print_output_re"
gdb_test "ptype \$" \
" = $ptype_output_re"
}
}
}
}
# Test with a string value created by gdb.Value in Python.
proc_with_prefix test_python_value { } {
clean_restart
if {![allow_python_tests]} {
untested "skipping test_python_value"
return
}
gdb_test_no_output "python gdb.set_convenience_variable(\"v\", \"bar\")" \
"set convenience var"
check_v_string "bar"
}
# Test with a string value created by make-value in Guile.
proc_with_prefix test_guile_value { } {
clean_restart
if {![allow_guile_tests]} {
untested "skipping test_guile_value"
return
}
# We can't set a convenience var from Guile, but we can append to history.
# Do that, then transfer to a convenience var with a CLI command.
gdb_test_no_output "guile (use-modules (gdb))"
gdb_test_multiple "guile (history-append! (make-value \"foo\"))" "make value" {
-re -wrap "($::decimal)" {
set histnum $expect_out(1,string)
}
}
gdb_test_no_output "set \$v = \$$histnum"
check_v_string "foo"
}
# Test with a string value coming from a string internal var. The only internal
# vars of this type, at the time of writing, are $trace_func and $trace_file.
# They both require inspecting a trace frame. So if the target is capable start
# tracing, record one trace frame, and use $trace_func.
proc_with_prefix test_internal_var { } {
if {![gdb_trace_common_supports_arch]} {
unsupported "arch does not support trace"
return
}
clean_restart $::binfile
if {![runto_main]} {
fail "could not run to main"
return
}
if {![gdb_target_supports_trace]} {
unsupported "target does not support trace"
return
}
gdb_breakpoint "end"
gdb_test "trace trace_me" "Tracepoint $::decimal at $::hex.*"
gdb_test_no_output "tstart"
gdb_continue_to_breakpoint "breakpoint at end"
gdb_test_no_output "tstop"
gdb_test "tfind" "Found trace frame 0, tracepoint $::decimal.*"
gdb_test_no_output "set \$v = \$trace_func"
gdb_test "tfind none" "No longer looking at any trace frame.*"
check_v_string "trace_me"
}
test_setting
test_python_value
test_guile_value
test_internal_var