From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5421 invoked by alias); 8 Feb 2013 23:32:30 -0000 Received: (qmail 5398 invoked by uid 22791); 8 Feb 2013 23:32:29 -0000 X-SWARE-Spam-Status: No, hits=-4.8 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,KHOP_RCVD_TRUST,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE,RP_MATCHES_RCVD,TW_DF,TW_WC X-Spam-Check-By: sourceware.org Received: from mail-wi0-f201.google.com (HELO mail-wi0-f201.google.com) (209.85.212.201) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 08 Feb 2013 23:32:19 +0000 Received: by mail-wi0-f201.google.com with SMTP id hm6so86389wib.0 for ; Fri, 08 Feb 2013 15:32:17 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:from:to:subject:date:message-id:mime-version :content-type:x-gm-message-state; bh=j6gn6jIwOA654/XfTX/QNJeJBfODPxFgy31SH5/YIVg=; b=U2tPYj4QF6+XQVVT8w3ZNUOp7MONAYThZSr6VNGrqaGXHI4sWKFLng7gdgNyTNr+pw nSbANahTsENBogCAFE6hz49fOp1t3p7F9ErTs/SvN3t4RbLELjd8pejfQdqU1i4BHwrP oW35KWhbIAlE6LnN0C8z0OOXBrQhfuLwEpIuY3MJVNWuGmkF5FecFX/HAGdAZ4FCOGlz 2zaiSgs+uhRUHLJ1zhwywPCqWTytbcw46ZT6I5WAdEWjD5FxtlcFk2x4QH517zKURfLd Hm4gV0vfxmnLGSJqr2JGF8+sC5gX4IQAhXr0YH5QhiiHrCYJEYfzjwuS6x3P588G3cvG k1PQ== X-Received: by 10.14.177.72 with SMTP id c48mr7191217eem.5.1360366337530; Fri, 08 Feb 2013 15:32:17 -0800 (PST) Received: from corp2gmr1-2.eem.corp.google.com (corp2gmr1-2.eem.corp.google.com [172.25.138.117]) by gmr-mx.google.com with ESMTPS id 6si4856904eej.0.2013.02.08.15.32.17 (version=TLSv1.1 cipher=AES128-SHA bits=128/128); Fri, 08 Feb 2013 15:32:17 -0800 (PST) Received: from ruffy2.mtv.corp.google.com (ruffy2.mtv.corp.google.com [172.17.128.107]) by corp2gmr1-2.eem.corp.google.com (Postfix) with ESMTP id C4AF71E40A0 for ; Fri, 8 Feb 2013 15:32:16 -0800 (PST) From: Doug Evans To: gdb-patches@sourceware.org Subject: [RFA] split up ui_printf, fix decfloat printing bugs Date: Fri, 08 Feb 2013 23:32:00 -0000 Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Gm-Message-State: ALoCoQmVCzPsliFy/O4BY43yR3w4cGOt5Ry3CZUGFCwvPj6Khw+RSeyj5X4anmx4GXyGRsHPy1xcpHwhDbXpkqXzv5i6l0f5nzMHutTmuywIut4ISicHVV96v2UrM+cdf/y8Tg4wKWsXPqjpQBpyuLW9s9X0FwzVkafDGas6Zx5nzLUBQmnqN37j05Dhvo0jKfXn09hF/rucf3zH8yPRgBP9unjAjaIBSdJPYIv82l0oJ8GNWpX+0Uk= X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2013-02/txt/msg00231.txt.bz2 Hi. While working on another patch, I found I wanted to break up some of the larger pieces in ui_printf into separate functions. This patch is mostly mechanical. I found a bug in decfloat printing (testcase included). I don't understand this code in ui_printf: /* Go through the whole format string and pull the correct number of chars back to compensate for the change in the format specifier. */ while (nnull_chars < nargs - i) { if (*eos == '\0') nnull_chars++; *++sos = *++eos; } It makes no sense given that the format string is parsed into pieces. Am I missing something? Regression tested on amd64-linux. Ok to check in? 2013-02-08 Doug Evans * printcmd.c (printf_c_string,printf_wide_c_string): New functions. (printf_decfloat): New function. Broken out from ui_printf. Remove unnecessary code to shift the entire format string down. (printf_pointer): New function. (ui_printf): Code to print C strings, wide C strings, decfloats, and pointers moved to separate functions. testsuite/ * gdb.base/printcmds.exp (test_printf_with_dfp): Add test for printing two decfloats. Index: printcmd.c =================================================================== RCS file: /cvs/src/src/gdb/printcmd.c,v retrieving revision 1.221 diff -u -p -r1.221 printcmd.c --- printcmd.c 3 Feb 2013 16:13:29 -0000 1.221 +++ printcmd.c 8 Feb 2013 22:56:31 -0000 @@ -1978,6 +1978,241 @@ print_variable_and_value (const char *na fprintf_filtered (stream, "\n"); } +/* Subroutine of ui_printf to simplify it. + Print VALUE to STREAM using FORMAT. + VALUE is either a C-style string on the target, or an internal variable. */ + +static void +printf_c_string (struct ui_file *stream, const char *format, + struct value *value) +{ + gdb_byte *str; + CORE_ADDR tem; + int j; + + tem = value_as_address (value); + + /* This is a %s argument. Find the length of the string. */ + for (j = 0;; j++) + { + gdb_byte c; + + QUIT; + read_memory (tem + j, &c, 1); + if (c == 0) + break; + } + + /* Copy the string contents into a string inside GDB. */ + str = (gdb_byte *) alloca (j + 1); + if (j != 0) + read_memory (tem, str, j); + str[j] = 0; + + fprintf_filtered (stream, format, (char *) str); +} + +/* Subroutine of ui_printf to simplify it. + Print VALUE to STREAM using FORMAT. + VALUE is either a wide C-style string on the target, + or an internal variable. */ + +static void +printf_wide_c_string (struct ui_file *stream, const char *format, + struct value *value) +{ + gdb_byte *str; + CORE_ADDR tem; + int j; + struct gdbarch *gdbarch = get_type_arch (value_type (value)); + enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); + struct type *wctype = lookup_typename (current_language, gdbarch, + "wchar_t", NULL, 0); + int wcwidth = TYPE_LENGTH (wctype); + gdb_byte *buf = alloca (wcwidth); + struct obstack output; + struct cleanup *inner_cleanup; + + tem = value_as_address (value); + + /* This is a %s argument. Find the length of the string. */ + for (j = 0;; j += wcwidth) + { + QUIT; + read_memory (tem + j, buf, wcwidth); + if (extract_unsigned_integer (buf, wcwidth, byte_order) == 0) + break; + } + + /* Copy the string contents into a string inside GDB. */ + str = (gdb_byte *) alloca (j + wcwidth); + if (j != 0) + read_memory (tem, str, j); + memset (&str[j], 0, wcwidth); + + obstack_init (&output); + inner_cleanup = make_cleanup_obstack_free (&output); + + convert_between_encodings (target_wide_charset (gdbarch), + host_charset (), + str, j, wcwidth, + &output, translit_char); + obstack_grow_str0 (&output, ""); + + fprintf_filtered (stream, format, obstack_base (&output)); + do_cleanups (inner_cleanup); +} + +/* Subroutine of ui_printf to simplify it. + Print VALUE, a decimal floating point value, to STREAM using FORMAT. */ + +static void +printf_decfloat (struct ui_file *stream, const char *format, + struct value *value) +{ + const gdb_byte *param_ptr = value_contents (value); + +#if defined (PRINTF_HAS_DECFLOAT) + /* If we have native support for Decimal floating + printing, handle it here. */ + fprintf_filtered (stream, format, param_ptr); +#else + /* As a workaround until vasprintf has native support for DFP + we convert the DFP values to string and print them using + the %s format specifier. */ + const char *p; + + /* Parameter data. */ + struct type *param_type = value_type (value); + struct gdbarch *gdbarch = get_type_arch (param_type); + enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); + + /* DFP output data. */ + struct value *dfp_value = NULL; + gdb_byte *dfp_ptr; + int dfp_len = 16; + gdb_byte dec[16]; + struct type *dfp_type = NULL; + char decstr[MAX_DECIMAL_STRING]; + + /* Points to the end of the string so that we can go back + and check for DFP length modifiers. */ + p = format + strlen (format); + + /* Look for the float/double format specifier. */ + while (*p != 'f' && *p != 'e' && *p != 'E' + && *p != 'g' && *p != 'G') + p--; + + /* Search for the '%' char and extract the size and type of + the output decimal value based on its modifiers + (%Hf, %Df, %DDf). */ + while (*--p != '%') + { + if (*p == 'H') + { + dfp_len = 4; + dfp_type = builtin_type (gdbarch)->builtin_decfloat; + } + else if (*p == 'D' && *(p - 1) == 'D') + { + dfp_len = 16; + dfp_type = builtin_type (gdbarch)->builtin_declong; + p--; + } + else + { + dfp_len = 8; + dfp_type = builtin_type (gdbarch)->builtin_decdouble; + } + } + + /* Conversion between different DFP types. */ + if (TYPE_CODE (param_type) == TYPE_CODE_DECFLOAT) + decimal_convert (param_ptr, TYPE_LENGTH (param_type), + byte_order, dec, dfp_len, byte_order); + else + /* If this is a non-trivial conversion, just output 0. + A correct converted value can be displayed by explicitly + casting to a DFP type. */ + decimal_from_string (dec, dfp_len, byte_order, "0"); + + dfp_value = value_from_decfloat (dfp_type, dec); + + dfp_ptr = (gdb_byte *) value_contents (dfp_value); + + decimal_to_string (dfp_ptr, dfp_len, byte_order, decstr); + + /* Print the DFP value. */ + fprintf_filtered (stream, "%s", decstr); +#endif +} + +/* Subroutine of ui_printf to simplify it. + Print VALUE, a target pointer, to STREAM using FORMAT. */ + +static void +printf_pointer (struct ui_file *stream, const char *format, + struct value *value) +{ + /* We avoid the host's %p because pointers are too + likely to be the wrong size. The only interesting + modifier for %p is a width; extract that, and then + handle %p as glibc would: %#x or a literal "(nil)". */ + + const char *p; + char *fmt, *fmt_p; +#ifdef PRINTF_HAS_LONG_LONG + long long val = value_as_long (value); +#else + long val = value_as_long (value); +#endif + + fmt = alloca (strlen (format) + 5); + + /* Copy up to the leading %. */ + p = format; + fmt_p = fmt; + while (*p) + { + int is_percent = (*p == '%'); + + *fmt_p++ = *p++; + if (is_percent) + { + if (*p == '%') + *fmt_p++ = *p++; + else + break; + } + } + + if (val != 0) + *fmt_p++ = '#'; + + /* Copy any width. */ + while (*p >= '0' && *p < '9') + *fmt_p++ = *p++; + + gdb_assert (*p == 'p' && *(p + 1) == '\0'); + if (val != 0) + { +#ifdef PRINTF_HAS_LONG_LONG + *fmt_p++ = 'l'; +#endif + *fmt_p++ = 'l'; + *fmt_p++ = 'x'; + *fmt_p++ = '\0'; + fprintf_filtered (stream, fmt, val); + } + else + { + *fmt_p++ = 's'; + *fmt_p++ = '\0'; + fprintf_filtered (stream, fmt, "(nil)"); + } +} + /* printf "printf format string" ARG to STREAM. */ static void @@ -2059,78 +2294,10 @@ ui_printf (char *arg, struct ui_file *st switch (fpieces[fr].argclass) { case string_arg: - { - gdb_byte *str; - CORE_ADDR tem; - int j; - - tem = value_as_address (val_args[i]); - - /* This is a %s argument. Find the length of the string. */ - for (j = 0;; j++) - { - gdb_byte c; - - QUIT; - read_memory (tem + j, &c, 1); - if (c == 0) - break; - } - - /* Copy the string contents into a string inside GDB. */ - str = (gdb_byte *) alloca (j + 1); - if (j != 0) - read_memory (tem, str, j); - str[j] = 0; - - fprintf_filtered (stream, current_substring, (char *) str); - } + printf_c_string (stream, current_substring, val_args[i]); break; case wide_string_arg: - { - gdb_byte *str; - CORE_ADDR tem; - int j; - struct gdbarch *gdbarch - = get_type_arch (value_type (val_args[i])); - enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); - struct type *wctype = lookup_typename (current_language, gdbarch, - "wchar_t", NULL, 0); - int wcwidth = TYPE_LENGTH (wctype); - gdb_byte *buf = alloca (wcwidth); - struct obstack output; - struct cleanup *inner_cleanup; - - tem = value_as_address (val_args[i]); - - /* This is a %s argument. Find the length of the string. */ - for (j = 0;; j += wcwidth) - { - QUIT; - read_memory (tem + j, buf, wcwidth); - if (extract_unsigned_integer (buf, wcwidth, byte_order) == 0) - break; - } - - /* Copy the string contents into a string inside GDB. */ - str = (gdb_byte *) alloca (j + wcwidth); - if (j != 0) - read_memory (tem, str, j); - memset (&str[j], 0, wcwidth); - - obstack_init (&output); - inner_cleanup = make_cleanup_obstack_free (&output); - - convert_between_encodings (target_wide_charset (gdbarch), - host_charset (), - str, j, wcwidth, - &output, translit_char); - obstack_grow_str0 (&output, ""); - - fprintf_filtered (stream, current_substring, - obstack_base (&output)); - do_cleanups (inner_cleanup); - } + printf_wide_c_string (stream, current_substring, val_args[i]); break; case wide_char_arg: { @@ -2227,169 +2394,13 @@ ui_printf (char *arg, struct ui_file *st fprintf_filtered (stream, current_substring, val); break; } - /* Handles decimal floating values. */ - case decfloat_arg: - { - const gdb_byte *param_ptr = value_contents (val_args[i]); - -#if defined (PRINTF_HAS_DECFLOAT) - /* If we have native support for Decimal floating - printing, handle it here. */ - fprintf_filtered (stream, current_substring, param_ptr); -#else - - /* As a workaround until vasprintf has native support for DFP - we convert the DFP values to string and print them using - the %s format specifier. */ - - char *eos, *sos; - int nnull_chars = 0; - - /* Parameter data. */ - struct type *param_type = value_type (val_args[i]); - struct gdbarch *gdbarch = get_type_arch (param_type); - enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); - - /* DFP output data. */ - struct value *dfp_value = NULL; - gdb_byte *dfp_ptr; - int dfp_len = 16; - gdb_byte dec[16]; - struct type *dfp_type = NULL; - char decstr[MAX_DECIMAL_STRING]; - - /* Points to the end of the string so that we can go back - and check for DFP length modifiers. */ - eos = current_substring + strlen (current_substring); - - /* Look for the float/double format specifier. */ - while (*eos != 'f' && *eos != 'e' && *eos != 'E' - && *eos != 'g' && *eos != 'G') - eos--; - - sos = eos; - - /* Search for the '%' char and extract the size and type of - the output decimal value based on its modifiers - (%Hf, %Df, %DDf). */ - while (*--sos != '%') - { - if (*sos == 'H') - { - dfp_len = 4; - dfp_type = builtin_type (gdbarch)->builtin_decfloat; - } - else if (*sos == 'D' && *(sos - 1) == 'D') - { - dfp_len = 16; - dfp_type = builtin_type (gdbarch)->builtin_declong; - sos--; - } - else - { - dfp_len = 8; - dfp_type = builtin_type (gdbarch)->builtin_decdouble; - } - } - - /* Replace %Hf, %Df and %DDf with %s's. */ - *++sos = 's'; - - /* Go through the whole format string and pull the correct - number of chars back to compensate for the change in the - format specifier. */ - while (nnull_chars < nargs - i) - { - if (*eos == '\0') - nnull_chars++; - - *++sos = *++eos; - } - - /* Conversion between different DFP types. */ - if (TYPE_CODE (param_type) == TYPE_CODE_DECFLOAT) - decimal_convert (param_ptr, TYPE_LENGTH (param_type), - byte_order, dec, dfp_len, byte_order); - else - /* If this is a non-trivial conversion, just output 0. - A correct converted value can be displayed by explicitly - casting to a DFP type. */ - decimal_from_string (dec, dfp_len, byte_order, "0"); - - dfp_value = value_from_decfloat (dfp_type, dec); - - dfp_ptr = (gdb_byte *) value_contents (dfp_value); - - decimal_to_string (dfp_ptr, dfp_len, byte_order, decstr); - - /* Print the DFP value. */ - fprintf_filtered (stream, current_substring, decstr); - - break; -#endif - } - + case decfloat_arg: + printf_decfloat (stream, current_substring, val_args[i]); + break; case ptr_arg: - { - /* We avoid the host's %p because pointers are too - likely to be the wrong size. The only interesting - modifier for %p is a width; extract that, and then - handle %p as glibc would: %#x or a literal "(nil)". */ - - char *p, *fmt, *fmt_p; -#ifdef PRINTF_HAS_LONG_LONG - long long val = value_as_long (val_args[i]); -#else - long val = value_as_long (val_args[i]); -#endif - - fmt = alloca (strlen (current_substring) + 5); - - /* Copy up to the leading %. */ - p = current_substring; - fmt_p = fmt; - while (*p) - { - int is_percent = (*p == '%'); - - *fmt_p++ = *p++; - if (is_percent) - { - if (*p == '%') - *fmt_p++ = *p++; - else - break; - } - } - - if (val != 0) - *fmt_p++ = '#'; - - /* Copy any width. */ - while (*p >= '0' && *p < '9') - *fmt_p++ = *p++; - - gdb_assert (*p == 'p' && *(p + 1) == '\0'); - if (val != 0) - { -#ifdef PRINTF_HAS_LONG_LONG - *fmt_p++ = 'l'; -#endif - *fmt_p++ = 'l'; - *fmt_p++ = 'x'; - *fmt_p++ = '\0'; - fprintf_filtered (stream, fmt, val); - } - else - { - *fmt_p++ = 's'; - *fmt_p++ = '\0'; - fprintf_filtered (stream, fmt, "(nil)"); - } - - break; - } + printf_pointer (stream, current_substring, val_args[i]); + break; case literal_piece: /* Print a portion of the format string that has no directives. Note that this will not include any Index: testsuite/gdb.base/printcmds.exp =================================================================== RCS file: /cvs/src/src/gdb/testsuite/gdb.base/printcmds.exp,v retrieving revision 1.48 diff -u -p -r1.48 printcmds.exp --- testsuite/gdb.base/printcmds.exp 1 Jan 2013 06:33:26 -0000 1.48 +++ testsuite/gdb.base/printcmds.exp 8 Feb 2013 22:56:31 -0000 @@ -770,6 +770,9 @@ proc test_printf_with_dfp {} { # The largest exponent for 128-bit dfp value is 6144. gdb_test "printf \"%DDf\\n\",1.2E6144dl" "1.200000000000000000000000000000000E\\+6144" + + # GDB used to get this wrong. + gdb_test "printf \"%Hf %Hf\\n\",1.2df,1.3df" "1.2 1.3" } proc test_print_symbol {} {