diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c index 56ae12a0c19..f6fc5ab854f 100644 --- a/gdb/cli/cli-cmds.c +++ b/gdb/cli/cli-cmds.c @@ -2146,7 +2146,8 @@ value_from_setting (const cmd_list_element *cmd, struct gdbarch *gdbarch) case var_filename: case var_enum: if (*(char **) cmd->var) - return value_cstring (*(char **) cmd->var, strlen (*(char **) cmd->var), + return value_cstring (*(char **) cmd->var, + strlen (*(char **) cmd->var) + 1, builtin_type (gdbarch)->builtin_char); else return value_cstring ("", 1, @@ -2198,7 +2199,7 @@ str_value_from_setting (const cmd_list_element *cmd, struct gdbarch *gdbarch) { std::string cmd_val = get_setshow_command_value_string (cmd); - return value_cstring (cmd_val.c_str (), cmd_val.size (), + return value_cstring (cmd_val.c_str (), cmd_val.size () + 1, builtin_type (gdbarch)->builtin_char); } @@ -2212,7 +2213,8 @@ str_value_from_setting (const cmd_list_element *cmd, struct gdbarch *gdbarch) escaping quotes. So, we directly use the cmd->var string value, similarly to the value_from_setting code for these cases. */ if (*(char **) cmd->var) - return value_cstring (*(char **) cmd->var, strlen (*(char **) cmd->var), + return value_cstring (*(char **) cmd->var, + strlen (*(char **) cmd->var) + 1, builtin_type (gdbarch)->builtin_char); else return value_cstring ("", 1, diff --git a/gdb/guile/scm-math.c b/gdb/guile/scm-math.c index d9fd6718196..8b7fe9b69ec 100644 --- a/gdb/guile/scm-math.c +++ b/gdb/guile/scm-math.c @@ -776,8 +776,6 @@ vlscm_convert_typed_value_from_scheme (const char *func_name, } else if (scm_is_string (obj)) { - size_t len; - if (type != NULL) { except_scm = gdbscm_make_misc_error (func_name, type_arg_pos, @@ -789,12 +787,12 @@ vlscm_convert_typed_value_from_scheme (const char *func_name, { /* TODO: Provide option to specify conversion strategy. */ gdb::unique_xmalloc_ptr s - = gdbscm_scm_to_string (obj, &len, + = gdbscm_scm_to_string (obj, nullptr, target_charset (gdbarch), 0 /*non-strict*/, &except_scm); if (s != NULL) - value = value_cstring (s.get (), len, + value = value_cstring (s.get (), strlen (s.get ()) + 1, language_string_char_type (language, gdbarch)); else diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c index 8df8a15f8d6..bc0f88b7a16 100644 --- a/gdb/python/py-value.c +++ b/gdb/python/py-value.c @@ -1907,7 +1907,7 @@ convert_value_from_python (PyObject *obj) gdb::unique_xmalloc_ptr s = python_string_to_target_string (obj); if (s != NULL) - value = value_cstring (s.get (), strlen (s.get ()), + value = value_cstring (s.get (), strlen (s.get ()) + 1, builtin_type_pychar); } else if (PyObject_TypeCheck (obj, &value_object_type)) diff --git a/gdb/testsuite/gdb.base/internal-string-values.c b/gdb/testsuite/gdb.base/internal-string-values.c new file mode 100644 index 00000000000..b9c7c847cbf --- /dev/null +++ b/gdb/testsuite/gdb.base/internal-string-values.c @@ -0,0 +1,32 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2021 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 . */ + +static void +trace_me (void) +{} + +static void +end (void) +{} + +int +main (void) +{ + trace_me (); + end (); + return 0; +} diff --git a/gdb/testsuite/gdb.base/internal-string-values.exp b/gdb/testsuite/gdb.base/internal-string-values.exp new file mode 100644 index 00000000000..0748ab0984c --- /dev/null +++ b/gdb/testsuite/gdb.base/internal-string-values.exp @@ -0,0 +1,198 @@ +# Copyright 2021 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 . + +# 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 +} + +# 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 {$_gdb_setting $_gdb_setting_str} { + 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 {$_gdb_setting $_gdb_setting_str} { + 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 {$_gdb_setting $_gdb_setting_str} { + 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 {$_gdb_maint_setting $_gdb_maint_setting_str} { + 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 {$_gdb_maint_setting $_gdb_maint_setting_str} { + 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 {$_gdb_maint_setting $_gdb_maint_setting_str} { + 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 with a string value created by gdb.Value in Python. + +proc_with_prefix test_python_value { } { + clean_restart + + if {[skip_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 {[skip_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_test "break end" "Breakpoint $::decimal at $::hex.*" + gdb_test "trace trace_me" "Tracepoint $::decimal at $::hex.*" + gdb_test_no_output "tstart" + gdb_test "continue" "Breakpoint $::decimal, 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 diff --git a/gdb/testsuite/gdb.base/settings.exp b/gdb/testsuite/gdb.base/settings.exp index 64257aaae5a..0a594685d6b 100644 --- a/gdb/testsuite/gdb.base/settings.exp +++ b/gdb/testsuite/gdb.base/settings.exp @@ -498,7 +498,7 @@ proc_with_prefix test-enum {} { gdb_test_no_output "$set_cmd zzz" show_setting "$show_cmd" "zzz" 0 "yyy" - check_type "test-settings enum" "type = char \\\[3\\\]" + check_type "test-settings enum" "type = char \\\[4\\\]" test_gdb_complete_multiple "$set_cmd " "" "" { "xxx" diff --git a/gdb/testsuite/gdb.python/py-mi.exp b/gdb/testsuite/gdb.python/py-mi.exp index 6f0c1c65e62..b81ee9acd74 100644 --- a/gdb/testsuite/gdb.python/py-mi.exp +++ b/gdb/testsuite/gdb.python/py-mi.exp @@ -289,7 +289,7 @@ mi_create_dynamic_varobj nstype2 nstype2 1 \ "create nstype2 varobj" mi_list_varobj_children nstype2 { - { {nstype2.} {} 6 {char \[6\]} } + { {nstype2.} {} 7 {char \[7\]} } } "list children after setting exception flag" mi_create_varobj me me \ diff --git a/gdb/valops.c b/gdb/valops.c index bd547923496..f67612e87b0 100644 --- a/gdb/valops.c +++ b/gdb/valops.c @@ -1747,6 +1747,8 @@ value_array (int lowbound, int highbound, struct value **elemvec) return val; } +/* See value.h. */ + struct value * value_cstring (const char *ptr, ssize_t len, struct type *char_type) { @@ -1761,14 +1763,7 @@ value_cstring (const char *ptr, ssize_t len, struct type *char_type) return val; } -/* Create a value for a string constant by allocating space in the - inferior, copying the data into that space, and returning the - address with type TYPE_CODE_STRING. PTR points to the string - constant data; LEN is number of characters. - - Note that string types are like array of char types with a lower - bound of zero and an upper bound of LEN - 1. Also note that the - string may contain embedded null bytes. */ +/* See value.h. */ struct value * value_string (const char *ptr, ssize_t len, struct type *char_type) diff --git a/gdb/value.c b/gdb/value.c index 6a07495d32b..db34d2e5762 100644 --- a/gdb/value.c +++ b/gdb/value.c @@ -2188,7 +2188,7 @@ value_of_internalvar (struct gdbarch *gdbarch, struct internalvar *var) break; case INTERNALVAR_STRING: - val = value_cstring (var->u.string, strlen (var->u.string), + val = value_cstring (var->u.string, strlen (var->u.string) + 1, builtin_type (gdbarch)->builtin_char); break; diff --git a/gdb/value.h b/gdb/value.h index 379cddafbe7..24392437ed0 100644 --- a/gdb/value.h +++ b/gdb/value.h @@ -782,8 +782,22 @@ class scoped_value_mark const struct value *m_value; }; +/* Create a not_lval value with type TYPE_CODE_ARRAY. + + PTR points to the string data; LEN is the length of the string including the + NULL terminator. */ + extern struct value *value_cstring (const char *ptr, ssize_t len, struct type *char_type); + +/* Create a not_lval value with type TYPE_CODE_STRING. + + PTR points to the string data; LEN is number of characters. + + Note that string types are like array of char types with a lower + bound of zero and an upper bound of LEN - 1. Also note that the + string may contain embedded null bytes. */ + extern struct value *value_string (const char *ptr, ssize_t len, struct type *char_type);