* [PATCH 0/4] Some TUI fixes
@ 2016-11-08 18:57 Andreas Arnez
2016-11-08 18:58 ` [PATCH 1/4] tui-disasm: Fix window content buffer overrun Andreas Arnez
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Andreas Arnez @ 2016-11-08 18:57 UTC (permalink / raw)
To: gdb-patches
This is a small series for fixing some problems in the TUI logic. The
first patch fixes a GDB crash, while the other patches make some of the
surrounding logic more consistent and readable.
Andreas Arnez (4):
tui-disasm: Fix window content buffer overrun
tui-disasm: Fix line buffer size calculation
tui-winsource: Allocate for actual lines only
tui-winsource: Remove failed-allocation logic
gdb/testsuite/gdb.base/tui-layout.c | 30 ++++++++++++++++++++++++
gdb/testsuite/gdb.base/tui-layout.exp | 17 +++++++++++---
gdb/tui/tui-disasm.c | 43 ++++++++++++++++-------------------
gdb/tui/tui-winsource.c | 22 ++++--------------
4 files changed, 69 insertions(+), 43 deletions(-)
create mode 100644 gdb/testsuite/gdb.base/tui-layout.c
--
2.5.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/4] tui-disasm: Fix window content buffer overrun
2016-11-08 18:57 [PATCH 0/4] Some TUI fixes Andreas Arnez
@ 2016-11-08 18:58 ` Andreas Arnez
2016-11-08 19:29 ` Pedro Alves
2016-11-08 18:59 ` [PATCH 2/4] tui-disasm: Fix line buffer size calculation Andreas Arnez
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Andreas Arnez @ 2016-11-08 18:58 UTC (permalink / raw)
To: gdb-patches
A user reported a GDB crash with TUI when trying to debug a function
with a long demangled C++ method name. It turned out that the logic for
displaying the TUI disassembly window has a bug that can cause a buffer
overrun, possibly overwriting GDB-internal data structures. In
particular, the logic performs an unguarded strcpy.
Another (harmless) bug in tui_alloc_source_buffer causes the buffer to
be two lines longer than needed. This may have made the crash appear
less frequently.
gdb/ChangeLog:
* tui/tui-disasm.c (tui_set_disassem_content): Fix line buffer
overrun due to unchecked strcpy.
gdb/testsuite/ChangeLog:
* gdb.base/tui-layout.c: New file.
* gdb.base/tui-layout.exp: Use tui-layout.c, to ensure that the
disassembly window contains very long lines.
---
gdb/testsuite/gdb.base/tui-layout.c | 30 ++++++++++++++++++++++++++++++
gdb/testsuite/gdb.base/tui-layout.exp | 17 ++++++++++++++---
gdb/tui/tui-disasm.c | 24 ++++++++++--------------
3 files changed, 54 insertions(+), 17 deletions(-)
create mode 100644 gdb/testsuite/gdb.base/tui-layout.c
diff --git a/gdb/testsuite/gdb.base/tui-layout.c b/gdb/testsuite/gdb.base/tui-layout.c
new file mode 100644
index 0000000..951bea7
--- /dev/null
+++ b/gdb/testsuite/gdb.base/tui-layout.c
@@ -0,0 +1,30 @@
+#define LONGER_NAME(x) x ## x
+#define LONGER(x) LONGER_NAME(x)
+#define LONGNAME1 d_this_identifier_of_32_chars_an
+#define LONGNAME2 LONGER (LONGER (LONGER (LONGER (LONGER (LONGNAME1)))))
+
+/* Construct a long identifier name. If SHORT_IDENTIFIERS is set, limit
+ it to 1024 chars. */
+
+#ifdef SHORT_IDENTIFIERS
+#define LONGNAME3 LONGNAME2
+#else
+#define LONGNAME3 LONGER (LONGER (LONGER (LONGER (LONGER (LONGNAME2)))))
+#endif
+
+void LONGNAME3 (void);
+
+int
+main ()
+{
+ LONGNAME3 ();
+ return 0;
+}
+
+/* Function with a long name. Placing it after main makes it more likely
+ to be shown in the disassembly window on startup. */
+
+void
+LONGNAME3 (void)
+{
+}
diff --git a/gdb/testsuite/gdb.base/tui-layout.exp b/gdb/testsuite/gdb.base/tui-layout.exp
index 43b3a4f..0c7169e 100644
--- a/gdb/testsuite/gdb.base/tui-layout.exp
+++ b/gdb/testsuite/gdb.base/tui-layout.exp
@@ -13,12 +13,23 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
-standard_testfile start.c
+standard_testfile
-if { [prepare_for_testing ${testfile}.exp ${testfile} $srcfile] } {
- return -1
+set ccopts {debug quiet}
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "$binfile" \
+ executable $ccopts] != "" } {
+ # Maybe the compiler can't handle arbitrarily long identfier names.
+ # Try with a shorter version.
+ lappend ccopts "additional_flags=-DSHORT_IDENTIFIERS"
+ if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "$binfile" \
+ executable $ccopts] != "" } {
+ fail "compile"
+ return -1
+ }
}
+clean_restart "$binfile"
+
if {[skip_tui_tests]} {
# TUI support is disabled. Check for error message.
gdb_test "layout asm" "Undefined command: \"layout\". Try \"help\"."
diff --git a/gdb/tui/tui-disasm.c b/gdb/tui/tui-disasm.c
index 29c1443..5368aa4 100644
--- a/gdb/tui/tui-disasm.c
+++ b/gdb/tui/tui-disasm.c
@@ -172,7 +172,7 @@ tui_set_disassem_content (struct gdbarch *gdbarch, CORE_ADDR pc)
enum tui_status ret = TUI_FAILURE;
int i;
int offset = TUI_DISASM_WIN->detail.source_info.horizontal_offset;
- int max_lines;
+ int max_lines, line_width;
CORE_ADDR cur_pc;
struct tui_gen_win_info *locator = tui_locator_win_info_ptr ();
int tab_len = tui_default_tab_len ();
@@ -193,8 +193,9 @@ tui_set_disassem_content (struct gdbarch *gdbarch, CORE_ADDR pc)
TUI_DISASM_WIN->detail.source_info.start_line_or_addr.u.addr = pc;
cur_pc = locator->content[0]->which_element.locator.addr;
- max_lines = TUI_DISASM_WIN->generic.height - 2; /* Account for
- hilite. */
+ /* Window size, excluding highlight box. */
+ max_lines = TUI_DISASM_WIN->generic.height - 2;
+ line_width = TUI_DISASM_WIN->generic.width - 2;
/* Get temporary table that will hold all strings (addr & insn). */
asm_lines = XALLOCAVEC (struct tui_asm_line, max_lines);
@@ -233,20 +234,15 @@ tui_set_disassem_content (struct gdbarch *gdbarch, CORE_ADDR pc)
src = &element->which_element.source;
strcpy (line, asm_lines[i].addr_string);
cur_len = strlen (line);
-
- /* Add spaces to make the instructions start on the same
- column. */
- while (cur_len < insn_pos)
- {
- strcat (line, " ");
- cur_len++;
- }
-
- strcat (line, asm_lines[i].insn);
+ memset (line + cur_len, ' ', insn_pos - cur_len);
+ strcpy (line + insn_pos, asm_lines[i].insn);
/* Now copy the line taking the offset into account. */
if (strlen (line) > offset)
- strcpy (src->line, &line[offset]);
+ {
+ strncpy (src->line, &line[offset], line_width);
+ src->line[line_width] = '\0';
+ }
else
src->line[0] = '\0';
--
2.5.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/4] tui-disasm: Fix line buffer size calculation
2016-11-08 18:57 [PATCH 0/4] Some TUI fixes Andreas Arnez
2016-11-08 18:58 ` [PATCH 1/4] tui-disasm: Fix window content buffer overrun Andreas Arnez
@ 2016-11-08 18:59 ` Andreas Arnez
2016-11-08 19:30 ` Pedro Alves
2016-11-08 18:59 ` [PATCH 3/4] tui-winsource: Allocate for actual lines only Andreas Arnez
2016-11-08 19:00 ` [PATCH 4/4] tui-winsource: Remove failed-allocation logic Andreas Arnez
3 siblings, 1 reply; 10+ messages in thread
From: Andreas Arnez @ 2016-11-08 18:59 UTC (permalink / raw)
To: gdb-patches
The code that fills the TUI disassembly window content first calculates
the maximum full length of a displayed disassembly line. This
calculation typically yields the wrong result. The result is too large,
so the bug does not cause any run-time failures, but unnecessary
confusion for the reader. This patch fixes the calculation.
gdb/ChangeLog:
* tui/tui-disasm.c (tui_set_disassem_content): Fix calculation of
the longest disassembly line's length.
---
gdb/tui/tui-disasm.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/gdb/tui/tui-disasm.c b/gdb/tui/tui-disasm.c
index 5368aa4..6811be3 100644
--- a/gdb/tui/tui-disasm.c
+++ b/gdb/tui/tui-disasm.c
@@ -178,7 +178,7 @@ tui_set_disassem_content (struct gdbarch *gdbarch, CORE_ADDR pc)
int tab_len = tui_default_tab_len ();
struct tui_asm_line *asm_lines;
int insn_pos;
- int addr_size, max_size;
+ int addr_size, insn_size;
char *line;
if (pc == 0)
@@ -203,9 +203,9 @@ tui_set_disassem_content (struct gdbarch *gdbarch, CORE_ADDR pc)
tui_disassemble (gdbarch, asm_lines, pc, max_lines);
- /* See what is the maximum length of an address and of a line. */
+ /* Determine maximum address- and instruction lengths. */
addr_size = 0;
- max_size = 0;
+ insn_size = 0;
for (i = 0; i < max_lines; i++)
{
size_t len = strlen (asm_lines[i].addr_string);
@@ -213,16 +213,17 @@ tui_set_disassem_content (struct gdbarch *gdbarch, CORE_ADDR pc)
if (len > addr_size)
addr_size = len;
- len = strlen (asm_lines[i].insn) + tab_len;
- if (len > max_size)
- max_size = len;
+ len = strlen (asm_lines[i].insn);
+ if (len > insn_size)
+ insn_size = len;
}
- max_size += addr_size + tab_len;
- /* Allocate memory to create each line. */
- line = (char*) alloca (max_size);
+ /* Align instructions to the same column. */
insn_pos = (1 + (addr_size / tab_len)) * tab_len;
+ /* Allocate memory to create each line. */
+ line = (char*) alloca (insn_pos + insn_size + 1);
+
/* Now construct each line. */
for (i = 0; i < max_lines; i++)
{
--
2.5.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/4] tui-winsource: Allocate for actual lines only
2016-11-08 18:57 [PATCH 0/4] Some TUI fixes Andreas Arnez
2016-11-08 18:58 ` [PATCH 1/4] tui-disasm: Fix window content buffer overrun Andreas Arnez
2016-11-08 18:59 ` [PATCH 2/4] tui-disasm: Fix line buffer size calculation Andreas Arnez
@ 2016-11-08 18:59 ` Andreas Arnez
2016-11-08 19:31 ` Pedro Alves
2016-11-08 19:00 ` [PATCH 4/4] tui-winsource: Remove failed-allocation logic Andreas Arnez
3 siblings, 1 reply; 10+ messages in thread
From: Andreas Arnez @ 2016-11-08 18:59 UTC (permalink / raw)
To: gdb-patches
The logic for allocating a TUI source window's content buffer allocates
two more lines than needed, because it does not reduce the window height
by the highlight box's overhead. However, it does reduce the line width
accordingly. This patch makes the height and width calculation
consistent and improves the comment.
gdb/ChangeLog:
* tui/tui-winsource.c (tui_alloc_source_buffer): Subtract
highlight box's overhead when calculating the content height.
---
gdb/tui/tui-winsource.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index 09080b8..4a82ae4 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -604,8 +604,11 @@ tui_alloc_source_buffer (struct tui_win_info *win_info)
char *src_line_buf;
int i, line_width, max_lines;
- max_lines = win_info->generic.height; /* Less the highlight box. */
- line_width = win_info->generic.width - 1;
+ /* The window width/height includes the highlight box. Determine actual
+ content dimensions, including string null-terminators. */
+ max_lines = win_info->generic.height - 2;
+ line_width = win_info->generic.width - 2 + 1;
+
/*
* Allocate the buffer for the source lines. Do this only once
* since they will be re-used for all source displays. The only
--
2.5.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 4/4] tui-winsource: Remove failed-allocation logic
2016-11-08 18:57 [PATCH 0/4] Some TUI fixes Andreas Arnez
` (2 preceding siblings ...)
2016-11-08 18:59 ` [PATCH 3/4] tui-winsource: Allocate for actual lines only Andreas Arnez
@ 2016-11-08 19:00 ` Andreas Arnez
2016-11-08 19:31 ` Pedro Alves
3 siblings, 1 reply; 10+ messages in thread
From: Andreas Arnez @ 2016-11-08 19:00 UTC (permalink / raw)
To: gdb-patches
This removes dead code in tui_alloc_source_buffer for handling a NULL
return value from xmalloc.
gdb/ChangeLog:
* tui/tui-winsource.c (tui_alloc_source_buffer): Remove
failed-xmalloc handling.
---
gdb/tui/tui-winsource.c | 15 ---------------
1 file changed, 15 deletions(-)
diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index 4a82ae4..8bbce83 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -618,23 +618,8 @@ tui_alloc_source_buffer (struct tui_win_info *win_info)
{
src_line_buf = (char *)
xmalloc ((max_lines * line_width) * sizeof (char));
- if (src_line_buf == (char *) NULL)
- {
- fputs_unfiltered ("Unable to Allocate Memory for "
- "Source or Disassembly Display.\n",
- gdb_stderr);
- return TUI_FAILURE;
- }
/* Allocate the content list. */
win_info->generic.content = tui_alloc_content (max_lines, SRC_WIN);
- if (win_info->generic.content == NULL)
- {
- xfree (src_line_buf);
- fputs_unfiltered ("Unable to Allocate Memory for "
- "Source or Disassembly Display.\n",
- gdb_stderr);
- return TUI_FAILURE;
- }
for (i = 0; i < max_lines; i++)
win_info->generic.content[i]->which_element.source.line
= src_line_buf + (line_width * i);
--
2.5.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] tui-disasm: Fix window content buffer overrun
2016-11-08 18:58 ` [PATCH 1/4] tui-disasm: Fix window content buffer overrun Andreas Arnez
@ 2016-11-08 19:29 ` Pedro Alves
2016-11-09 12:07 ` Andreas Arnez
0 siblings, 1 reply; 10+ messages in thread
From: Pedro Alves @ 2016-11-08 19:29 UTC (permalink / raw)
To: Andreas Arnez, gdb-patches
On 11/08/2016 06:56 PM, Andreas Arnez wrote:
> A user reported a GDB crash with TUI when trying to debug a function
> with a long demangled C++ method name. It turned out that the logic for
> displaying the TUI disassembly window has a bug that can cause a buffer
> overrun, possibly overwriting GDB-internal data structures. In
> particular, the logic performs an unguarded strcpy.
>
> Another (harmless) bug in tui_alloc_source_buffer causes the buffer to
> be two lines longer than needed. This may have made the crash appear
> less frequently.
>
> gdb/ChangeLog:
>
> * tui/tui-disasm.c (tui_set_disassem_content): Fix line buffer
> overrun due to unchecked strcpy.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.base/tui-layout.c: New file.
> * gdb.base/tui-layout.exp: Use tui-layout.c, to ensure that the
> disassembly window contains very long lines.
> ---
> gdb/testsuite/gdb.base/tui-layout.c | 30 ++++++++++++++++++++++++++++++
> gdb/testsuite/gdb.base/tui-layout.exp | 17 ++++++++++++++---
> gdb/tui/tui-disasm.c | 24 ++++++++++--------------
> 3 files changed, 54 insertions(+), 17 deletions(-)
> create mode 100644 gdb/testsuite/gdb.base/tui-layout.c
>
> diff --git a/gdb/testsuite/gdb.base/tui-layout.c b/gdb/testsuite/gdb.base/tui-layout.c
> new file mode 100644
> index 0000000..951bea7
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/tui-layout.c
> @@ -0,0 +1,30 @@
Missing copyright header.
> +#define LONGER_NAME(x) x ## x
> +#define LONGER(x) LONGER_NAME(x)
> +#define LONGNAME1 d_this_identifier_of_32_chars_an
> +#define LONGNAME2 LONGER (LONGER (LONGER (LONGER (LONGER (LONGNAME1)))))
Funny, I was just playing with throwing a large
function name at the compiler last week, after seeing:
https://sourceware.org/ml/binutils/2016-11/msg00027.html
#define STRCAT_1(a) a ## a
#define STRCAT(a) STRCAT_1 (a)
#define F1 x
#define F2 STRCAT (F1)
#define F4 STRCAT (F2)
#define F8 STRCAT (F4)
#define F10 STRCAT (F8)
#define F20 STRCAT (F10)
#define F40 STRCAT (F20)
#define F80 STRCAT (F40)
#define F100 STRCAT (F80)
#define F200 STRCAT (F100)
#define F400 STRCAT (F200)
#define F800 STRCAT (F400)
#define F1000 STRCAT (F800)
#define F2000 STRCAT (F1000)
#define F4000 STRCAT (F2000)
#define F8000 STRCAT (F4000)
#define F10000 STRCAT (F8000)
#define F20000 STRCAT (F10000)
#define F40000 STRCAT (F20000)
#define F80000 STRCAT (F40000)
#define F100000 STRCAT (F80000)
#define F200000 STRCAT (F100000)
#define F400000 STRCAT (F200000)
#define F800000 STRCAT (F400000)
#define F1000000 STRCAT (F800000)
#define F2000000 STRCAT (F1000000)
#define F4000000 STRCAT (F2000000)
#define F8000000 STRCAT (F4000000)
#define F10000000 STRCAT (F8000000)
#define F20000000 STRCAT (F10000000)
#define F40000000 STRCAT (F20000000)
#define F80000000 STRCAT (F40000000)
#define bigfunctionname F80000000
void
bigfunctionname ()
{
}
int
main ()
{
bigfunctionname ();
bigfunctionname ();
bigfunctionname ();
bigfunctionname ();
bigfunctionname ();
bigfunctionname ();
return 0;
}
$ /opt/gcc/bin/gcc -v
[...]
gcc version 7.0.0 20161024 (experimental) (GCC)
$ /opt/gcc/bin/gcc big.c -o big
gcc: internal compiler error: Segmentation fault (program cc1)
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.
Yay! :-)
> +
> +/* Construct a long identifier name. If SHORT_IDENTIFIERS is set, limit
> + it to 1024 chars. */
> +
> +#ifdef SHORT_IDENTIFIERS
> +#define LONGNAME3 LONGNAME2
> +#else
> +#define LONGNAME3 LONGER (LONGER (LONGER (LONGER (LONGER (LONGNAME2)))))
> +#endif
> +
> +void LONGNAME3 (void);
> +
> +int
> +main ()
> +{
> + LONGNAME3 ();
> + return 0;
> +}
> +
> +/* Function with a long name. Placing it after main makes it more likely
> + to be shown in the disassembly window on startup. */
> +
> +void
> +LONGNAME3 (void)
> +{
> +}
> diff --git a/gdb/testsuite/gdb.base/tui-layout.exp b/gdb/testsuite/gdb.base/tui-layout.exp
> index 43b3a4f..0c7169e 100644
> --- a/gdb/testsuite/gdb.base/tui-layout.exp
> +++ b/gdb/testsuite/gdb.base/tui-layout.exp
> @@ -13,12 +13,23 @@
> # You should have received a copy of the GNU General Public License
> # along with this program. If not, see <http://www.gnu.org/licenses/>.
>
> -standard_testfile start.c
> +standard_testfile
>
> -if { [prepare_for_testing ${testfile}.exp ${testfile} $srcfile] } {
> - return -1
> +set ccopts {debug quiet}
> +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "$binfile" \
> + executable $ccopts] != "" } {
> + # Maybe the compiler can't handle arbitrarily long identfier names.
> + # Try with a shorter version.
> + lappend ccopts "additional_flags=-DSHORT_IDENTIFIERS"
> + if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "$binfile" \
> + executable $ccopts] != "" } {
> + fail "compile"
Shouldn't this be "untested" ?
> + return -1
> + }
> }
>
> +clean_restart "$binfile"
> +
> if {[skip_tui_tests]} {
> # TUI support is disabled. Check for error message.
> gdb_test "layout asm" "Undefined command: \"layout\". Try \"help\"."
> diff --git a/gdb/tui/tui-disasm.c b/gdb/tui/tui-disasm.c
> index 29c1443..5368aa4 100644
> --- a/gdb/tui/tui-disasm.c
> +++ b/gdb/tui/tui-disasm.c
> @@ -172,7 +172,7 @@ tui_set_disassem_content (struct gdbarch *gdbarch, CORE_ADDR pc)
> enum tui_status ret = TUI_FAILURE;
> int i;
> int offset = TUI_DISASM_WIN->detail.source_info.horizontal_offset;
> - int max_lines;
> + int max_lines, line_width;
> CORE_ADDR cur_pc;
> struct tui_gen_win_info *locator = tui_locator_win_info_ptr ();
> int tab_len = tui_default_tab_len ();
> @@ -193,8 +193,9 @@ tui_set_disassem_content (struct gdbarch *gdbarch, CORE_ADDR pc)
> TUI_DISASM_WIN->detail.source_info.start_line_or_addr.u.addr = pc;
> cur_pc = locator->content[0]->which_element.locator.addr;
>
> - max_lines = TUI_DISASM_WIN->generic.height - 2; /* Account for
> - hilite. */
> + /* Window size, excluding highlight box. */
> + max_lines = TUI_DISASM_WIN->generic.height - 2;
> + line_width = TUI_DISASM_WIN->generic.width - 2;
>
> /* Get temporary table that will hold all strings (addr & insn). */
> asm_lines = XALLOCAVEC (struct tui_asm_line, max_lines);
> @@ -233,20 +234,15 @@ tui_set_disassem_content (struct gdbarch *gdbarch, CORE_ADDR pc)
> src = &element->which_element.source;
> strcpy (line, asm_lines[i].addr_string);
> cur_len = strlen (line);
> -
> - /* Add spaces to make the instructions start on the same
> - column. */
> - while (cur_len < insn_pos)
> - {
> - strcat (line, " ");
> - cur_len++;
> - }
> -
> - strcat (line, asm_lines[i].insn);
> + memset (line + cur_len, ' ', insn_pos - cur_len);
> + strcpy (line + insn_pos, asm_lines[i].insn);
>
> /* Now copy the line taking the offset into account. */
> if (strlen (line) > offset)
> - strcpy (src->line, &line[offset]);
> + {
> + strncpy (src->line, &line[offset], line_width);
> + src->line[line_width] = '\0';
> + }
> else
> src->line[0] = '\0';
>
>
This is OK.
This could probably all be greatly simplified with std::string
and getting rid of the alloca...
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/4] tui-disasm: Fix line buffer size calculation
2016-11-08 18:59 ` [PATCH 2/4] tui-disasm: Fix line buffer size calculation Andreas Arnez
@ 2016-11-08 19:30 ` Pedro Alves
0 siblings, 0 replies; 10+ messages in thread
From: Pedro Alves @ 2016-11-08 19:30 UTC (permalink / raw)
To: Andreas Arnez, gdb-patches
On 11/08/2016 06:56 PM, Andreas Arnez wrote:
> The code that fills the TUI disassembly window content first calculates
> the maximum full length of a displayed disassembly line. This
> calculation typically yields the wrong result. The result is too large,
> so the bug does not cause any run-time failures, but unnecessary
> confusion for the reader. This patch fixes the calculation.
>
> gdb/ChangeLog:
>
> * tui/tui-disasm.c (tui_set_disassem_content): Fix calculation of
> the longest disassembly line's length.
OK.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/4] tui-winsource: Allocate for actual lines only
2016-11-08 18:59 ` [PATCH 3/4] tui-winsource: Allocate for actual lines only Andreas Arnez
@ 2016-11-08 19:31 ` Pedro Alves
0 siblings, 0 replies; 10+ messages in thread
From: Pedro Alves @ 2016-11-08 19:31 UTC (permalink / raw)
To: Andreas Arnez, gdb-patches
On 11/08/2016 06:56 PM, Andreas Arnez wrote:
> The logic for allocating a TUI source window's content buffer allocates
> two more lines than needed, because it does not reduce the window height
> by the highlight box's overhead. However, it does reduce the line width
> accordingly. This patch makes the height and width calculation
> consistent and improves the comment.
>
> gdb/ChangeLog:
>
> * tui/tui-winsource.c (tui_alloc_source_buffer): Subtract
> highlight box's overhead when calculating the content height.
OK.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] tui-winsource: Remove failed-allocation logic
2016-11-08 19:00 ` [PATCH 4/4] tui-winsource: Remove failed-allocation logic Andreas Arnez
@ 2016-11-08 19:31 ` Pedro Alves
0 siblings, 0 replies; 10+ messages in thread
From: Pedro Alves @ 2016-11-08 19:31 UTC (permalink / raw)
To: Andreas Arnez, gdb-patches
On 11/08/2016 06:56 PM, Andreas Arnez wrote:
> This removes dead code in tui_alloc_source_buffer for handling a NULL
> return value from xmalloc.
>
> gdb/ChangeLog:
>
> * tui/tui-winsource.c (tui_alloc_source_buffer): Remove
> failed-xmalloc handling.
OK.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] tui-disasm: Fix window content buffer overrun
2016-11-08 19:29 ` Pedro Alves
@ 2016-11-09 12:07 ` Andreas Arnez
0 siblings, 0 replies; 10+ messages in thread
From: Andreas Arnez @ 2016-11-09 12:07 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On Tue, Nov 08 2016, Pedro Alves wrote:
[...]
> Missing copyright header.
Added.
> Funny, I was just playing with throwing a large
> function name at the compiler last week, after seeing:
>
> https://sourceware.org/ml/binutils/2016-11/msg00027.html
Wow, "mangled name of some 590K characters"... that's impressive. The
TUI crash already occurs with much shorter names. On the other hand,
IIRC, I had a version of my test with more than that (I think it was
over 1MB), and it worked. So I'm not sure why I didn't hit the problem
above.
[...]
>> + fail "compile"
>
> Shouldn't this be "untested" ?
Yeah, probably more appropriate. Changed.
[...]
> This is OK.
Thanks. I pushed the whole patch series with the changes above.
> This could probably all be greatly simplified with std::string
> and getting rid of the alloca...
Also note that the current implementation has a usability issue. With
long mangled names, the instructions are no longer visible on the
screen, and a lot of scrolling may be required. See also
https://sourceware.org/bugzilla/show_bug.cgi?id=15580
Of course, fixing that would be a different topic.
--
Andreas
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-11-09 12:07 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-08 18:57 [PATCH 0/4] Some TUI fixes Andreas Arnez
2016-11-08 18:58 ` [PATCH 1/4] tui-disasm: Fix window content buffer overrun Andreas Arnez
2016-11-08 19:29 ` Pedro Alves
2016-11-09 12:07 ` Andreas Arnez
2016-11-08 18:59 ` [PATCH 2/4] tui-disasm: Fix line buffer size calculation Andreas Arnez
2016-11-08 19:30 ` Pedro Alves
2016-11-08 18:59 ` [PATCH 3/4] tui-winsource: Allocate for actual lines only Andreas Arnez
2016-11-08 19:31 ` Pedro Alves
2016-11-08 19:00 ` [PATCH 4/4] tui-winsource: Remove failed-allocation logic Andreas Arnez
2016-11-08 19:31 ` Pedro Alves
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox