* [PATCH v2 3/5] dwarf2-frame.c: Fix FDE processing bug involving non-contiguous ranges
2019-07-04 4:55 [PATCH v2 0/5] Non-contiguous address range bug fixes / improvements Kevin Buettner
@ 2019-07-04 4:55 ` Kevin Buettner
2019-07-04 4:55 ` [PATCH v2 1/5] Prefer symtab symbol over minsym for function names in non-contiguous blocks Kevin Buettner
` (4 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Kevin Buettner @ 2019-07-04 4:55 UTC (permalink / raw)
To: gdb-patches; +Cc: Kevin Buettner
In the course of revising the test case for
gdb.dwarf2/dw2-ranges-func.exp, I added a new .c file which would
cause the "cold" range to be at a higher address than the rest of the
function. In these tests, the range in question isn't really cold in
the sense that a compiler has determined that it'll be executed less
frequently. Instead, it's simply the range that does not include the
entry pc. These tests are intended to mimic the output of such a
compiler, so I'll continue to refer to this range as "cold" in the
following discussion.
The original test case had only tested a cold range placed
at lower addresses than the rest of the function. During testing of the
new code where the cold range was placed at higher addresses, I found
that I could produce the following backtrace:
(gdb) bt
#0 0x0000000000401138 in baz ()
at dw2-ranges-func-hi-cold.c:72
#1 0x0000000000401131 in foo_cold ()
at dw2-ranges-func-hi-cold.c:64
#2 0x000000000040111e in foo ()
at dw2-ranges-func-hi-cold.c:50
#3 0x0000000000401144 in main ()
at dw2-ranges-func-hi-cold.c:78
This is correct, except that we'd like to see foo() listed instead
of foo_cold(). (I handle that problem in another patch.)
Now look at what happens for a similar backtrace where the cold range
is at a lower address than the foo's entry pc:
(gdb) bt
#0 0x000000000040110a in baz ()
at dw2-ranges-func-lo-cold.c:48
#1 0x0000000000401116 in foo ()
at dw2-ranges-func-lo-cold.c:54
#2 0x00007fffffffd4c0 in ?? ()
#3 0x0000000000401138 in foo ()
at dw2-ranges-func-lo-cold.c:70
Note that the backtrace doesn't go all the way back to main(). Moreover,
frame #2 is messed up.
I had seen this behavior when I had worked on the non-contiguous
address problem last year. At the time I convinced myself that the
mangled backtrace was "okay" since we're doing strange things with
the DWARF assembler. We're taking a function called foo_cold (though
it was originally called foo_low - my recent changes to the test case
changed the name) and via the magic of the DWARF assembler, we're
combining it into a separate (non-contiguous) range for foo. Thus,
it was a surprise to me when I got a good and complete backtrace when
the cold symbol is placed at an address that's greater than entry pc.
The function dwarf2_frame_cache (in dwarf2-frame.c) is making this
call:
if (get_frame_func_if_available (this_frame, &entry_pc)) ...
If that call succeeds (returns a true value), the FDE is then
processed up to the entry pc. It doesn't make sense to do this,
however, when the FDE in question does not contain the entry pc. This
can happen when the function in question is comprised of more than one
(non-contiguous) address range.
My fix is to add some comparisons to the test above to ensure that
ENTRY_PC is within the address range covered by the FDE.
gdb/ChangeLog:
* dwarf2-frame.c (dwarf2_frame_cache): Don't decode FDE instructions
for entry pc when entry pc is out of range for that FDE.
---
gdb/dwarf2-frame.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index 0f2502b4c6..c98e2f232f 100644
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -1023,7 +1023,13 @@ dwarf2_frame_cache (struct frame_info *this_frame, void **this_cache)
/* Save the initialized register set. */
fs.initial = fs.regs;
- if (get_frame_func_if_available (this_frame, &entry_pc))
+ /* Fetching the entry pc for THIS_FRAME won't necessarily result
+ in an address that's within the range of FDE locations. This
+ is due to the possibility of the function occupying non-contiguous
+ ranges. */
+ if (get_frame_func_if_available (this_frame, &entry_pc)
+ && fde->initial_location <= entry_pc
+ && entry_pc < fde->initial_location + fde->address_range)
{
/* Decode the insns in the FDE up to the entry PC. */
instr = execute_cfa_program (fde, fde->instructions, fde->end, gdbarch,
--
2.21.0
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH v2 1/5] Prefer symtab symbol over minsym for function names in non-contiguous blocks
2019-07-04 4:55 [PATCH v2 0/5] Non-contiguous address range bug fixes / improvements Kevin Buettner
2019-07-04 4:55 ` [PATCH v2 3/5] dwarf2-frame.c: Fix FDE processing bug involving non-contiguous ranges Kevin Buettner
@ 2019-07-04 4:55 ` Kevin Buettner
2019-07-04 5:03 ` Kevin Buettner
2019-07-04 4:55 ` [PATCH v2 2/5] Restrict use of minsym names when printing addresses in disassembled code Kevin Buettner
` (3 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Kevin Buettner @ 2019-07-04 4:55 UTC (permalink / raw)
To: gdb-patches; +Cc: Kevin Buettner
The discussion on gdb-patches which led to this patch may be found
here:
https://www.sourceware.org/ml/gdb-patches/2019-05/msg00018.html
Here's a brief synopsis/analysis:
Eli Zaretskii, while debugging a Windows emacs executable, found
that functions comprised of more than one (non-contiguous)
address range were not being displayed correctly in a backtrace. This
is the example that Eli provided:
(gdb) bt
#0 0x76a63227 in KERNELBASE!DebugBreak ()
from C:\Windows\syswow64\KernelBase.dll
#1 0x012e7b89 in emacs_abort () at w32fns.c:10768
#2 0x012e1f3b in print_vectorlike.cold () at print.c:1824
#3 0x011d2dec in print_object (obj=<optimized out>, printcharfun=XIL(0),
escapeflag=true) at print.c:2150
The function print_vectorlike consists of two address ranges, one of
which contains "cold" code which is expected to not execute very often.
There is a minimal symbol, print_vectorlike.cold.65, which is the address
of the "cold" range.
GDB is prefering this minsym over the the name provided by the
DWARF info due to some really old code in GDB which handles
"certain pathological cases". This comment reads as follows:
/* In certain pathological cases, the symtabs give the wrong
function (when we are in the first function in a file which
is compiled without debugging symbols, the previous function
is compiled with debugging symbols, and the "foo.o" symbol
that is supposed to tell us where the file with debugging
symbols ends has been truncated by ar because it is longer
than 15 characters). This also occurs if the user uses asm()
to create a function but not stabs for it (in a file compiled
with -g).
So look in the minimal symbol tables as well, and if it comes
up with a larger address for the function use that instead.
I don't think this can ever cause any problems; there
shouldn't be any minimal symbols in the middle of a function;
if this is ever changed many parts of GDB will need to be
changed (and we'll create a find_pc_minimal_function or some
such). */
In an earlier version of this patch, I had left the code for the
pathological case intact, but those who reviwed that patch recommended
removing it. So that's what I've done - I've removed it.
gdb/ChangeLog:
* stack.c (find_frame_funname): Remove code which preferred
minsym over symtab sym in "certain pathological cases".
---
gdb/stack.c | 71 +++++++++++------------------------------------------
1 file changed, 15 insertions(+), 56 deletions(-)
diff --git a/gdb/stack.c b/gdb/stack.c
index b3d113d3b4..e0a7403ec0 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1135,66 +1135,25 @@ find_frame_funname (struct frame_info *frame, enum language *funlang,
func = get_frame_function (frame);
if (func)
{
- /* In certain pathological cases, the symtabs give the wrong
- function (when we are in the first function in a file which
- is compiled without debugging symbols, the previous function
- is compiled with debugging symbols, and the "foo.o" symbol
- that is supposed to tell us where the file with debugging
- symbols ends has been truncated by ar because it is longer
- than 15 characters). This also occurs if the user uses asm()
- to create a function but not stabs for it (in a file compiled
- with -g).
-
- So look in the minimal symbol tables as well, and if it comes
- up with a larger address for the function use that instead.
- I don't think this can ever cause any problems; there
- shouldn't be any minimal symbols in the middle of a function;
- if this is ever changed many parts of GDB will need to be
- changed (and we'll create a find_pc_minimal_function or some
- such). */
+ const char *print_name = SYMBOL_PRINT_NAME (func);
- struct bound_minimal_symbol msymbol;
-
- /* Don't attempt to do this for inlined functions, which do not
- have a corresponding minimal symbol. */
- if (!block_inlined_p (SYMBOL_BLOCK_VALUE (func)))
- msymbol
- = lookup_minimal_symbol_by_pc (get_frame_address_in_block (frame));
- else
- memset (&msymbol, 0, sizeof (msymbol));
-
- if (msymbol.minsym != NULL
- && (BMSYMBOL_VALUE_ADDRESS (msymbol)
- > BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (func))))
+ *funlang = SYMBOL_LANGUAGE (func);
+ if (funcp)
+ *funcp = func;
+ if (*funlang == language_cplus)
{
- /* We also don't know anything about the function besides
- its address and name. */
- func = 0;
- funname.reset (xstrdup (MSYMBOL_PRINT_NAME (msymbol.minsym)));
- *funlang = MSYMBOL_LANGUAGE (msymbol.minsym);
+ /* It seems appropriate to use SYMBOL_PRINT_NAME() here,
+ to display the demangled name that we already have
+ stored in the symbol table, but we stored a version
+ with DMGL_PARAMS turned on, and here we don't want to
+ display parameters. So remove the parameters. */
+ funname = cp_remove_params (print_name);
}
- else
- {
- const char *print_name = SYMBOL_PRINT_NAME (func);
- *funlang = SYMBOL_LANGUAGE (func);
- if (funcp)
- *funcp = func;
- if (*funlang == language_cplus)
- {
- /* It seems appropriate to use SYMBOL_PRINT_NAME() here,
- to display the demangled name that we already have
- stored in the symbol table, but we stored a version
- with DMGL_PARAMS turned on, and here we don't want to
- display parameters. So remove the parameters. */
- funname = cp_remove_params (print_name);
- }
-
- /* If we didn't hit the C++ case above, set *funname
- here. */
- if (funname == NULL)
- funname.reset (xstrdup (print_name));
- }
+ /* If we didn't hit the C++ case above, set *funname
+ here. */
+ if (funname == NULL)
+ funname.reset (xstrdup (print_name));
}
else
{
--
2.21.0
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2 1/5] Prefer symtab symbol over minsym for function names in non-contiguous blocks
2019-07-04 4:55 ` [PATCH v2 1/5] Prefer symtab symbol over minsym for function names in non-contiguous blocks Kevin Buettner
@ 2019-07-04 5:03 ` Kevin Buettner
0 siblings, 0 replies; 11+ messages in thread
From: Kevin Buettner @ 2019-07-04 5:03 UTC (permalink / raw)
To: gdb-patches
I find the diff easier to read when the -w switch is used. So here's
just the patch using -w:
diff --git a/gdb/stack.c b/gdb/stack.c
index b3d113d3b4..e0a7403ec0 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1134,46 +1134,6 @@ find_frame_funname (struct frame_info *frame, enum language *funlang,
func = get_frame_function (frame);
if (func)
- {
- /* In certain pathological cases, the symtabs give the wrong
- function (when we are in the first function in a file which
- is compiled without debugging symbols, the previous function
- is compiled with debugging symbols, and the "foo.o" symbol
- that is supposed to tell us where the file with debugging
- symbols ends has been truncated by ar because it is longer
- than 15 characters). This also occurs if the user uses asm()
- to create a function but not stabs for it (in a file compiled
- with -g).
-
- So look in the minimal symbol tables as well, and if it comes
- up with a larger address for the function use that instead.
- I don't think this can ever cause any problems; there
- shouldn't be any minimal symbols in the middle of a function;
- if this is ever changed many parts of GDB will need to be
- changed (and we'll create a find_pc_minimal_function or some
- such). */
-
- struct bound_minimal_symbol msymbol;
-
- /* Don't attempt to do this for inlined functions, which do not
- have a corresponding minimal symbol. */
- if (!block_inlined_p (SYMBOL_BLOCK_VALUE (func)))
- msymbol
- = lookup_minimal_symbol_by_pc (get_frame_address_in_block (frame));
- else
- memset (&msymbol, 0, sizeof (msymbol));
-
- if (msymbol.minsym != NULL
- && (BMSYMBOL_VALUE_ADDRESS (msymbol)
- > BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (func))))
- {
- /* We also don't know anything about the function besides
- its address and name. */
- func = 0;
- funname.reset (xstrdup (MSYMBOL_PRINT_NAME (msymbol.minsym)));
- *funlang = MSYMBOL_LANGUAGE (msymbol.minsym);
- }
- else
{
const char *print_name = SYMBOL_PRINT_NAME (func);
@@ -1195,7 +1155,6 @@ find_frame_funname (struct frame_info *frame, enum language *funlang,
if (funname == NULL)
funname.reset (xstrdup (print_name));
}
- }
else
{
struct bound_minimal_symbol msymbol;
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 2/5] Restrict use of minsym names when printing addresses in disassembled code
2019-07-04 4:55 [PATCH v2 0/5] Non-contiguous address range bug fixes / improvements Kevin Buettner
2019-07-04 4:55 ` [PATCH v2 3/5] dwarf2-frame.c: Fix FDE processing bug involving non-contiguous ranges Kevin Buettner
2019-07-04 4:55 ` [PATCH v2 1/5] Prefer symtab symbol over minsym for function names in non-contiguous blocks Kevin Buettner
@ 2019-07-04 4:55 ` Kevin Buettner
2019-07-19 17:07 ` Pedro Alves
2019-07-04 5:05 ` [PATCH v2 5/5] Improve test gdb.dwarf2/dw2-ranges-func.exp Kevin Buettner
` (2 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Kevin Buettner @ 2019-07-04 4:55 UTC (permalink / raw)
To: gdb-patches; +Cc: Kevin Buettner
build_address_symbolic contains some code which causes it to
prefer the minsym over the the function symbol in certain cases.
The cases where this occurs are the same as the "certain pathological
cases" that used to exist in find_frame_funname().
This commit largely disables that code; it will only prefer the
minsym when the address of minsym is identical to that of the address
under consideration AND the function address for the symbtab sym is
not the same as the address under consideration.
So, without this change, when using the dw2-ranges-func-lo-cold
executable from the gdb.dwarf2/dw2-ranges-func.exp test, GDB exhibits
the following behavior:
(gdb) x/5i foo_cold
0x40110d <foo+4294967277>: push %rbp
0x40110e <foo+4294967278>: mov %rsp,%rbp
0x401111 <foo+4294967281>: callq 0x401106 <baz>
0x401116 <foo+4294967286>: nop
0x401117 <foo+4294967287>: pop %rbp
On the other hand, still without this change, using the
dw2-ranges-func-hi-cold executable from the same test, GDB
does this instead:
(gdb) x/5i foo_cold
0x401128 <foo_cold>: push %rbp
0x401129 <foo_cold+1>: mov %rsp,%rbp
0x40112c <foo_cold+4>: callq 0x401134 <baz>
0x401131 <foo_cold+9>: nop
0x401132 <foo_cold+10>: pop %rbp
This is inconsistent behavior. When foo_cold is at a lower
address than the function's entry point, the symtab symbol (foo)
is displayed along with a large positive offset which would wrap
around the address space if the address space were only 32 bits wide.
(A later patch fixes this problem by displaying negative offsets.)
This commit makes the behavior uniform for both the "lo-cold" and
"hi-cold" cases:
lo-cold:
(gdb) x/5i foo_cold
0x40110d <foo_cold>: push %rbp
0x40110e <foo-18>: mov %rsp,%rbp
0x401111 <foo-15>: callq 0x401106 <baz>
0x401116 <foo-10>: nop
0x401117 <foo-9>: pop %rbp
hi-cold:
(gdb) x/5i foo_cold
0x401128 <foo_cold>: push %rbp
0x401129 <foo+35>: mov %rsp,%rbp
0x40112c <foo+38>: callq 0x401134 <baz>
0x401131 <foo+43>: nop
0x401132 <foo+44>: pop %rbp
In both cases, the symbol shown for the address at which foo_cold
resides is shown as <foo_cold>. Subsequent offsets are shown as
either negative or positive offsets from the entry pc for foo.
When disassembling a function, care must be taken to NOT display
<+0> as the offset for the second range. For this reason, I found
it necessary to add the "prefer_sym_over_minsym" parameter to
build_address_symbolic. The type of this flag is a bool; do_demangle
ought to be a bool also, so I made this change at the same time.
gdb/ChangeLog:
* valprint.h (build_address_symbolic): Add "prefer_sym_over_minsym"
parameter. Change type of "do_demangle" to bool.
* disasm.c (gdb_pretty_print_disassembler::pretty_print_insn):
Pass suitable "prefer_sym_over_minsym" flag to
build_address_symbolic(). Don't output "+" for negative offsets.
* printcmd.c (print_address_symbolic): Update invocation of
build_address_symbolic to include a "prefer_sym_over_minsym"
flag.
(build_address_symbolic): Add "prefer_sym_over_minsym" parameter.
Restrict cases in which use of minimal symbol is preferred to that
of a found symbol. Update comments.
---
gdb/disasm.c | 12 ++++++++----
gdb/printcmd.c | 29 +++++++++++++++++++++--------
gdb/valprint.h | 13 +++++++++----
3 files changed, 38 insertions(+), 16 deletions(-)
diff --git a/gdb/disasm.c b/gdb/disasm.c
index 4e58cb67f9..30c3b06936 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -237,16 +237,20 @@ gdb_pretty_print_disassembler::pretty_print_insn (struct ui_out *uiout,
uiout->field_core_addr ("address", gdbarch, pc);
std::string name, filename;
- if (!build_address_symbolic (gdbarch, pc, 0, &name, &offset, &filename,
- &line, &unmapped))
+ bool omit_fname = ((flags & DISASSEMBLY_OMIT_FNAME) != 0);
+ if (!build_address_symbolic (gdbarch, pc, false, omit_fname, &name,
+ &offset, &filename, &line, &unmapped))
{
/* We don't care now about line, filename and unmapped. But we might in
the future. */
uiout->text (" <");
- if ((flags & DISASSEMBLY_OMIT_FNAME) == 0)
+ if (!omit_fname)
uiout->field_string ("func-name", name.c_str (),
ui_out_style_kind::FUNCTION);
- uiout->text ("+");
+ /* For negative offsets, avoid displaying them as +-N; the sign of
+ the offset takes the place of the "+" here. */
+ if (offset >= 0)
+ uiout->text ("+");
uiout->field_int ("offset", offset);
uiout->text (">:\t");
}
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 0509360581..1109cb3046 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -528,8 +528,8 @@ print_address_symbolic (struct gdbarch *gdbarch, CORE_ADDR addr,
int offset = 0;
int line = 0;
- if (build_address_symbolic (gdbarch, addr, do_demangle, &name, &offset,
- &filename, &line, &unmapped))
+ if (build_address_symbolic (gdbarch, addr, do_demangle, false, &name,
+ &offset, &filename, &line, &unmapped))
return 0;
fputs_filtered (leadin, stream);
@@ -563,7 +563,8 @@ print_address_symbolic (struct gdbarch *gdbarch, CORE_ADDR addr,
int
build_address_symbolic (struct gdbarch *gdbarch,
CORE_ADDR addr, /* IN */
- int do_demangle, /* IN */
+ bool do_demangle, /* IN */
+ bool prefer_sym_over_minsym, /* IN */
std::string *name, /* OUT */
int *offset, /* OUT */
std::string *filename, /* OUT */
@@ -591,8 +592,10 @@ build_address_symbolic (struct gdbarch *gdbarch,
}
}
- /* First try to find the address in the symbol table, then
- in the minsyms. Take the closest one. */
+ /* Try to find the address in both the symbol table and the minsyms.
+ In most cases, we'll prefer to use the symbol instead of the
+ minsym. However, there are cases (see below) where we'll choose
+ to use the minsym instead. */
/* This is defective in the sense that it only finds text symbols. So
really this is kind of pointless--we should make sure that the
@@ -629,7 +632,19 @@ build_address_symbolic (struct gdbarch *gdbarch,
if (msymbol.minsym != NULL)
{
- if (BMSYMBOL_VALUE_ADDRESS (msymbol) > name_location || symbol == NULL)
+ /* Use the minsym if no symbol is found.
+
+ Additionally, use the minsym instead of a (found) symbol if
+ the following conditions all hold:
+ 1) The prefer_sym_over_minsym flag is false.
+ 2) The minsym address is identical to that of the address under
+ consideration.
+ 3) The symbol address is not identical to that of the address
+ under consideration. */
+ if (symbol == NULL ||
+ (!prefer_sym_over_minsym
+ && BMSYMBOL_VALUE_ADDRESS (msymbol) == addr
+ && name_location != addr))
{
/* If this is a function (i.e. a code address), strip out any
non-address bits. For instance, display a pointer to the
@@ -642,8 +657,6 @@ build_address_symbolic (struct gdbarch *gdbarch,
|| MSYMBOL_TYPE (msymbol.minsym) == mst_solib_trampoline)
addr = gdbarch_addr_bits_remove (gdbarch, addr);
- /* The msymbol is closer to the address than the symbol;
- use the msymbol instead. */
symbol = 0;
name_location = BMSYMBOL_VALUE_ADDRESS (msymbol);
if (do_demangle || asm_demangle)
diff --git a/gdb/valprint.h b/gdb/valprint.h
index 987c534eaf..07014c11b9 100644
--- a/gdb/valprint.h
+++ b/gdb/valprint.h
@@ -255,13 +255,18 @@ extern void print_command_completer (struct cmd_list_element *ignore,
/* Given an address ADDR return all the elements needed to print the
address in a symbolic form. NAME can be mangled or not depending
on DO_DEMANGLE (and also on the asm_demangle global variable,
- manipulated via ''set print asm-demangle''). Return 0 in case of
- success, when all the info in the OUT paramters is valid. Return 1
- otherwise. */
+ manipulated via ''set print asm-demangle''). When
+ PREFER_SYM_OVER_MINSYM is true, names (and offsets) from minimal
+ symbols won't be used except in instances where no symbol was
+ found; otherwise, a minsym might be used in some instances (mostly
+ involving function with non-contiguous address ranges). Return
+ 0 in case of success, when all the info in the OUT paramters is
+ valid. Return 1 otherwise. */
extern int build_address_symbolic (struct gdbarch *,
CORE_ADDR addr,
- int do_demangle,
+ bool do_demangle,
+ bool prefer_sym_over_minsym,
std::string *name,
int *offset,
std::string *filename,
--
2.21.0
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2 2/5] Restrict use of minsym names when printing addresses in disassembled code
2019-07-04 4:55 ` [PATCH v2 2/5] Restrict use of minsym names when printing addresses in disassembled code Kevin Buettner
@ 2019-07-19 17:07 ` Pedro Alves
0 siblings, 0 replies; 11+ messages in thread
From: Pedro Alves @ 2019-07-19 17:07 UTC (permalink / raw)
To: Kevin Buettner, gdb-patches
Below, more of a brain dump than an objection. FWIW.
On 7/4/19 5:55 AM, Kevin Buettner wrote:
>
> (gdb) x/5i foo_cold
> 0x401128 <foo_cold>: push %rbp
> 0x401129 <foo+35>: mov %rsp,%rbp
> 0x40112c <foo+38>: callq 0x401134 <baz>
> 0x401131 <foo+43>: nop
> 0x401132 <foo+44>: pop %rbp
I admit that it took me a while to reply because I'm still finding
it a bit hard to convince myself that that is the ideal output.
E.g., the first two instructions are obviously part of the same
prologue, I find it a bit odd that the disassembly shows
different function names for that instruction pair.
Maybe this won't matter in practice since IIUC your testcase is a
bit contrived, and real cold functions are just fragments of
functions jmp/ret'ed to/from, without a prologue.
BTW, I noticed (with your series applied), this divergence:
(gdb) x /5i foo_cold
0x40048e <foo_cold>: push %rbp
0x40048f <foo-18>: mov %rsp,%rbp
0x400492 <foo-15>: callq 0x400487 <baz>
0x400497 <foo-10>: nop
0x400498 <foo-9>: pop %rbp
vs:
(gdb) info symbol 0x40048f
foo_cold + 1 in section .text
(gdb) info symbol 0x400492
foo_cold + 4 in section .text
(gdb) info symbol
Argument required (address).
(gdb) info symbol 0x400497
foo_cold + 9 in section .text
That's of course because "info symbol" only looks at minimal symbols.
On the disassemble side, I think I'd be less confused with:
(gdb) disassemble foo
Dump of assembler code for function foo:
Address range 0x4004a1 to 0x4004bc:
0x00000000004004a1 <foo+0>: push %rbp
0x00000000004004a2 <foo+1>: mov %rsp,%rbp
0x00000000004004a5 <foo+4>: callq 0x40049a <bar>
0x00000000004004aa <foo+9>: mov 0x200b70(%rip),%eax # 0x601020 <e>
0x00000000004004b0 <foo+15>: test %eax,%eax
0x00000000004004b2 <foo+17>: je 0x4004b9 <foo+24>
0x00000000004004b4 <foo+19>: callq 0x40048e <foo_cold>
0x00000000004004b9 <foo+24>: nop
0x00000000004004ba <foo+25>: pop %rbp
0x00000000004004bb <foo+26>: retq
Address range 0x40048e to 0x40049a:
0x000000000040048e <foo.cold+0>: push %rbp
0x000000000040048f <foo.cold+1>: mov %rsp,%rbp
0x0000000000400492 <foo.cold+4>: callq 0x400487 <baz>
0x0000000000400497 <foo.cold+9>: nop
0x0000000000400498 <foo.cold+10>: pop %rbp
0x0000000000400499 <foo.cold+11>: retq
End of assembler dump.
Instead of:
(gdb) disassemble foo
Dump of assembler code for function foo:
Address range 0x4004a1 to 0x4004bc:
0x00000000004004a1 <+0>: push %rbp
0x00000000004004a2 <+1>: mov %rsp,%rbp
0x00000000004004a5 <+4>: callq 0x40049a <bar>
0x00000000004004aa <+9>: mov 0x200b70(%rip),%eax # 0x601020 <e>
0x00000000004004b0 <+15>: test %eax,%eax
0x00000000004004b2 <+17>: je 0x4004b9 <foo+24>
0x00000000004004b4 <+19>: callq 0x40048e <foo_cold>
0x00000000004004b9 <+24>: nop
0x00000000004004ba <+25>: pop %rbp
0x00000000004004bb <+26>: retq
Address range 0x40048e to 0x40049a:
0x000000000040048e <-19>: push %rbp
0x000000000040048f <-18>: mov %rsp,%rbp
0x0000000000400492 <-15>: callq 0x400487 <baz>
0x0000000000400497 <-10>: nop
0x0000000000400498 <-9>: pop %rbp
0x0000000000400499 <-8>: retq
End of assembler dump.
In your examples / testcase, the cold function is adjacent/near
the hot / main entry point of foo. But I think that
on a real cold function, the offsets between the cold and
hot entry points can potentially be much larger (e.g., place in
different elf sections), as the point is exactly to
move cold code away from the hot path / cache.
That that would mean that we're likely to see output like:
(gdb) disassemble foo
Dump of assembler code for function foo:
Address range 0x6004a1 to 0x6004bc:
0x00000000006004a1 <+0>: push %rbp
0x00000000006004a2 <+1>: mov %rsp,%rbp
0x00000000006004a5 <+4>: callq 0x40049a <bar>
0x00000000006004aa <+9>: mov 0x200b70(%rip),%eax # 0x601020 <e>
0x00000000006004b0 <+15>: test %eax,%eax
0x00000000006004b2 <+17>: je 0x6004b9 <foo+24>
0x00000000006004b4 <+19>: callq 0x40048e <foo_cold>
0x00000000006004b9 <+24>: nop
0x00000000006004ba <+25>: pop %rbp
0x00000000006004bb <+26>: retq
Address range 0x40048e to 0x40049a:
0x000000000040048e <-2097171>: push %rbp
0x000000000040048f <-2097170>: mov %rsp,%rbp
0x0000000000400492 <-2097167>: callq 0x400487 <baz>
0x0000000000400497 <-2097162>: nop
0x0000000000400498 <-2097161>: pop %rbp
0x0000000000400499 <-2097160>: retq
End of assembler dump.
... and those negative offsets kind of look a bit odd.
But maybe it's just that I'm not thinking it right.
If I think of the offset in terms of offset from the
"foo"'s entry point, which is what it really is, then I can
convince myself that I can explain why that's the right output,
pedantically.
So I guess I'll grow into it.
And with that out of the way, the series looks good to me as is.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 5/5] Improve test gdb.dwarf2/dw2-ranges-func.exp
2019-07-04 4:55 [PATCH v2 0/5] Non-contiguous address range bug fixes / improvements Kevin Buettner
` (2 preceding siblings ...)
2019-07-04 4:55 ` [PATCH v2 2/5] Restrict use of minsym names when printing addresses in disassembled code Kevin Buettner
@ 2019-07-04 5:05 ` Kevin Buettner
2019-07-04 5:08 ` Kevin Buettner
2019-07-04 5:05 ` [PATCH v2 4/5] Allow display of negative offsets in print_address_symbolic() Kevin Buettner
2019-07-27 20:48 ` [PATCH v2 0/5] Non-contiguous address range bug fixes / improvements Kevin Buettner
5 siblings, 1 reply; 11+ messages in thread
From: Kevin Buettner @ 2019-07-04 5:05 UTC (permalink / raw)
To: gdb-patches; +Cc: Kevin Buettner
The original dw2-ranges-func.exp test caused a function named foo to be
created with two non-contiguous address ranges. In the C source file,
a function named foo_low was incorporated into the function foo which
was also defined in that file. The DWARF assembler is used to do this
manipulation. The source file had been laid out so that foo_low would
likely be placed (by the compiler and linker) at a lower address than
foo().
The case where a range at a higher set of addresses (than foo) was not
being tested. In a recent discussion on gdb-patches, it became clear
that performing such tests are desirable because bugs were discovered
which only became evident when the other range was located at high(er)
addresses than the range containing the entry point for the function.
This other (non entry pc) address range is typically used for "cold"
code which executes less frequently. Thus, I renamed foo_low to
foo_cold and renamed the C source file from dw-ranges-func.c to
dw-ranges-func-lo.c. I then made a copy of this file, naming it
dw-ranges-func-hi.c. (That was my intent anyway. According to git,
I renamed dw-ranges-func.c to dw-ranges-func-hi.c and then modified it.
dw-ranges-func-lo.c shows up as an entirely new file.)
Within dw-ranges-func-hi.c, I changed the placement of foo_cold()
along with some of the other functions so that foo_cold() would be at
a higher address than foo() while also remaining non-contiguous. The
two files, dw-ranges-func-lo.c and dw-ranges-func-hi.c, are
essentially the same except for the placement of some of the functions
therein.
The tests in dw2-ranges-func.exp where then wrapped in a new proc named
do_test which was then called in a loop from the outermost level. The
loop causes each of the source files to have the same tests run upon
them.
I also added a few new tests which test functionality fixed by the other
commits to this patch series. Due to the reorganization of the file,
it's hard to identify these changes in the patch. So, here are the
tests which were added:
with_test_prefix "no-cold-names" {
# Due to the calling sequence, this backtrace would normally
# show function foo_cold for frame #1. However, we don't want
# this to be the case due to placing it in the same block
# (albeit at a different range) as foo. Thus it is correct to
# see foo for frames #1 and #2. It is incorrect to see
# foo_cold at frame #1.
gdb_test_sequence "bt" "backtrace from baz" {
"\[\r\n\]#0 .*? baz \\(\\) "
"\[\r\n\]#1 .*? foo \\(\\) "
"\[\r\n\]#2 .*? foo \\(\\) "
"\[\r\n\]#3 .*? main \\(\\) "
}
# Doing x/2i foo_cold should show foo_cold as the first symbolic
# address and an offset from foo for the second. We also check to
# make sure that the offset is not too large - we don't GDB to
# display really large offsets that would (try to) wrap around the
# address space.
set foo_cold_offset 0
set test "x/2i foo_cold"
gdb_test_multiple $test $test {
-re " (?:$hex) <foo_cold>.*?\n (?:$hex) <foo\[+-\](\[0-9\]+)>.*${gdb_prompt}" {
set foo_cold_offset $expect_out(1,string)
pass $test
}
}
gdb_assert {$foo_cold_offset <= 10000} "offset to foo_cold is not too large"
# Likewise, verify that second address shown by "info line" is at
# and offset from foo instead of foo_cold.
gdb_test "info line *foo_cold" "starts at address $hex <foo_cold> and ends at $hex <foo\[+-\].*?>.*"
}
When run against a GDB without the requisite bug fixes (from this patch
series), these 6 failures should be seen:
FAIL: gdb.dwarf2/dw2-ranges-func.exp: lo-cold: no-cold-names: backtrace from baz (pattern 4)
FAIL: gdb.dwarf2/dw2-ranges-func.exp: lo-cold: no-cold-names: x/2i foo_cold
FAIL: gdb.dwarf2/dw2-ranges-func.exp: lo-cold: no-cold-names: info line *foo_cold
FAIL: gdb.dwarf2/dw2-ranges-func.exp: hi-cold: no-cold-names: backtrace from baz (pattern 3)
FAIL: gdb.dwarf2/dw2-ranges-func.exp: hi-cold: no-cold-names: x/2i foo_cold
FAIL: gdb.dwarf2/dw2-ranges-func.exp: hi-cold: no-cold-names: info line *foo_cold
gdb/testsuite/ChangeLog:
* gdb.dwarf2/dw2-ranges-func.c: Rename to...
* gdb.dwarf2/dw2-ranges-func-lo-cold.c: ...this.
* gdb.dwarf2/dw2-ranges-func-lo-cold.c (foo_low): Change name to
foo_cold. Revise comments to match.
* gdb.dwarf2/dw2-ranges-func-hi-cold.c: New file.
* gdb.dwarf2/dw2-ranges-func.exp (do_test): New proc. Existing tests
were wrapped into this proc; Call do_test in loop from outermost
level.
(foo_low): Rename all occurrences to "foo_cold".
(backtrace from baz): New test.
(x2/i foo_cold): New test.
(info line *foo_cold): New test.
---
...anges-func.c => dw2-ranges-func-hi-cold.c} | 44 +-
.../gdb.dwarf2/dw2-ranges-func-lo-cold.c | 82 +++
gdb/testsuite/gdb.dwarf2/dw2-ranges-func.exp | 693 ++++++++++--------
3 files changed, 480 insertions(+), 339 deletions(-)
rename gdb/testsuite/gdb.dwarf2/{dw2-ranges-func.c => dw2-ranges-func-hi-cold.c} (85%)
create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-ranges-func-lo-cold.c
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-ranges-func.c b/gdb/testsuite/gdb.dwarf2/dw2-ranges-func-hi-cold.c
similarity index 85%
rename from gdb/testsuite/gdb.dwarf2/dw2-ranges-func.c
rename to gdb/testsuite/gdb.dwarf2/dw2-ranges-func-hi-cold.c
index fa1b7bab41..06c3a3485a 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-ranges-func.c
+++ b/gdb/testsuite/gdb.dwarf2/dw2-ranges-func-hi-cold.c
@@ -16,7 +16,7 @@
/* The idea here is to, via use of the dwarf assembler, create a function
which occupies two non-contiguous address ranges.
- foo_low and foo will be combined into a single function foo with a
+ foo_cold and foo will be combined into a single function foo with a
function bar in between these two ranges.
This test case was motivated by a bug in which a function which
@@ -37,19 +37,19 @@
volatile int e = 0;
-void
-baz (void)
-{
- asm ("baz_label: .globl baz_label");
-} /* baz end */
+void bar (void);
+void foo_cold (void);
+void baz (void);
void
-foo_low (void)
-{ /* foo_low prologue */
- asm ("foo_low_label: .globl foo_low_label");
- baz (); /* foo_low baz call */
- asm ("foo_low_label2: .globl foo_low_label2");
-} /* foo_low end */
+foo (void)
+{ /* foo prologue */
+ asm ("foo_label: .globl foo_label");
+ bar (); /* foo bar call */
+ asm ("foo_label2: .globl foo_label2");
+ if (e) foo_cold (); /* foo foo_cold call */
+ asm ("foo_label3: .globl foo_label3");
+} /* foo end */
void
bar (void)
@@ -58,14 +58,18 @@ bar (void)
} /* bar end */
void
-foo (void)
-{ /* foo prologue */
- asm ("foo_label: .globl foo_label");
- bar (); /* foo bar call */
- asm ("foo_label2: .globl foo_label2");
- if (e) foo_low (); /* foo foo_low call */
- asm ("foo_label3: .globl foo_label3");
-} /* foo end */
+foo_cold (void)
+{ /* foo_cold prologue */
+ asm ("foo_cold_label: .globl foo_cold_label");
+ baz (); /* foo_cold baz call */
+ asm ("foo_cold_label2: .globl foo_cold_label2");
+} /* foo_cold end */
+
+void
+baz (void)
+{
+ asm ("baz_label: .globl baz_label");
+} /* baz end */
int
main (void)
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-ranges-func-lo-cold.c b/gdb/testsuite/gdb.dwarf2/dw2-ranges-func-lo-cold.c
new file mode 100644
index 0000000000..40966ce6dc
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-ranges-func-lo-cold.c
@@ -0,0 +1,82 @@
+/* Copyright 2018-2019 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/>. */
+
+/* The idea here is to, via use of the dwarf assembler, create a function
+ which occupies two non-contiguous address ranges.
+
+ foo_cold and foo will be combined into a single function foo with a
+ function bar in between these two ranges.
+
+ This test case was motivated by a bug in which a function which
+ occupied two non-contiguous address ranges was calling another
+ function which resides in between these ranges. So we end up with
+ a situation in which the low/start address of our constructed foo
+ (in this case) will be less than any of the addresses in bar, but
+ the high/end address of foo will be greater than any of bar's
+ addresses.
+
+ This situation was causing a problem in the caching code of
+ find_pc_partial_function: When the low and high addresses of foo
+ are placed in the cache, the simple check that was used to see if
+ the cache was applicable would (incorrectly) succeed when presented
+ with an address in bar. I.e. an address in bar presented as an
+ input to find_pc_partial_function could produce the answer "this
+ address belongs to foo". */
+
+volatile int e = 0;
+
+void bar (void);
+void foo_cold (void);
+void baz (void);
+
+void
+baz (void)
+{
+ asm ("baz_label: .globl baz_label");
+} /* baz end */
+
+void
+foo_cold (void)
+{ /* foo_cold prologue */
+ asm ("foo_cold_label: .globl foo_cold_label");
+ baz (); /* foo_cold baz call */
+ asm ("foo_cold_label2: .globl foo_cold_label2");
+} /* foo_cold end */
+
+void
+bar (void)
+{
+ asm ("bar_label: .globl bar_label");
+} /* bar end */
+
+void
+foo (void)
+{ /* foo prologue */
+ asm ("foo_label: .globl foo_label");
+ bar (); /* foo bar call */
+ asm ("foo_label2: .globl foo_label2");
+ if (e) foo_cold (); /* foo foo_cold call */
+ asm ("foo_label3: .globl foo_label3");
+} /* foo end */
+
+int
+main (void)
+{ /* main prologue */
+ asm ("main_label: .globl main_label");
+ foo (); /* main foo call */
+ asm ("main_label2: .globl main_label2");
+ return 0; /* main return */
+} /* main end */
+
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-ranges-func.exp b/gdb/testsuite/gdb.dwarf2/dw2-ranges-func.exp
index ccf18b6983..fdc488ae92 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-ranges-func.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-ranges-func.exp
@@ -30,376 +30,431 @@ if !$gcc_compiled {
return 0
}
-standard_testfile dw2-ranges-func.c dw2-ranges-func-dw.S
-
-# We need to know the size of integer and address types in order to
-# write some of the debugging info we'd like to generate.
-#
-# For that, we ask GDB by debugging our test program. Any program
-# would do, but since we already have it specifically for this
-# testcase, might as well use that.
+proc do_test {suffix} {
+ global gdb_test_file_name
+ global testfile binfile srcfile srcfile2 gdb_prompt hex
+
+ # Don't use standard_testfile; we want different binaries for
+ # each suffix.
+ set testfile $gdb_test_file_name-$suffix
+ set binfile [standard_output_file ${testfile}]
+ set srcfile $testfile.c
+ set srcfile2 $testfile-dw2.S
+
+ # We need to know the size of integer and address types in order to
+ # write some of the debugging info we'd like to generate.
+ #
+ # For that, we ask GDB by debugging our test program. Any program
+ # would do, but since we already have it specifically for this
+ # testcase, might as well use that.
-if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
- return -1
-}
+ if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
+ return -1
+ }
-set asm_file [standard_output_file $srcfile2]
-Dwarf::assemble $asm_file {
- global srcdir subdir srcfile srcfile2
- declare_labels integer_label volatile_label func_ranges_label cu_ranges_label L
- set int_size [get_sizeof "int" 4]
-
- # Find start address and length for our functions.
- lassign [function_range main [list ${srcdir}/${subdir}/$srcfile]] \
- main_start main_len
- set main_end "$main_start + $main_len"
- lassign [function_range foo [list ${srcdir}/${subdir}/$srcfile]] \
- foo_start foo_len
- set foo_end "$foo_start + $foo_len"
- lassign [function_range foo_low [list ${srcdir}/${subdir}/$srcfile]] \
- foo_low_start foo_low_len
- set foo_low_end "$foo_low_start + $foo_low_len"
- lassign [function_range bar [list ${srcdir}/${subdir}/$srcfile]] \
- bar_start bar_len
- set bar_end "$bar_start + $bar_len"
- lassign [function_range baz [list ${srcdir}/${subdir}/$srcfile]] \
- baz_start baz_len
- set baz_end "$baz_start + $baz_len"
-
- set e_var [gdb_target_symbol e]
-
- cu {} {
- compile_unit {
- {language @DW_LANG_C}
- {name dw-ranges-func.c}
- {stmt_list $L DW_FORM_sec_offset}
- {low_pc 0 addr}
- {ranges ${cu_ranges_label} DW_FORM_sec_offset}
- } {
- integer_label: DW_TAG_base_type {
- {DW_AT_byte_size $int_size DW_FORM_sdata}
- {DW_AT_encoding @DW_ATE_signed}
- {DW_AT_name integer}
- }
- volatile_label: DW_TAG_volatile_type {
- {type :$integer_label}
- }
- DW_TAG_variable {
- {name e}
- {external 1 flag}
- {type :$volatile_label}
- {location {addr $e_var} SPECIAL_expr}
+ set asm_file [standard_output_file $srcfile2]
+ Dwarf::assemble $asm_file {
+ global srcdir subdir srcfile srcfile2
+ declare_labels integer_label volatile_label func_ranges_label cu_ranges_label L
+ set int_size [get_sizeof "int" 4]
+
+ # Find start address and length for our functions.
+ lassign [function_range main [list ${srcdir}/${subdir}/$srcfile]] \
+ main_start main_len
+ set main_end "$main_start + $main_len"
+ lassign [function_range foo [list ${srcdir}/${subdir}/$srcfile]] \
+ foo_start foo_len
+ set foo_end "$foo_start + $foo_len"
+ lassign [function_range foo_cold [list ${srcdir}/${subdir}/$srcfile]] \
+ foo_cold_start foo_cold_len
+ set foo_cold_end "$foo_cold_start + $foo_cold_len"
+ lassign [function_range bar [list ${srcdir}/${subdir}/$srcfile]] \
+ bar_start bar_len
+ set bar_end "$bar_start + $bar_len"
+ lassign [function_range baz [list ${srcdir}/${subdir}/$srcfile]] \
+ baz_start baz_len
+ set baz_end "$baz_start + $baz_len"
+
+ set e_var [gdb_target_symbol e]
+
+ cu {} {
+ compile_unit {
+ {language @DW_LANG_C}
+ {name dw-ranges-func2.c}
+ {stmt_list $L DW_FORM_sec_offset}
+ {low_pc 0 addr}
+ {ranges ${cu_ranges_label} DW_FORM_sec_offset}
+ } {
+ integer_label: DW_TAG_base_type {
+ {DW_AT_byte_size $int_size DW_FORM_sdata}
+ {DW_AT_encoding @DW_ATE_signed}
+ {DW_AT_name integer}
+ }
+ volatile_label: DW_TAG_volatile_type {
+ {type :$integer_label}
+ }
+ DW_TAG_variable {
+ {name e}
+ {external 1 flag}
+ {type :$volatile_label}
+ {location {addr $e_var} SPECIAL_expr}
+ }
+ subprogram {
+ {external 1 flag}
+ {name main}
+ {DW_AT_type :$integer_label}
+ {low_pc $main_start addr}
+ {high_pc $main_len DW_FORM_data4}
+ }
+ subprogram {
+ {external 1 flag}
+ {name foo}
+ {ranges ${func_ranges_label} DW_FORM_sec_offset}
+ }
+ subprogram {
+ {external 1 flag}
+ {name bar}
+ {low_pc $bar_start addr}
+ {high_pc $bar_len DW_FORM_data4}
+ }
+ subprogram {
+ {external 1 flag}
+ {name baz}
+ {low_pc $baz_start addr}
+ {high_pc $baz_len DW_FORM_data4}
+ }
}
- subprogram {
- {external 1 flag}
- {name main}
- {DW_AT_type :$integer_label}
- {low_pc $main_start addr}
- {high_pc $main_len DW_FORM_data4}
- }
- subprogram {
- {external 1 flag}
- {name foo}
- {ranges ${func_ranges_label} DW_FORM_sec_offset}
+ }
+
+ lines {version 2} L {
+ include_dir "${srcdir}/${subdir}"
+ file_name "$srcfile" 1
+
+ # Generate a line table program. An attempt was made to make it
+ # reasonably accurate as it made debugging the test case easier.
+ program {
+ {DW_LNE_set_address $main_start}
+ {DW_LNS_advance_line [expr [gdb_get_line_number "main prologue"] - 1]}
+ {DW_LNS_copy}
+ {DW_LNE_set_address main_label}
+ {DW_LNS_advance_line [expr [gdb_get_line_number "main foo call"] - [gdb_get_line_number "main prologue"]]}
+ {DW_LNS_copy}
+ {DW_LNE_set_address main_label2}
+ {DW_LNS_advance_line [expr [gdb_get_line_number "main return"] - [gdb_get_line_number "main foo call"]]}
+ {DW_LNS_copy}
+ {DW_LNE_set_address $main_end}
+ {DW_LNS_advance_line [expr [gdb_get_line_number "main end"] - [gdb_get_line_number "main return"] + 1]}
+ {DW_LNS_copy}
+ {DW_LNE_end_sequence}
+
+ {DW_LNE_set_address $foo_start}
+ {DW_LNS_advance_line [expr [gdb_get_line_number "foo prologue"] - 1] }
+ {DW_LNS_copy}
+ {DW_LNE_set_address foo_label}
+ {DW_LNS_advance_line [expr [gdb_get_line_number "foo bar call"] - [gdb_get_line_number "foo prologue"]]}
+ {DW_LNS_copy}
+ {DW_LNE_set_address foo_label2}
+ {DW_LNS_advance_line [expr [gdb_get_line_number "foo foo_cold call"] - [gdb_get_line_number "foo bar call"]]}
+ {DW_LNS_copy}
+ {DW_LNE_set_address foo_label3}
+ {DW_LNS_advance_line [expr [gdb_get_line_number "foo end"] - [gdb_get_line_number "foo foo_cold call"]]}
+ {DW_LNS_copy}
+ {DW_LNE_set_address $foo_end}
+ {DW_LNS_advance_line 1}
+ {DW_LNS_copy}
+ {DW_LNE_end_sequence}
+
+ {DW_LNE_set_address $bar_start}
+ {DW_LNS_advance_line [expr [gdb_get_line_number "bar end"] - 1]}
+ {DW_LNS_copy}
+ {DW_LNS_advance_pc $bar_len}
+ {DW_LNS_advance_line 1}
+ {DW_LNS_copy}
+ {DW_LNE_end_sequence}
+
+ {DW_LNE_set_address $baz_start}
+ {DW_LNS_advance_line [expr [gdb_get_line_number "baz end"] - 1]}
+ {DW_LNS_copy}
+ {DW_LNS_advance_pc $baz_len}
+ {DW_LNS_advance_line 1}
+ {DW_LNS_copy}
+ {DW_LNE_end_sequence}
+
+ {DW_LNE_set_address $foo_cold_start}
+ {DW_LNS_advance_line [expr [gdb_get_line_number "foo_cold prologue"] - 1]}
+ {DW_LNS_copy}
+ {DW_LNE_set_address foo_cold_label}
+ {DW_LNS_advance_line [expr [gdb_get_line_number "foo_cold baz call"] - [gdb_get_line_number "foo_cold prologue"]]}
+ {DW_LNS_copy}
+ {DW_LNE_set_address foo_cold_label2}
+ {DW_LNS_advance_line [expr [gdb_get_line_number "foo_cold end"] - [gdb_get_line_number "foo_cold baz call"]]}
+ {DW_LNS_copy}
+ {DW_LNE_set_address $foo_cold_end}
+ {DW_LNS_advance_line 1}
+ {DW_LNS_copy}
+ {DW_LNE_end_sequence}
}
- subprogram {
- {external 1 flag}
- {name bar}
- {low_pc $bar_start addr}
- {high_pc $bar_len DW_FORM_data4}
+ }
+
+ # Generate ranges data.
+ ranges {is_64 [is_64_target]} {
+ func_ranges_label: sequence {
+ {range {$foo_start } $foo_end}
+ {range {$foo_cold_start} $foo_cold_end}
}
- subprogram {
- {external 1 flag}
- {name baz}
- {low_pc $baz_start addr}
- {high_pc $baz_len DW_FORM_data4}
+ cu_ranges_label: sequence {
+ {range {$foo_start } $foo_end}
+ {range {$foo_cold_start} $foo_cold_end}
+ {range {$main_start} $main_end}
+ {range {$bar_start} $bar_end}
+ {range {$baz_start} $baz_end}
}
}
}
- lines {version 2} L {
- include_dir "${srcdir}/${subdir}"
- file_name "$srcfile" 1
-
- # Generate a line table program. An attempt was made to make it
- # reasonably accurate as it made debugging the test case easier.
- program {
- {DW_LNE_set_address $main_start}
- {DW_LNS_advance_line [expr [gdb_get_line_number "main prologue"] - 1]}
- {DW_LNS_copy}
- {DW_LNE_set_address main_label}
- {DW_LNS_advance_line [expr [gdb_get_line_number "main foo call"] - [gdb_get_line_number "main prologue"]]}
- {DW_LNS_copy}
- {DW_LNE_set_address main_label2}
- {DW_LNS_advance_line [expr [gdb_get_line_number "main return"] - [gdb_get_line_number "main foo call"]]}
- {DW_LNS_copy}
- {DW_LNE_set_address $main_end}
- {DW_LNS_advance_line [expr [gdb_get_line_number "main end"] - [gdb_get_line_number "main return"] + 1]}
- {DW_LNS_copy}
- {DW_LNE_end_sequence}
-
- {DW_LNE_set_address $foo_start}
- {DW_LNS_advance_line [expr [gdb_get_line_number "foo prologue"] - 1] }
- {DW_LNS_copy}
- {DW_LNE_set_address foo_label}
- {DW_LNS_advance_line [expr [gdb_get_line_number "foo bar call"] - [gdb_get_line_number "foo prologue"]]}
- {DW_LNS_copy}
- {DW_LNE_set_address foo_label2}
- {DW_LNS_advance_line [expr [gdb_get_line_number "foo foo_low call"] - [gdb_get_line_number "foo bar call"]]}
- {DW_LNS_copy}
- {DW_LNE_set_address foo_label3}
- {DW_LNS_advance_line [expr [gdb_get_line_number "foo end"] - [gdb_get_line_number "foo foo_low call"]]}
- {DW_LNS_copy}
- {DW_LNE_set_address $foo_end}
- {DW_LNS_advance_line 1}
- {DW_LNS_copy}
- {DW_LNE_end_sequence}
-
- {DW_LNE_set_address $bar_start}
- {DW_LNS_advance_line [expr [gdb_get_line_number "bar end"] - 1]}
- {DW_LNS_copy}
- {DW_LNS_advance_pc $bar_len}
- {DW_LNS_advance_line 1}
- {DW_LNS_copy}
- {DW_LNE_end_sequence}
-
- {DW_LNE_set_address $baz_start}
- {DW_LNS_advance_line [expr [gdb_get_line_number "baz end"] - 1]}
- {DW_LNS_copy}
- {DW_LNS_advance_pc $baz_len}
- {DW_LNS_advance_line 1}
- {DW_LNS_copy}
- {DW_LNE_end_sequence}
-
- {DW_LNE_set_address $foo_low_start}
- {DW_LNS_advance_line [expr [gdb_get_line_number "foo_low prologue"] - 1]}
- {DW_LNS_copy}
- {DW_LNE_set_address foo_low_label}
- {DW_LNS_advance_line [expr [gdb_get_line_number "foo_low baz call"] - [gdb_get_line_number "foo_low prologue"]]}
- {DW_LNS_copy}
- {DW_LNE_set_address foo_low_label2}
- {DW_LNS_advance_line [expr [gdb_get_line_number "foo_low end"] - [gdb_get_line_number "foo_low baz call"]]}
- {DW_LNS_copy}
- {DW_LNE_set_address $foo_low_end}
- {DW_LNS_advance_line 1}
- {DW_LNS_copy}
- {DW_LNE_end_sequence}
- }
+ if { [prepare_for_testing "failed to prepare" ${testfile} \
+ [list $srcfile $asm_file] {nodebug}] } {
+ return -1
}
- # Generate ranges data.
- ranges {is_64 [is_64_target]} {
- func_ranges_label: sequence {
- {range {$foo_start } $foo_end}
- {range {$foo_low_start} $foo_low_end}
- }
- cu_ranges_label: sequence {
- {range {$foo_start } $foo_end}
- {range {$foo_low_start} $foo_low_end}
- {range {$main_start} $main_end}
- {range {$bar_start} $bar_end}
- {range {$baz_start} $baz_end}
- }
+ if ![runto_main] {
+ return -1
}
-}
-if { [prepare_for_testing "failed to prepare" ${testfile} \
- [list $srcfile $asm_file] {nodebug}] } {
- return -1
-}
+ set main_prologue_line_num [gdb_get_line_number "main prologue"]
+ # Do a sanity check to make sure that line number info is available.
+ gdb_test "info line main" \
+ "Line ${main_prologue_line_num} of .* starts at address .* and ends at .*"
-if ![runto_main] {
- return -1
-}
+ with_test_prefix "step-test-1" {
+ set bp_foo_bar [gdb_get_line_number "foo bar call"]
-set main_prologue_line_num [gdb_get_line_number "main prologue"]
-# Do a sanity check to make sure that line number info is available.
-gdb_test "info line main" \
- "Line ${main_prologue_line_num} of .* starts at address .* and ends at .*"
+ gdb_test "break $bp_foo_bar" \
+ "Breakpoint.*at.* file .*$srcfile, line $bp_foo_bar\\." \
+ "break at call to bar"
-with_test_prefix "step-test-1" {
- set bp_foo_bar [gdb_get_line_number "foo bar call"]
+ gdb_test "continue" \
+ "Continuing\\..*Breakpoint \[0-9\]+, foo \\(\\).*$bp_foo_bar\\s+bar\\s\\(\\);.*foo bar call.*" \
+ "continue to call of bar"
- gdb_test "break $bp_foo_bar" \
- "Breakpoint.*at.* file .*$srcfile, line $bp_foo_bar\\." \
- "break at call to bar"
+ gdb_test "step" \
+ "bar \\(\\).*bar end.*" \
+ "step into bar"
- gdb_test "continue" \
- "Continuing\\..*Breakpoint \[0-9\]+, foo \\(\\).*$bp_foo_bar\\s+bar\\s\\(\\);.*foo bar call.*" \
- "continue to call of bar"
+ gdb_test "step" \
+ "foo \\(\\).*foo foo_cold call.*" \
+ "step out of bar, back into foo"
+ }
- gdb_test "step" \
- "bar \\(\\).*bar end.*" \
- "step into bar"
+ with_test_prefix "step-test-2" {
+ clean_restart ${testfile}
+ if ![runto_main] {
+ return -1
+ }
- gdb_test "step" \
- "foo \\(\\).*foo foo_low call.*" \
- "step out of bar, back into foo"
-}
+ # Note that the RE used for the following test will fail when the
+ # breakpoint has been set on multiple locations. E.g. "(2 locations)".
+ # This is intentional since that behavior is one of the bugs that
+ # this test case tests for.
+ gdb_test "break foo" \
+ "Breakpoint.*at.* file .*$srcfile, line \\d+\\." \
+ "break foo"
+
+ # Continue to foo. Allow execution to stop either on the prologue
+ # or on the call to bar since either behavior is acceptable though
+ # the latter is preferred.
+ set test "continue to foo"
+ gdb_test_multiple "continue" $test {
+ -re "Breakpoint \\d+, foo \\(\\).*foo prologue.*${gdb_prompt}" {
+ pass $test
+ gdb_test "step" \
+ "foo bar call .*" \
+ "step to call of bar after landing on prologue"
+ }
+ -re "Breakpoint \\d+, foo \\(\\).*foo bar call.*${gdb_prompt}" {
+ pass $test
+ }
+ }
+
+ gdb_test "step" \
+ "bar \\(\\).*bar end.*" \
+ "step into bar"
+
+ gdb_test "step" \
+ "foo \\(\\).*foo foo_cold call.*" \
+ "step out of bar, back into foo"
+ }
-with_test_prefix "step-test-2" {
clean_restart ${testfile}
if ![runto_main] {
return -1
}
- # Note that the RE used for the following test will fail when the
- # breakpoint has been set on multiple locations. E.g. "(2 locations)".
- # This is intentional since that behavior is one of the bugs that
- # this test case tests for.
- gdb_test "break foo" \
- "Breakpoint.*at.* file .*$srcfile, line \\d+\\." \
- "break foo"
-
- # Continue to foo. Allow execution to stop either on the prologue
- # or on the call to bar since either behavior is acceptable though
- # the latter is preferred.
- set test "continue to foo"
- gdb_test_multiple "continue" $test {
- -re "Breakpoint \\d+, foo \\(\\).*foo prologue.*${gdb_prompt}" {
+ # Disassembly of foo should have multiple address ranges.
+ gdb_test_sequence "disassemble foo" "" [list \
+ "Dump of assembler code for function foo:" \
+ "Address range $hex to $hex:" \
+ " $hex <\\+0>:" \
+ "Address range $hex to $hex:" \
+ " $hex <(.+?)>:" \
+ "End of assembler dump\\." \
+ ]
+
+ set foo_cold_addr -1
+ set test "x/i foo_cold"
+ gdb_test_multiple $test $test {
+ -re " ($hex) <foo.*?>.*${gdb_prompt}" {
+ set foo_cold_addr $expect_out(1,string)
pass $test
- gdb_test "step" \
- "foo bar call .*" \
- "step to call of bar after landing on prologue"
}
- -re "Breakpoint \\d+, foo \\(\\).*foo bar call.*${gdb_prompt}" {
+ }
+
+ set foo_addr -1
+ set test "x/i foo"
+ gdb_test_multiple $test $test {
+ -re " ($hex) <foo.*?>.*${gdb_prompt}" {
+ set foo_addr $expect_out(1,string)
pass $test
}
}
- gdb_test "step" \
- "bar \\(\\).*bar end.*" \
- "step into bar"
-
- gdb_test "step" \
- "foo \\(\\).*foo foo_low call.*" \
- "step out of bar, back into foo"
-}
+ gdb_assert {$foo_cold_addr != $foo_addr} "foo and foo_cold are at different addresses"
-clean_restart ${testfile}
-if ![runto_main] {
- return -1
-}
+ # This more permissive RE for "break foo" will allow a breakpoint on
+ # multiple locations to PASS. */
+ gdb_test "break foo" \
+ "Breakpoint.*at.*" \
+ "break foo"
-# Disassembly of foo should have multiple address ranges.
-gdb_test_sequence "disassemble foo" "" [list \
- "Dump of assembler code for function foo:" \
- "Address range $hex to $hex:" \
- " $hex <\\+0>:" \
- "Address range $hex to $hex:" \
- " $hex <(.+?)>:" \
- "End of assembler dump\\." \
-]
-
-set foo_low_addr -1
-set test "x/i foo_low"
-gdb_test_multiple $test $test {
- -re " ($hex) <foo.*?>.*${gdb_prompt}" {
- set foo_low_addr $expect_out(1,string)
- pass $test
- }
-}
+ gdb_test "break baz" \
+ "Breakpoint.*at.* file .*$srcfile, line \\d+\\."
-set foo_addr -1
-set test "x/i foo"
-gdb_test_multiple $test $test {
- -re " ($hex) <foo.*?>.*${gdb_prompt}" {
- set foo_addr $expect_out(1,string)
- pass $test
- }
-}
+ gdb_test "continue" \
+ "Breakpoint \\d+, foo \\(\\).*" \
+ "continue to foo"
-gdb_assert {$foo_low_addr != $foo_addr} "foo and foo_low are at different addresses"
+ gdb_test_no_output "set variable e=1"
-# This more permissive RE for "break foo" will allow a breakpoint on
-# multiple locations to PASS. */
-gdb_test "break foo" \
- "Breakpoint.*at.*" \
- "break foo"
+ # If GDB incorrectly places the foo breakpoint on multiple locations,
+ # then GDB will (incorrectly) stop in foo_cold instead of in baz.
+ gdb_test "continue" \
+ "Breakpoint \\d+, (?:$hex in )?baz \\(\\).*" \
+ "continue to baz"
+
+ with_test_prefix "no-cold-names" {
+
+ # Due to the calling sequence, this backtrace would normally
+ # show function foo_cold for frame #1. However, we don't want
+ # this to be the case due to placing it in the same block
+ # (albeit at a different range) as foo. Thus it is correct to
+ # see foo for frames #1 and #2. It is incorrect to see
+ # foo_cold at frame #1.
+ gdb_test_sequence "bt" "backtrace from baz" {
+ "\[\r\n\]#0 .*? baz \\(\\) "
+ "\[\r\n\]#1 .*? foo \\(\\) "
+ "\[\r\n\]#2 .*? foo \\(\\) "
+ "\[\r\n\]#3 .*? main \\(\\) "
+ }
-gdb_test "break baz" \
- "Breakpoint.*at.* file .*$srcfile, line \\d+\\."
+ # Doing x/2i foo_cold should show foo_cold as the first symbolic
+ # address and an offset from foo for the second. We also check to
+ # make sure that the offset is not too large - we don't GDB to
+ # display really large offsets that would (try to) wrap around the
+ # address space.
+ set foo_cold_offset 0
+ set test "x/2i foo_cold"
+ gdb_test_multiple $test $test {
+ -re " (?:$hex) <foo_cold>.*?\n (?:$hex) <foo\[+-\](\[0-9\]+)>.*${gdb_prompt}" {
+ set foo_cold_offset $expect_out(1,string)
+ pass $test
+ }
+ }
+ gdb_assert {$foo_cold_offset <= 10000} "offset to foo_cold is not too large"
-gdb_test "continue" \
- "Breakpoint \\d+, foo \\(\\).*" \
- "continue to foo"
+ # Likewise, verify that second address shown by "info line" is at
+ # and offset from foo instead of foo_cold.
+ gdb_test "info line *foo_cold" "starts at address $hex <foo_cold> and ends at $hex <foo\[+-\].*?>.*"
-gdb_test_no_output "set variable e=1"
+ }
-# If GDB incorrectly places the foo breakpoint on multiple locations,
-# then GDB will (incorrectly) stop in foo_low instead of in baz.
-gdb_test "continue" \
- "Breakpoint \\d+, (?:$hex in )?baz \\(\\).*" \
- "continue to baz"
+ with_test_prefix "step-test-3" {
+ clean_restart ${testfile}
+ if ![runto_main] {
+ return -1
+ }
-with_test_prefix "step-test-3" {
- clean_restart ${testfile}
- if ![runto_main] {
- return -1
- }
+ gdb_test "step" \
+ "foo \\(\\).*bar \\(\\);.*foo bar call.*" \
+ "step into foo from main"
- gdb_test "step" \
- "foo \\(\\).*bar \\(\\);.*foo bar call.*" \
- "step into foo from main"
-
- gdb_test "step" \
- "bar \\(\\).*\}.* bar end.*" \
- "step into bar from foo"
-
- gdb_test "step" \
- "foo(_label2)? \\(\\).*foo_low \\(\\);.*foo foo_low call.*" \
- "step out of bar to foo"
-
- # The tests in the "enable_foo_low_stepping" section, below, work
- # with some versions of gcc, though it's not clear that they
- # should. This test case causes foo_low, originally a separate
- # function invoked via a subroutine call, to be considered as part
- # of foo via use of DW_AT_ranges. Real code that I've looked at
- # uses a branch instruction to cause code in the "cold" range to
- # be executed.
- #
- # For the moment though, these tests have been left in place, but
- # disabled, in case we decide that making such a subroutine call
- # is a reasonable thing to do that should also be supported by
- # GDB.
+ gdb_test "step" \
+ "bar \\(\\).*\}.* bar end.*" \
+ "step into bar from foo"
- set enable_foo_low_stepping false
+ gdb_test "step" \
+ "foo(_label2)? \\(\\).*foo_cold \\(\\);.*foo foo_cold call.*" \
+ "step out of bar to foo"
+
+ # The tests in the "enable_foo_cold_stepping" section, below, work
+ # with some versions of gcc, though it's not clear that they
+ # should. This test case causes foo_cold, originally a separate
+ # function invoked via a subroutine call, to be considered as part
+ # of foo via use of DW_AT_ranges. Real code that I've looked at
+ # uses a branch instruction to cause code in the "cold" range to
+ # be executed.
+ #
+ # For the moment though, these tests have been left in place, but
+ # disabled, in case we decide that making such a subroutine call
+ # is a reasonable thing to do that should also be supported by
+ # GDB.
+
+ set enable_foo_cold_stepping false
+
+ if { $enable_foo_cold_stepping } {
+ gdb_test_no_output "set variable e=1"
+
+ set test "step into foo_cold from foo"
+ gdb_test_multiple "step" $test {
+ -re "foo(_low)? \\(\\).*\{.*foo_cold prologue.*${gdb_prompt}" {
+ pass $test
+ gdb_test "step" \
+ "foo \\(\\).*baz \\(\\);.*foo_cold baz call.*" \
+ "step to baz call in foo_cold"
+
+ }
+ -re "foo(_low)? \\(\\).*baz \\(\\);.*foo_cold baz call.*${gdb_prompt}" {
+ pass $test
+ }
+ }
- if { $enable_foo_low_stepping } {
- gdb_test_no_output "set variable e=1"
+ gdb_test "step" \
+ "baz \\(\\).*\}.*baz end.*" \
+ "step into baz from foo_cold"
- set test "step into foo_low from foo"
- gdb_test_multiple "step" $test {
- -re "foo(_low)? \\(\\).*\{.*foo_low prologue.*${gdb_prompt}" {
- pass $test
- gdb_test "step" \
- "foo \\(\\).*baz \\(\\);.*foo_low baz call.*" \
- "step to baz call in foo_low"
+ gdb_test "step" \
+ "foo(?:_low(?:_label2)?)? \\(\\).*\}.*foo_cold end.*" \
+ "step out of baz to foo_cold"
- }
- -re "foo(_low)? \\(\\).*baz \\(\\);.*foo_low baz call.*${gdb_prompt}" {
- pass $test
- }
+ gdb_test "step" \
+ "foo(?:_label3)? \\(\\).*\}.*foo end.*" \
+ "step out of foo_cold to foo"
+ } else {
+ gdb_test "next" \
+ ".*foo end.*" \
+ "next over foo_cold call"
}
gdb_test "step" \
- "baz \\(\\).*\}.*baz end.*" \
- "step into baz from foo_low"
+ "main(?:_label2)? \\(\\).*" \
+ "step out of foo to main"
+ }
+}
- gdb_test "step" \
- "foo(?:_low(?:_label2)?)? \\(\\).*\}.*foo_low end.*" \
- "step out of baz to foo_low"
+# foreach_with_prefix could be used here, but the log file output is somewhat
+# less verbose when using an explicit "with_test_prefix".
- gdb_test "step" \
- "foo(?:_label3)? \\(\\).*\}.*foo end.*" \
- "step out of foo_low to foo"
- } else {
- gdb_test "next" \
- ".*foo end.*" \
- "next over foo_low call"
+foreach test_suffix { "lo-cold" "hi-cold" } {
+ with_test_prefix $test_suffix {
+ do_test $test_suffix
}
-
- gdb_test "step" \
- "main(?:_label2)? \\(\\).*" \
- "step out of foo to main"
}
--
2.21.0
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2 5/5] Improve test gdb.dwarf2/dw2-ranges-func.exp
2019-07-04 5:05 ` [PATCH v2 5/5] Improve test gdb.dwarf2/dw2-ranges-func.exp Kevin Buettner
@ 2019-07-04 5:08 ` Kevin Buettner
0 siblings, 0 replies; 11+ messages in thread
From: Kevin Buettner @ 2019-07-04 5:08 UTC (permalink / raw)
To: gdb-patches
I think that the dw2-ranges-func.exp portion of this patch is
easier to review by ignoring white space changes. Here's just the
diff (using -w) for that set of changes:
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-ranges-func.exp b/gdb/testsuite/gdb.dwarf2/dw2-ranges-func.exp
index ccf18b6983..fdc488ae92 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-ranges-func.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-ranges-func.exp
@@ -30,7 +30,16 @@ if !$gcc_compiled {
return 0
}
-standard_testfile dw2-ranges-func.c dw2-ranges-func-dw.S
+proc do_test {suffix} {
+ global gdb_test_file_name
+ global testfile binfile srcfile srcfile2 gdb_prompt hex
+
+ # Don't use standard_testfile; we want different binaries for
+ # each suffix.
+ set testfile $gdb_test_file_name-$suffix
+ set binfile [standard_output_file ${testfile}]
+ set srcfile $testfile.c
+ set srcfile2 $testfile-dw2.S
# We need to know the size of integer and address types in order to
# write some of the debugging info we'd like to generate.
@@ -56,9 +65,9 @@ Dwarf::assemble $asm_file {
lassign [function_range foo [list ${srcdir}/${subdir}/$srcfile]] \
foo_start foo_len
set foo_end "$foo_start + $foo_len"
- lassign [function_range foo_low [list ${srcdir}/${subdir}/$srcfile]] \
- foo_low_start foo_low_len
- set foo_low_end "$foo_low_start + $foo_low_len"
+ lassign [function_range foo_cold [list ${srcdir}/${subdir}/$srcfile]] \
+ foo_cold_start foo_cold_len
+ set foo_cold_end "$foo_cold_start + $foo_cold_len"
lassign [function_range bar [list ${srcdir}/${subdir}/$srcfile]] \
bar_start bar_len
set bar_end "$bar_start + $bar_len"
@@ -71,7 +80,7 @@ Dwarf::assemble $asm_file {
cu {} {
compile_unit {
{language @DW_LANG_C}
- {name dw-ranges-func.c}
+ {name dw-ranges-func2.c}
{stmt_list $L DW_FORM_sec_offset}
{low_pc 0 addr}
{ranges ${cu_ranges_label} DW_FORM_sec_offset}
@@ -145,10 +154,10 @@ Dwarf::assemble $asm_file {
{DW_LNS_advance_line [expr [gdb_get_line_number "foo bar call"] - [gdb_get_line_number "foo prologue"]]}
{DW_LNS_copy}
{DW_LNE_set_address foo_label2}
- {DW_LNS_advance_line [expr [gdb_get_line_number "foo foo_low call"] - [gdb_get_line_number "foo bar call"]]}
+ {DW_LNS_advance_line [expr [gdb_get_line_number "foo foo_cold call"] - [gdb_get_line_number "foo bar call"]]}
{DW_LNS_copy}
{DW_LNE_set_address foo_label3}
- {DW_LNS_advance_line [expr [gdb_get_line_number "foo end"] - [gdb_get_line_number "foo foo_low call"]]}
+ {DW_LNS_advance_line [expr [gdb_get_line_number "foo end"] - [gdb_get_line_number "foo foo_cold call"]]}
{DW_LNS_copy}
{DW_LNE_set_address $foo_end}
{DW_LNS_advance_line 1}
@@ -171,16 +180,16 @@ Dwarf::assemble $asm_file {
{DW_LNS_copy}
{DW_LNE_end_sequence}
- {DW_LNE_set_address $foo_low_start}
- {DW_LNS_advance_line [expr [gdb_get_line_number "foo_low prologue"] - 1]}
+ {DW_LNE_set_address $foo_cold_start}
+ {DW_LNS_advance_line [expr [gdb_get_line_number "foo_cold prologue"] - 1]}
{DW_LNS_copy}
- {DW_LNE_set_address foo_low_label}
- {DW_LNS_advance_line [expr [gdb_get_line_number "foo_low baz call"] - [gdb_get_line_number "foo_low prologue"]]}
+ {DW_LNE_set_address foo_cold_label}
+ {DW_LNS_advance_line [expr [gdb_get_line_number "foo_cold baz call"] - [gdb_get_line_number "foo_cold prologue"]]}
{DW_LNS_copy}
- {DW_LNE_set_address foo_low_label2}
- {DW_LNS_advance_line [expr [gdb_get_line_number "foo_low end"] - [gdb_get_line_number "foo_low baz call"]]}
+ {DW_LNE_set_address foo_cold_label2}
+ {DW_LNS_advance_line [expr [gdb_get_line_number "foo_cold end"] - [gdb_get_line_number "foo_cold baz call"]]}
{DW_LNS_copy}
- {DW_LNE_set_address $foo_low_end}
+ {DW_LNE_set_address $foo_cold_end}
{DW_LNS_advance_line 1}
{DW_LNS_copy}
{DW_LNE_end_sequence}
@@ -191,11 +200,11 @@ Dwarf::assemble $asm_file {
ranges {is_64 [is_64_target]} {
func_ranges_label: sequence {
{range {$foo_start } $foo_end}
- {range {$foo_low_start} $foo_low_end}
+ {range {$foo_cold_start} $foo_cold_end}
}
cu_ranges_label: sequence {
{range {$foo_start } $foo_end}
- {range {$foo_low_start} $foo_low_end}
+ {range {$foo_cold_start} $foo_cold_end}
{range {$main_start} $main_end}
{range {$bar_start} $bar_end}
{range {$baz_start} $baz_end}
@@ -233,7 +242,7 @@ with_test_prefix "step-test-1" {
"step into bar"
gdb_test "step" \
- "foo \\(\\).*foo foo_low call.*" \
+ "foo \\(\\).*foo foo_cold call.*" \
"step out of bar, back into foo"
}
@@ -272,7 +281,7 @@ with_test_prefix "step-test-2" {
"step into bar"
gdb_test "step" \
- "foo \\(\\).*foo foo_low call.*" \
+ "foo \\(\\).*foo foo_cold call.*" \
"step out of bar, back into foo"
}
@@ -291,11 +300,11 @@ gdb_test_sequence "disassemble foo" "" [list \
"End of assembler dump\\." \
]
-set foo_low_addr -1
-set test "x/i foo_low"
+ set foo_cold_addr -1
+ set test "x/i foo_cold"
gdb_test_multiple $test $test {
-re " ($hex) <foo.*?>.*${gdb_prompt}" {
- set foo_low_addr $expect_out(1,string)
+ set foo_cold_addr $expect_out(1,string)
pass $test
}
}
@@ -309,7 +318,7 @@ gdb_test_multiple $test $test {
}
}
-gdb_assert {$foo_low_addr != $foo_addr} "foo and foo_low are at different addresses"
+ gdb_assert {$foo_cold_addr != $foo_addr} "foo and foo_cold are at different addresses"
# This more permissive RE for "break foo" will allow a breakpoint on
# multiple locations to PASS. */
@@ -327,11 +336,47 @@ gdb_test "continue" \
gdb_test_no_output "set variable e=1"
# If GDB incorrectly places the foo breakpoint on multiple locations,
-# then GDB will (incorrectly) stop in foo_low instead of in baz.
+ # then GDB will (incorrectly) stop in foo_cold instead of in baz.
gdb_test "continue" \
"Breakpoint \\d+, (?:$hex in )?baz \\(\\).*" \
"continue to baz"
+ with_test_prefix "no-cold-names" {
+
+ # Due to the calling sequence, this backtrace would normally
+ # show function foo_cold for frame #1. However, we don't want
+ # this to be the case due to placing it in the same block
+ # (albeit at a different range) as foo. Thus it is correct to
+ # see foo for frames #1 and #2. It is incorrect to see
+ # foo_cold at frame #1.
+ gdb_test_sequence "bt" "backtrace from baz" {
+ "\[\r\n\]#0 .*? baz \\(\\) "
+ "\[\r\n\]#1 .*? foo \\(\\) "
+ "\[\r\n\]#2 .*? foo \\(\\) "
+ "\[\r\n\]#3 .*? main \\(\\) "
+ }
+
+ # Doing x/2i foo_cold should show foo_cold as the first symbolic
+ # address and an offset from foo for the second. We also check to
+ # make sure that the offset is not too large - we don't GDB to
+ # display really large offsets that would (try to) wrap around the
+ # address space.
+ set foo_cold_offset 0
+ set test "x/2i foo_cold"
+ gdb_test_multiple $test $test {
+ -re " (?:$hex) <foo_cold>.*?\n (?:$hex) <foo\[+-\](\[0-9\]+)>.*${gdb_prompt}" {
+ set foo_cold_offset $expect_out(1,string)
+ pass $test
+ }
+ }
+ gdb_assert {$foo_cold_offset <= 10000} "offset to foo_cold is not too large"
+
+ # Likewise, verify that second address shown by "info line" is at
+ # and offset from foo instead of foo_cold.
+ gdb_test "info line *foo_cold" "starts at address $hex <foo_cold> and ends at $hex <foo\[+-\].*?>.*"
+
+ }
+
with_test_prefix "step-test-3" {
clean_restart ${testfile}
if ![runto_main] {
@@ -347,12 +392,12 @@ with_test_prefix "step-test-3" {
"step into bar from foo"
gdb_test "step" \
- "foo(_label2)? \\(\\).*foo_low \\(\\);.*foo foo_low call.*" \
+ "foo(_label2)? \\(\\).*foo_cold \\(\\);.*foo foo_cold call.*" \
"step out of bar to foo"
- # The tests in the "enable_foo_low_stepping" section, below, work
+ # The tests in the "enable_foo_cold_stepping" section, below, work
# with some versions of gcc, though it's not clear that they
- # should. This test case causes foo_low, originally a separate
+ # should. This test case causes foo_cold, originally a separate
# function invoked via a subroutine call, to be considered as part
# of foo via use of DW_AT_ranges. Real code that I've looked at
# uses a branch instruction to cause code in the "cold" range to
@@ -363,43 +408,53 @@ with_test_prefix "step-test-3" {
# is a reasonable thing to do that should also be supported by
# GDB.
- set enable_foo_low_stepping false
+ set enable_foo_cold_stepping false
- if { $enable_foo_low_stepping } {
+ if { $enable_foo_cold_stepping } {
gdb_test_no_output "set variable e=1"
- set test "step into foo_low from foo"
+ set test "step into foo_cold from foo"
gdb_test_multiple "step" $test {
- -re "foo(_low)? \\(\\).*\{.*foo_low prologue.*${gdb_prompt}" {
+ -re "foo(_low)? \\(\\).*\{.*foo_cold prologue.*${gdb_prompt}" {
pass $test
gdb_test "step" \
- "foo \\(\\).*baz \\(\\);.*foo_low baz call.*" \
- "step to baz call in foo_low"
+ "foo \\(\\).*baz \\(\\);.*foo_cold baz call.*" \
+ "step to baz call in foo_cold"
}
- -re "foo(_low)? \\(\\).*baz \\(\\);.*foo_low baz call.*${gdb_prompt}" {
+ -re "foo(_low)? \\(\\).*baz \\(\\);.*foo_cold baz call.*${gdb_prompt}" {
pass $test
}
}
gdb_test "step" \
"baz \\(\\).*\}.*baz end.*" \
- "step into baz from foo_low"
+ "step into baz from foo_cold"
gdb_test "step" \
- "foo(?:_low(?:_label2)?)? \\(\\).*\}.*foo_low end.*" \
- "step out of baz to foo_low"
+ "foo(?:_low(?:_label2)?)? \\(\\).*\}.*foo_cold end.*" \
+ "step out of baz to foo_cold"
gdb_test "step" \
"foo(?:_label3)? \\(\\).*\}.*foo end.*" \
- "step out of foo_low to foo"
+ "step out of foo_cold to foo"
} else {
gdb_test "next" \
".*foo end.*" \
- "next over foo_low call"
+ "next over foo_cold call"
}
gdb_test "step" \
"main(?:_label2)? \\(\\).*" \
"step out of foo to main"
}
+}
+
+# foreach_with_prefix could be used here, but the log file output is somewhat
+# less verbose when using an explicit "with_test_prefix".
+
+foreach test_suffix { "lo-cold" "hi-cold" } {
+ with_test_prefix $test_suffix {
+ do_test $test_suffix
+ }
+}
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 4/5] Allow display of negative offsets in print_address_symbolic()
2019-07-04 4:55 [PATCH v2 0/5] Non-contiguous address range bug fixes / improvements Kevin Buettner
` (3 preceding siblings ...)
2019-07-04 5:05 ` [PATCH v2 5/5] Improve test gdb.dwarf2/dw2-ranges-func.exp Kevin Buettner
@ 2019-07-04 5:05 ` Kevin Buettner
2019-07-30 16:47 ` Kevin Buettner
2019-07-27 20:48 ` [PATCH v2 0/5] Non-contiguous address range bug fixes / improvements Kevin Buettner
5 siblings, 1 reply; 11+ messages in thread
From: Kevin Buettner @ 2019-07-04 5:05 UTC (permalink / raw)
To: gdb-patches; +Cc: Kevin Buettner
When examining addresses associated with blocks with non-contiguous
address ranges, it's not uncommon to see large positive offsets which,
for some address width, actually represent a smaller negative offset.
Here's an example taken from the test case (using the
dw2-ranges-func-lo-cold executable):
(gdb) x/5i foo_cold
0x40110d <foo+4294967277>: push %rbp
0x40110e <foo+4294967278>: mov %rsp,%rbp
0x401111 <foo+4294967281>: callq 0x401106 <baz>
0x401116 <foo+4294967286>: nop
0x401117 <foo+4294967287>: pop %rbp
This commit, in conjuction with an earlier patch from this series, causes
cases like the above to be displayed like this (below) instead:
(gdb) x/5i foo_cold
0x40110d <foo_cold>: push %rbp
0x40110e <foo-18>: mov %rsp,%rbp
0x401111 <foo-15>: callq 0x401106 <baz>
0x401116 <foo-10>: nop
0x401117 <foo-9>: pop %rbp
Note that the address of foo_cold is now (due to another patch) being
displayed as <foo_cold> instead of <foo+BigOffset>. The subsequent
lines are shown as negative offsets from foo.
Disassembly using the "disassemble" command is somewhat affected by
these changes:
Before:
(gdb) disassemble foo_cold
Dump of assembler code for function foo:
Address range 0x401120 to 0x40113b:
0x0000000000401120 <+0>: push %rbp
0x0000000000401121 <+1>: mov %rsp,%rbp
0x0000000000401124 <+4>: callq 0x401119 <bar>
0x0000000000401129 <+9>: mov 0x2ef1(%rip),%eax # 0x404020 <e>
0x000000000040112f <+15>: test %eax,%eax
0x0000000000401131 <+17>: je 0x401138 <foo+24>
0x0000000000401133 <+19>: callq 0x40110d <foo+4294967277>
0x0000000000401138 <+24>: nop
0x0000000000401139 <+25>: pop %rbp
0x000000000040113a <+26>: retq
Address range 0x40110d to 0x401119:
0x000000000040110d <+-19>: push %rbp
0x000000000040110e <+-18>: mov %rsp,%rbp
0x0000000000401111 <+-15>: callq 0x401106 <baz>
0x0000000000401116 <+-10>: nop
0x0000000000401117 <+-9>: pop %rbp
0x0000000000401118 <+-8>: retq
End of assembler dump.
After:
(gdb) disassemble foo_cold
Dump of assembler code for function foo:
Address range 0x401120 to 0x40113b:
0x0000000000401120 <+0>: push %rbp
0x0000000000401121 <+1>: mov %rsp,%rbp
0x0000000000401124 <+4>: callq 0x401119 <bar>
0x0000000000401129 <+9>: mov 0x2ef1(%rip),%eax # 0x404020 <e>
0x000000000040112f <+15>: test %eax,%eax
0x0000000000401131 <+17>: je 0x401138 <foo+24>
0x0000000000401133 <+19>: callq 0x40110d <foo_cold>
0x0000000000401138 <+24>: nop
0x0000000000401139 <+25>: pop %rbp
0x000000000040113a <+26>: retq
Address range 0x40110d to 0x401119:
0x000000000040110d <-19>: push %rbp
0x000000000040110e <-18>: mov %rsp,%rbp
0x0000000000401111 <-15>: callq 0x401106 <baz>
0x0000000000401116 <-10>: nop
0x0000000000401117 <-9>: pop %rbp
0x0000000000401118 <-8>: retq
End of assembler dump.
Note that negative offsets are now displayed without the leading "+".
Also, the callq to foo_cold is now displayed as such instead of a callq
to foo with a large positive offset.
gdb/ChangeLog:
* printcmd.c (print_address_symbolic): Print negative offsets.
(build_address_symbolic): Force signed arithmetic when computing
offset.
---
gdb/printcmd.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 1109cb3046..dce6ab2db9 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -539,7 +539,7 @@ print_address_symbolic (struct gdbarch *gdbarch, CORE_ADDR addr,
fputs_filtered ("<", stream);
fputs_styled (name.c_str (), function_name_style.style (), stream);
if (offset != 0)
- fprintf_filtered (stream, "+%u", (unsigned int) offset);
+ fprintf_filtered (stream, "%+d", offset);
/* Append source filename and line number if desired. Give specific
line # of this addr, if we have it; else line # of the nearest symbol. */
@@ -679,7 +679,7 @@ build_address_symbolic (struct gdbarch *gdbarch,
&& name_location + max_symbolic_offset > name_location)
return 1;
- *offset = addr - name_location;
+ *offset = (LONGEST) addr - name_location;
*name = name_temp;
--
2.21.0
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2 4/5] Allow display of negative offsets in print_address_symbolic()
2019-07-04 5:05 ` [PATCH v2 4/5] Allow display of negative offsets in print_address_symbolic() Kevin Buettner
@ 2019-07-30 16:47 ` Kevin Buettner
0 siblings, 0 replies; 11+ messages in thread
From: Kevin Buettner @ 2019-07-30 16:47 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom de Vries
I somehow missed pushing this commit when I pushed the rest of the
series. It was strange though - that second hunk of the patch was in,
but the first was not. Moreover, I didn't see an actual commit in master.
Regardless, it should be in now.
Thanks to Tom de Vries for his help in finding this problem.
Kevin
On Wed, 3 Jul 2019 21:55:02 -0700
Kevin Buettner <kevinb@redhat.com> wrote:
> When examining addresses associated with blocks with non-contiguous
> address ranges, it's not uncommon to see large positive offsets which,
> for some address width, actually represent a smaller negative offset.
> Here's an example taken from the test case (using the
> dw2-ranges-func-lo-cold executable):
>
> (gdb) x/5i foo_cold
> 0x40110d <foo+4294967277>: push %rbp
> 0x40110e <foo+4294967278>: mov %rsp,%rbp
> 0x401111 <foo+4294967281>: callq 0x401106 <baz>
> 0x401116 <foo+4294967286>: nop
> 0x401117 <foo+4294967287>: pop %rbp
>
> This commit, in conjuction with an earlier patch from this series, causes
> cases like the above to be displayed like this (below) instead:
>
> (gdb) x/5i foo_cold
> 0x40110d <foo_cold>: push %rbp
> 0x40110e <foo-18>: mov %rsp,%rbp
> 0x401111 <foo-15>: callq 0x401106 <baz>
> 0x401116 <foo-10>: nop
> 0x401117 <foo-9>: pop %rbp
>
> Note that the address of foo_cold is now (due to another patch) being
> displayed as <foo_cold> instead of <foo+BigOffset>. The subsequent
> lines are shown as negative offsets from foo.
>
> Disassembly using the "disassemble" command is somewhat affected by
> these changes:
>
> Before:
>
> (gdb) disassemble foo_cold
> Dump of assembler code for function foo:
> Address range 0x401120 to 0x40113b:
> 0x0000000000401120 <+0>: push %rbp
> 0x0000000000401121 <+1>: mov %rsp,%rbp
> 0x0000000000401124 <+4>: callq 0x401119 <bar>
> 0x0000000000401129 <+9>: mov 0x2ef1(%rip),%eax # 0x404020 <e>
> 0x000000000040112f <+15>: test %eax,%eax
> 0x0000000000401131 <+17>: je 0x401138 <foo+24>
> 0x0000000000401133 <+19>: callq 0x40110d <foo+4294967277>
> 0x0000000000401138 <+24>: nop
> 0x0000000000401139 <+25>: pop %rbp
> 0x000000000040113a <+26>: retq
> Address range 0x40110d to 0x401119:
> 0x000000000040110d <+-19>: push %rbp
> 0x000000000040110e <+-18>: mov %rsp,%rbp
> 0x0000000000401111 <+-15>: callq 0x401106 <baz>
> 0x0000000000401116 <+-10>: nop
> 0x0000000000401117 <+-9>: pop %rbp
> 0x0000000000401118 <+-8>: retq
> End of assembler dump.
>
> After:
>
> (gdb) disassemble foo_cold
> Dump of assembler code for function foo:
> Address range 0x401120 to 0x40113b:
> 0x0000000000401120 <+0>: push %rbp
> 0x0000000000401121 <+1>: mov %rsp,%rbp
> 0x0000000000401124 <+4>: callq 0x401119 <bar>
> 0x0000000000401129 <+9>: mov 0x2ef1(%rip),%eax # 0x404020 <e>
> 0x000000000040112f <+15>: test %eax,%eax
> 0x0000000000401131 <+17>: je 0x401138 <foo+24>
> 0x0000000000401133 <+19>: callq 0x40110d <foo_cold>
> 0x0000000000401138 <+24>: nop
> 0x0000000000401139 <+25>: pop %rbp
> 0x000000000040113a <+26>: retq
> Address range 0x40110d to 0x401119:
> 0x000000000040110d <-19>: push %rbp
> 0x000000000040110e <-18>: mov %rsp,%rbp
> 0x0000000000401111 <-15>: callq 0x401106 <baz>
> 0x0000000000401116 <-10>: nop
> 0x0000000000401117 <-9>: pop %rbp
> 0x0000000000401118 <-8>: retq
> End of assembler dump.
>
> Note that negative offsets are now displayed without the leading "+".
> Also, the callq to foo_cold is now displayed as such instead of a callq
> to foo with a large positive offset.
>
> gdb/ChangeLog:
>
> * printcmd.c (print_address_symbolic): Print negative offsets.
> (build_address_symbolic): Force signed arithmetic when computing
> offset.
> ---
> gdb/printcmd.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/printcmd.c b/gdb/printcmd.c
> index 1109cb3046..dce6ab2db9 100644
> --- a/gdb/printcmd.c
> +++ b/gdb/printcmd.c
> @@ -539,7 +539,7 @@ print_address_symbolic (struct gdbarch *gdbarch, CORE_ADDR addr,
> fputs_filtered ("<", stream);
> fputs_styled (name.c_str (), function_name_style.style (), stream);
> if (offset != 0)
> - fprintf_filtered (stream, "+%u", (unsigned int) offset);
> + fprintf_filtered (stream, "%+d", offset);
>
> /* Append source filename and line number if desired. Give specific
> line # of this addr, if we have it; else line # of the nearest symbol. */
> @@ -679,7 +679,7 @@ build_address_symbolic (struct gdbarch *gdbarch,
> && name_location + max_symbolic_offset > name_location)
> return 1;
>
> - *offset = addr - name_location;
> + *offset = (LONGEST) addr - name_location;
>
> *name = name_temp;
>
> --
> 2.21.0
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 0/5] Non-contiguous address range bug fixes / improvements
2019-07-04 4:55 [PATCH v2 0/5] Non-contiguous address range bug fixes / improvements Kevin Buettner
` (4 preceding siblings ...)
2019-07-04 5:05 ` [PATCH v2 4/5] Allow display of negative offsets in print_address_symbolic() Kevin Buettner
@ 2019-07-27 20:48 ` Kevin Buettner
5 siblings, 0 replies; 11+ messages in thread
From: Kevin Buettner @ 2019-07-27 20:48 UTC (permalink / raw)
To: gdb-patches
On Wed, 3 Jul 2019 21:54:58 -0700
Kevin Buettner <kevinb@redhat.com> wrote:
> This five part series fixes some bugs associated with GDB's non-contiguous
> address range support.
I've pushed this series of commits.
Kevin
^ permalink raw reply [flat|nested] 11+ messages in thread