public inbox for gdb-cvs@sourceware.org
help / color / mirror / Atom feed
* [binutils-gdb] Fix undefined behavior in the Fortran, Go and Pascal number parsers
@ 2022-04-08 15:19 Pedro Alves
0 siblings, 0 replies; only message in thread
From: Pedro Alves @ 2022-04-08 15:19 UTC (permalink / raw)
To: gdb-cvs
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=01772c548b91499272dcb0f6864ba990e2abf873
commit 01772c548b91499272dcb0f6864ba990e2abf873
Author: Pedro Alves <pedro@palves.net>
Date: Thu Apr 7 18:01:12 2022 +0100
Fix undefined behavior in the Fortran, Go and Pascal number parsers
This commit ports these two fixes to the C parser:
commit ebf13736b42af47c9907b5157c8e80c78dbe00e1
CommitDate: Thu Sep 4 21:46:28 2014 +0100
parse_number("0") reads uninitialized memory
commit 20562150d8a894bc91657c843ee88c508188e32e
CommitDate: Wed Oct 3 15:19:06 2018 -0600
Avoid undefined behavior in parse_number
... to the Fortran, Go, and Fortran number parsers, fixing the same
problems there.
Also add a new testcase that exercises printing 0xffffffffffffffff
(max 64-bit) in all languages, which crashes a GDB built with UBsan
without the fix.
I moved get_set_option_choices out of all-architectures.exp.tcl to
common code to be able to extract all the supported languages. I did
a tweak to it to generalize it a bit -- you now have to pass down the
"set" part of the command as well. This is so that the proc can be
used with "maintenance set" commands as well in future.
Change-Id: I8e8f2fdc1e8407f63d923c26fd55d98148b9e16a
Diff:
---
gdb/f-exp.y | 8 ++--
gdb/go-exp.y | 12 +++---
gdb/p-exp.y | 12 +++---
gdb/testsuite/gdb.base/all-architectures.exp.tcl | 32 +++-----------
gdb/testsuite/gdb.base/parse_number.exp | 53 ++++++++++++++++++++++++
gdb/testsuite/lib/gdb.exp | 20 +++++++++
6 files changed, 93 insertions(+), 44 deletions(-)
diff --git a/gdb/f-exp.y b/gdb/f-exp.y
index f9622e63fb2..ae5cc4ef1f1 100644
--- a/gdb/f-exp.y
+++ b/gdb/f-exp.y
@@ -837,8 +837,8 @@ static int
parse_number (struct parser_state *par_state,
const char *p, int len, int parsed_float, YYSTYPE *putithere)
{
- LONGEST n = 0;
- LONGEST prevn = 0;
+ ULONGEST n = 0;
+ ULONGEST prevn = 0;
int c;
int base = input_radix;
int unsigned_p = 0;
@@ -869,7 +869,7 @@ parse_number (struct parser_state *par_state,
}
/* Handle base-switching prefixes 0x, 0t, 0d, 0 */
- if (p[0] == '0')
+ if (p[0] == '0' && len > 1)
switch (p[1])
{
case 'x':
@@ -929,7 +929,7 @@ parse_number (struct parser_state *par_state,
/* If range checking enabled, portably test for unsigned overflow. */
if (RANGE_CHECK && n != 0)
{
- if ((unsigned_p && (unsigned)prevn >= (unsigned)n))
+ if ((unsigned_p && prevn >= n))
range_error (_("Overflow on numeric constant."));
}
prevn = n;
diff --git a/gdb/go-exp.y b/gdb/go-exp.y
index 456920456cd..1b9b6ea7e90 100644
--- a/gdb/go-exp.y
+++ b/gdb/go-exp.y
@@ -647,10 +647,8 @@ static int
parse_number (struct parser_state *par_state,
const char *p, int len, int parsed_float, YYSTYPE *putithere)
{
- /* FIXME: Shouldn't these be unsigned? We don't deal with negative values
- here, and we do kind of silly things like cast to unsigned. */
- LONGEST n = 0;
- LONGEST prevn = 0;
+ ULONGEST n = 0;
+ ULONGEST prevn = 0;
ULONGEST un;
int i = 0;
@@ -702,7 +700,7 @@ parse_number (struct parser_state *par_state,
}
/* Handle base-switching prefixes 0x, 0t, 0d, 0. */
- if (p[0] == '0')
+ if (p[0] == '0' && len > 1)
switch (p[1])
{
case 'x':
@@ -790,7 +788,7 @@ parse_number (struct parser_state *par_state,
on 0x123456789 when LONGEST is 32 bits. */
if (c != 'l' && c != 'u' && n != 0)
{
- if ((unsigned_p && (ULONGEST) prevn >= (ULONGEST) n))
+ if ((unsigned_p && prevn >= n))
error (_("Numeric constant too large."));
}
prevn = n;
@@ -808,7 +806,7 @@ parse_number (struct parser_state *par_state,
the case where it is we just always shift the value more than
once, with fewer bits each time. */
- un = (ULONGEST)n >> 2;
+ un = n >> 2;
if (long_p == 0
&& (un >> (gdbarch_int_bit (par_state->gdbarch ()) - 2)) == 0)
{
diff --git a/gdb/p-exp.y b/gdb/p-exp.y
index adceff28636..3d1ea722d81 100644
--- a/gdb/p-exp.y
+++ b/gdb/p-exp.y
@@ -803,10 +803,8 @@ static int
parse_number (struct parser_state *par_state,
const char *p, int len, int parsed_float, YYSTYPE *putithere)
{
- /* FIXME: Shouldn't these be unsigned? We don't deal with negative values
- here, and we do kind of silly things like cast to unsigned. */
- LONGEST n = 0;
- LONGEST prevn = 0;
+ ULONGEST n = 0;
+ ULONGEST prevn = 0;
ULONGEST un;
int i = 0;
@@ -855,7 +853,7 @@ parse_number (struct parser_state *par_state,
}
/* Handle base-switching prefixes 0x, 0t, 0d, 0. */
- if (p[0] == '0')
+ if (p[0] == '0' && len > 1)
switch (p[1])
{
case 'x':
@@ -933,7 +931,7 @@ parse_number (struct parser_state *par_state,
on 0x123456789 when LONGEST is 32 bits. */
if (c != 'l' && c != 'u' && n != 0)
{
- if ((unsigned_p && (ULONGEST) prevn >= (ULONGEST) n))
+ if (unsigned_p && prevn >= n)
error (_("Numeric constant too large."));
}
prevn = n;
@@ -951,7 +949,7 @@ parse_number (struct parser_state *par_state,
the case where it is we just always shift the value more than
once, with fewer bits each time. */
- un = (ULONGEST)n >> 2;
+ un = n >> 2;
if (long_p == 0
&& (un >> (gdbarch_int_bit (par_state->gdbarch ()) - 2)) == 0)
{
diff --git a/gdb/testsuite/gdb.base/all-architectures.exp.tcl b/gdb/testsuite/gdb.base/all-architectures.exp.tcl
index 94b5efe80f3..d6679a1e9a1 100644
--- a/gdb/testsuite/gdb.base/all-architectures.exp.tcl
+++ b/gdb/testsuite/gdb.base/all-architectures.exp.tcl
@@ -91,45 +91,25 @@ proc gdb_test_internal {cmd pattern {message ""}} {
gdb_test_internal "set max-completions unlimited" \
"^set max-completions unlimited"
-# Return a list of all the accepted values of "set WHAT".
-
-proc get_set_option_choices {what} {
- global gdb_prompt
-
- set values {}
-
- set test "complete set $what"
- gdb_test_multiple "complete set $what " "$test" {
- -re "set $what (\[^\r\n\]+)\r\n" {
- lappend values $expect_out(1,string)
- exp_continue
- }
- -re "$gdb_prompt " {
- internal_pass $test
- }
- }
- return $values
-}
-
-set supported_archs [get_set_option_choices "architecture"]
+set supported_archs [get_set_option_choices "set architecture"]
# There should be at least one more than "auto".
gdb_assert {[llength $supported_archs] > 1} "at least one architecture"
-set supported_osabis [get_set_option_choices "osabi"]
+set supported_osabis [get_set_option_choices "set osabi"]
# There should be at least one more than "auto" and "default".
gdb_assert {[llength $supported_osabis] > 2} "at least one osabi"
if {[lsearch $supported_archs "mips"] >= 0} {
- set supported_mipsfpu [get_set_option_choices "mipsfpu"]
- set supported_mips_abi [get_set_option_choices "mips abi"]
+ set supported_mipsfpu [get_set_option_choices "set mipsfpu"]
+ set supported_mips_abi [get_set_option_choices "set mips abi"]
gdb_assert {[llength $supported_mipsfpu] != 0} "at least one mipsfpu"
gdb_assert {[llength $supported_mips_abi] != 0} "at least one mips abi"
}
if {[lsearch $supported_archs "arm"] >= 0} {
- set supported_arm_fpu [get_set_option_choices "arm fpu"]
- set supported_arm_abi [get_set_option_choices "arm abi"]
+ set supported_arm_fpu [get_set_option_choices "set arm fpu"]
+ set supported_arm_abi [get_set_option_choices "set arm abi"]
gdb_assert {[llength $supported_arm_fpu] != 0} "at least one arm fpu"
gdb_assert {[llength $supported_arm_abi] != 0} "at least one arm abi"
diff --git a/gdb/testsuite/gdb.base/parse_number.exp b/gdb/testsuite/gdb.base/parse_number.exp
new file mode 100644
index 00000000000..8849c99a3f9
--- /dev/null
+++ b/gdb/testsuite/gdb.base/parse_number.exp
@@ -0,0 +1,53 @@
+# Copyright 2022 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 parsing numbers. Several language parsers had the same bug
+# around parsing large 64-bit numbers, hitting undefined behavior, and
+# thus crashing a GDB built with UBSan. This testcase goes over all
+# languages exercising printing the max 64-bit number, making sure
+# that GDB doesn't crash.
+
+proc test_parse_numbers {} {
+ clean_restart
+
+ set all_languages [get_set_option_choices "set language"]
+ foreach_with_prefix lang $all_languages {
+ gdb_test_no_output "set language $lang"
+
+ set val "0xffffffffffffffff"
+ if {$lang == "ada"} {
+ gdb_test "p/x $val" "Integer literal out of range"
+ } elseif {$lang == "fortran"} {
+ gdb_test "p/x $val" " = 0xffffffff"
+ gdb_test "ptype $val" " = unsigned int"
+ } elseif {$lang == "modula-2"} {
+ gdb_test "p/x 0FFFFFFFFFFFFFFFFH" "Overflow on numeric constant\\."
+ } elseif {$lang == "unknown"} {
+ gdb_test "p/x $val" \
+ "expression parsing not implemented for language \"Unknown\""
+ } else {
+ gdb_test "p/x $val" " = $val"
+ if {$lang == "d"} {
+ gdb_test "ptype $val" " = ulong"
+ } elseif {$lang == "rust"} {
+ gdb_test "ptype $val" " = i64"
+ } else {
+ gdb_test "ptype $val" " = unsigned long long"
+ }
+ }
+ }
+}
+
+test_parse_numbers
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 0ef7e1215a4..2eb711748e7 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -8486,5 +8486,25 @@ gdb_caching_proc has_hw_wp_support {
return $has_hw_wp_support
}
+# Return a list of all the accepted values of the set command SET_CMD.
+
+proc get_set_option_choices {set_cmd} {
+ global gdb_prompt
+
+ set values {}
+
+ set test "complete $set_cmd"
+ gdb_test_multiple "complete $set_cmd " "$test" {
+ -re "$set_cmd (\[^\r\n\]+)\r\n" {
+ lappend values $expect_out(1,string)
+ exp_continue
+ }
+ -re "$gdb_prompt " {
+ pass $test
+ }
+ }
+ return $values
+}
+
# Always load compatibility stuff.
load_lib future.exp
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2022-04-08 15:19 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-08 15:19 [binutils-gdb] Fix undefined behavior in the Fortran, Go and Pascal number parsers Pedro Alves
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).