* [PATCH 0/9] Centralize unwinder api exceptions handling
@ 2017-07-31 22:22 Yao Qi
2017-07-31 22:22 ` [PATCH 2/9] Class-fy dwarf2_frame_state Yao Qi
` (9 more replies)
0 siblings, 10 replies; 13+ messages in thread
From: Yao Qi @ 2017-07-31 22:22 UTC (permalink / raw)
To: gdb-patches
This patch series changes GDB to handle unwinder api exceptions in target
independent code, instead of in each unwinder. See more details on patch
#5. Patch 6 ~ 9 lifts the restrictions on each unwinder, that is, each
unwinder has to handle NOT_AVAILABLE_ERROR. Patches 1 ~ 4 are preparatory
ones. They can go in independently.
The patch series is tested on x86_64-linux and aarch64-linux. Note that
rs6000 and s390 unwinders still catch exceptions. I'll change them latter,
as I still need sometime understanding the code.
*** BLURB HERE ***
Yao Qi (9):
Move dwarf2_frame_state_reg.exp_len to union .loc
Class-fy dwarf2_frame_state
Class-fy dwarf2_frame_state_reg_info
Reset *THIS_CACHE in frame_unwind_try_unwinder in case of exception
Handle unwinding exceptions
Throw exception in dwarf2 unwinders
Throw exception in amd64 unwinders
Throw exception in i386 unwinders
Throw exception in aarch64 unwinder
gdb/aarch64-tdep.c | 48 +-------
gdb/amd64-tdep.c | 95 ++++-----------
gdb/dwarf2-frame.c | 331 +++++++++++++++++++++++------------------------------
gdb/dwarf2-frame.h | 114 ++++++++++++++----
gdb/frame-unwind.c | 7 +-
gdb/frame-unwind.h | 3 +-
gdb/frame.c | 109 +++++++++++++++---
gdb/i386-tdep.c | 111 +++++-------------
gdb/sparc-tdep.c | 2 +-
9 files changed, 390 insertions(+), 430 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 6/9] Throw exception in dwarf2 unwinders
2017-07-31 22:22 [PATCH 0/9] Centralize unwinder api exceptions handling Yao Qi
` (5 preceding siblings ...)
2017-07-31 22:22 ` [PATCH 5/9] Handle unwinding exceptions Yao Qi
@ 2017-07-31 22:22 ` Yao Qi
2017-07-31 22:22 ` [PATCH 8/9] Throw exception in i386 unwinders Yao Qi
` (2 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Yao Qi @ 2017-07-31 22:22 UTC (permalink / raw)
To: gdb-patches
This patch changes dwarf2 unwinders to not catch exceptions.
gdb:
2017-07-20 Yao Qi <yao.qi@linaro.org>
* dwarf2-frame.c (struct dwarf2_frame_cache) <unavailable_retaddr>:
Remove.
(dwarf2_frame_cache): Don't catch exception.
(dwarf2_frame_unwind_stop_reason): Don't check
cache->unavailable_retaddr.
(dwarf2_frame_this_id): Likewise.
---
gdb/dwarf2-frame.c | 63 ++++++++++++++++--------------------------------------
1 file changed, 19 insertions(+), 44 deletions(-)
diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index f8e6522..d380e83 100644
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -922,10 +922,6 @@ struct dwarf2_frame_cache
/* DWARF Call Frame Address. */
CORE_ADDR cfa;
- /* Set if the return address column was marked as unavailable
- (required non-collected memory or registers to compute). */
- int unavailable_retaddr;
-
/* Set if the return address column was marked as undefined. */
int undefined_retaddr;
@@ -1037,41 +1033,27 @@ dwarf2_frame_cache (struct frame_info *this_frame, void **this_cache)
execute_cfa_program (fde, instr, fde->end, gdbarch,
get_frame_address_in_block (this_frame), &fs);
- TRY
- {
- /* Calculate the CFA. */
- switch (fs.regs.cfa_how)
- {
- case CFA_REG_OFFSET:
- cache->cfa = read_addr_from_reg (this_frame, fs.regs.cfa_reg);
- if (fs.armcc_cfa_offsets_reversed)
- cache->cfa -= fs.regs.cfa_offset;
- else
- cache->cfa += fs.regs.cfa_offset;
- break;
-
- case CFA_EXP:
- cache->cfa =
- execute_stack_op (fs.regs.cfa_exp, fs.regs.cfa_exp_len,
- cache->addr_size, cache->text_offset,
- this_frame, 0, 0);
- break;
-
- default:
- internal_error (__FILE__, __LINE__, _("Unknown CFA rule."));
- }
- }
- CATCH (ex, RETURN_MASK_ERROR)
+ /* Calculate the CFA. */
+ switch (fs.regs.cfa_how)
{
- if (ex.error == NOT_AVAILABLE_ERROR)
- {
- cache->unavailable_retaddr = 1;
- return cache;
- }
+ case CFA_REG_OFFSET:
+ cache->cfa = read_addr_from_reg (this_frame, fs.regs.cfa_reg);
+ if (fs.armcc_cfa_offsets_reversed)
+ cache->cfa -= fs.regs.cfa_offset;
+ else
+ cache->cfa += fs.regs.cfa_offset;
+ break;
+
+ case CFA_EXP:
+ cache->cfa =
+ execute_stack_op (fs.regs.cfa_exp, fs.regs.cfa_exp_len,
+ cache->addr_size, cache->text_offset,
+ this_frame, 0, 0);
+ break;
- throw_exception (ex);
+ default:
+ internal_error (__FILE__, __LINE__, _("Unknown CFA rule."));
}
- END_CATCH
/* Initialize the register state. */
{
@@ -1181,9 +1163,6 @@ dwarf2_frame_unwind_stop_reason (struct frame_info *this_frame,
struct dwarf2_frame_cache *cache
= dwarf2_frame_cache (this_frame, this_cache);
- if (cache->unavailable_retaddr)
- return UNWIND_UNAVAILABLE;
-
if (cache->undefined_retaddr)
return UNWIND_OUTERMOST;
@@ -1197,11 +1176,7 @@ dwarf2_frame_this_id (struct frame_info *this_frame, void **this_cache,
struct dwarf2_frame_cache *cache =
dwarf2_frame_cache (this_frame, this_cache);
- if (cache->unavailable_retaddr)
- (*this_id) = frame_id_build_unavailable_stack (get_frame_func (this_frame));
- else if (cache->undefined_retaddr)
- return;
- else
+ if (!cache->undefined_retaddr)
(*this_id) = frame_id_build (cache->cfa, get_frame_func (this_frame));
}
--
1.9.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 7/9] Throw exception in amd64 unwinders
2017-07-31 22:22 [PATCH 0/9] Centralize unwinder api exceptions handling Yao Qi
2017-07-31 22:22 ` [PATCH 2/9] Class-fy dwarf2_frame_state Yao Qi
2017-07-31 22:22 ` [PATCH 4/9] Reset *THIS_CACHE in frame_unwind_try_unwinder in case of exception Yao Qi
@ 2017-07-31 22:22 ` Yao Qi
2017-07-31 22:22 ` [PATCH 3/9] Class-fy dwarf2_frame_state_reg_info Yao Qi
` (6 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Yao Qi @ 2017-07-31 22:22 UTC (permalink / raw)
To: gdb-patches
This patch changes amd64 unwinders not to throw exceptions.
gdb:
2017-07-31 Yao Qi <yao.qi@linaro.org>
* amd64-tdep.c (amd64_frame_cache) <base_p>: Remove.
(amd64_init_frame_cache): Update.
(amd64_frame_cache_1): Update.
(amd64_frame_unwind_stop_reason): Don't check cache->base_p.
(amd64_frame_this_id): Likewise.
(amd64_sigtramp_frame_cache): Don't catch exception.
(amd64_sigtramp_frame_unwind_stop_reason): Don't check
cache->base_p.
(amd64_sigtramp_frame_this_id): Likewise.
(amd64_epilogue_frame_cache): Don't catch exception.
(amd64_epilogue_frame_unwind_stop_reason): Don't check
cache->base_p.
(amd64_epilogue_frame_this_id): Likewise.
---
gdb/amd64-tdep.c | 95 +++++++++++++-------------------------------------------
1 file changed, 22 insertions(+), 73 deletions(-)
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index f647402..256147b 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -1876,7 +1876,6 @@ struct amd64_frame_cache
{
/* Base address. */
CORE_ADDR base;
- int base_p;
CORE_ADDR sp_offset;
CORE_ADDR pc;
@@ -1898,7 +1897,6 @@ amd64_init_frame_cache (struct amd64_frame_cache *cache)
/* Base address. */
cache->base = 0;
- cache->base_p = 0;
cache->sp_offset = -8;
cache->pc = 0;
@@ -2531,8 +2529,6 @@ amd64_frame_cache_1 (struct frame_info *this_frame,
for (i = 0; i < AMD64_NUM_SAVED_REGS; i++)
if (cache->saved_regs[i] != -1)
cache->saved_regs[i] += cache->base;
-
- cache->base_p = 1;
}
static struct amd64_frame_cache *
@@ -2546,16 +2542,7 @@ amd64_frame_cache (struct frame_info *this_frame, void **this_cache)
cache = amd64_alloc_frame_cache ();
*this_cache = cache;
- TRY
- {
- amd64_frame_cache_1 (this_frame, cache);
- }
- CATCH (ex, RETURN_MASK_ERROR)
- {
- if (ex.error != NOT_AVAILABLE_ERROR)
- throw_exception (ex);
- }
- END_CATCH
+ amd64_frame_cache_1 (this_frame, cache);
return cache;
}
@@ -2567,9 +2554,6 @@ amd64_frame_unwind_stop_reason (struct frame_info *this_frame,
struct amd64_frame_cache *cache =
amd64_frame_cache (this_frame, this_cache);
- if (!cache->base_p)
- return UNWIND_UNAVAILABLE;
-
/* This marks the outermost frame. */
if (cache->base == 0)
return UNWIND_OUTERMOST;
@@ -2584,9 +2568,7 @@ amd64_frame_this_id (struct frame_info *this_frame, void **this_cache,
struct amd64_frame_cache *cache =
amd64_frame_cache (this_frame, this_cache);
- if (!cache->base_p)
- (*this_id) = frame_id_build_unavailable_stack (cache->pc);
- else if (cache->base == 0)
+ if (cache->base == 0)
{
/* This marks the outermost frame. */
return;
@@ -2664,26 +2646,15 @@ amd64_sigtramp_frame_cache (struct frame_info *this_frame, void **this_cache)
cache = amd64_alloc_frame_cache ();
- TRY
- {
- get_frame_register (this_frame, AMD64_RSP_REGNUM, buf);
- cache->base = extract_unsigned_integer (buf, 8, byte_order) - 8;
-
- addr = tdep->sigcontext_addr (this_frame);
- gdb_assert (tdep->sc_reg_offset);
- gdb_assert (tdep->sc_num_regs <= AMD64_NUM_SAVED_REGS);
- for (i = 0; i < tdep->sc_num_regs; i++)
- if (tdep->sc_reg_offset[i] != -1)
- cache->saved_regs[i] = addr + tdep->sc_reg_offset[i];
+ get_frame_register (this_frame, AMD64_RSP_REGNUM, buf);
+ cache->base = extract_unsigned_integer (buf, 8, byte_order) - 8;
- cache->base_p = 1;
- }
- CATCH (ex, RETURN_MASK_ERROR)
- {
- if (ex.error != NOT_AVAILABLE_ERROR)
- throw_exception (ex);
- }
- END_CATCH
+ addr = tdep->sigcontext_addr (this_frame);
+ gdb_assert (tdep->sc_reg_offset);
+ gdb_assert (tdep->sc_num_regs <= AMD64_NUM_SAVED_REGS);
+ for (i = 0; i < tdep->sc_num_regs; i++)
+ if (tdep->sc_reg_offset[i] != -1)
+ cache->saved_regs[i] = addr + tdep->sc_reg_offset[i];
*this_cache = cache;
return cache;
@@ -2696,9 +2667,6 @@ amd64_sigtramp_frame_unwind_stop_reason (struct frame_info *this_frame,
struct amd64_frame_cache *cache =
amd64_sigtramp_frame_cache (this_frame, this_cache);
- if (!cache->base_p)
- return UNWIND_UNAVAILABLE;
-
return UNWIND_NO_REASON;
}
@@ -2709,9 +2677,7 @@ amd64_sigtramp_frame_this_id (struct frame_info *this_frame,
struct amd64_frame_cache *cache =
amd64_sigtramp_frame_cache (this_frame, this_cache);
- if (!cache->base_p)
- (*this_id) = frame_id_build_unavailable_stack (get_frame_pc (this_frame));
- else if (cache->base == 0)
+ if (cache->base == 0)
{
/* This marks the outermost frame. */
return;
@@ -2841,30 +2807,19 @@ amd64_epilogue_frame_cache (struct frame_info *this_frame, void **this_cache)
cache = amd64_alloc_frame_cache ();
*this_cache = cache;
- TRY
- {
- /* Cache base will be %esp plus cache->sp_offset (-8). */
- get_frame_register (this_frame, AMD64_RSP_REGNUM, buf);
- cache->base = extract_unsigned_integer (buf, 8,
- byte_order) + cache->sp_offset;
+ /* Cache base will be %esp plus cache->sp_offset (-8). */
+ get_frame_register (this_frame, AMD64_RSP_REGNUM, buf);
+ cache->base = extract_unsigned_integer (buf, 8,
+ byte_order) + cache->sp_offset;
- /* Cache pc will be the frame func. */
- cache->pc = get_frame_pc (this_frame);
+ /* Cache pc will be the frame func. */
+ cache->pc = get_frame_pc (this_frame);
- /* The saved %esp will be at cache->base plus 16. */
- cache->saved_sp = cache->base + 16;
+ /* The saved %esp will be at cache->base plus 16. */
+ cache->saved_sp = cache->base + 16;
- /* The saved %eip will be at cache->base plus 8. */
- cache->saved_regs[AMD64_RIP_REGNUM] = cache->base + 8;
-
- cache->base_p = 1;
- }
- CATCH (ex, RETURN_MASK_ERROR)
- {
- if (ex.error != NOT_AVAILABLE_ERROR)
- throw_exception (ex);
- }
- END_CATCH
+ /* The saved %eip will be at cache->base plus 8. */
+ cache->saved_regs[AMD64_RIP_REGNUM] = cache->base + 8;
return cache;
}
@@ -2876,9 +2831,6 @@ amd64_epilogue_frame_unwind_stop_reason (struct frame_info *this_frame,
struct amd64_frame_cache *cache
= amd64_epilogue_frame_cache (this_frame, this_cache);
- if (!cache->base_p)
- return UNWIND_UNAVAILABLE;
-
return UNWIND_NO_REASON;
}
@@ -2890,10 +2842,7 @@ amd64_epilogue_frame_this_id (struct frame_info *this_frame,
struct amd64_frame_cache *cache = amd64_epilogue_frame_cache (this_frame,
this_cache);
- if (!cache->base_p)
- (*this_id) = frame_id_build_unavailable_stack (cache->pc);
- else
- (*this_id) = frame_id_build (cache->base + 8, cache->pc);
+ (*this_id) = frame_id_build (cache->base + 8, cache->pc);
}
static const struct frame_unwind amd64_epilogue_frame_unwind =
--
1.9.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 5/9] Handle unwinding exceptions
2017-07-31 22:22 [PATCH 0/9] Centralize unwinder api exceptions handling Yao Qi
` (4 preceding siblings ...)
2017-07-31 22:22 ` [PATCH 9/9] Throw exception in aarch64 unwinder Yao Qi
@ 2017-07-31 22:22 ` Yao Qi
2017-07-31 22:22 ` [PATCH 6/9] Throw exception in dwarf2 unwinders Yao Qi
` (3 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Yao Qi @ 2017-07-31 22:22 UTC (permalink / raw)
To: gdb-patches
unwinder methods (sniffer, this_id, stop_reason, etc) can throw
exceptions, at least when some data is unavailable during examining
traceframes. We deal with this problem by catching exceptions in each
unwinders, dwarf unwinders and different arch-specific prologue unwinders.
done by https://www.sourceware.org/ml/gdb-patches/2011-02/msg00611.html
This requires each arch-specific prologue unwinder needs to catch
exceptions.
This patch centralizes the exception handling in the callers of unwinders
methods, and the following patches remove the exception handling in each
arch prologue unwinders. This patch is a follow-up to discussion
https://sourceware.org/ml/gdb-patches/2016-02/msg00778.html
This patch not only catches exceptions from unwinder methods, but also
catches exceptions from frame_base methods, because they call
arch-specific prologue analyzer, which can throw exceptions.
gdb:
2017-07-27 Yao Qi <yao.qi@linaro.org>
* frame.c (compute_frame_id): Catch exception, if it is
NOT_AVAILABLE_ERROR, call frame_id_build_unavailable_stack,
otherwise throw the exception again.
(frame_unwind_register_value): Catch exception from
unwind->prev_register, if it is NOT_AVAILABLE_ERROR, set value
unavailable, otherwise throw the exception again.
(get_prev_frame_always_1): Catch exception from
unwind->stop_reason, if it is NOT_AVAILABLE_ERROR, set stop_reason
to UNWIND_UNAVAILABLE, otherwise throw the exception.
(get_frame_locals_address): Catch exception base->this_locals.
(get_frame_args_address): Likewise.
---
gdb/frame.c | 109 +++++++++++++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 93 insertions(+), 16 deletions(-)
diff --git a/gdb/frame.c b/gdb/frame.c
index 30e4aea..f5037d5 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -502,7 +502,32 @@ compute_frame_id (struct frame_info *fi)
/* Find THIS frame's ID. */
/* Default to outermost if no ID is found. */
fi->this_id.value = outer_frame_id;
- fi->unwind->this_id (fi, &fi->prologue_cache, &fi->this_id.value);
+
+ TRY
+ {
+ fi->unwind->this_id (fi, &fi->prologue_cache, &fi->this_id.value);
+ }
+ CATCH (ex, RETURN_MASK_ERROR)
+ {
+ if (ex.error == NOT_AVAILABLE_ERROR)
+ {
+ CORE_ADDR pc;
+
+ /* Use function start address as part of the frame ID. If we
+ can't find the start address (due to missing symbol
+ information), faill back to just using the current PC. If
+ PC isn't available, we can't construct any meaningful frame
+ ID, so do nothing here, and the frame id is
+ outer_frame_id. */
+ if ((get_frame_func_if_available (fi, &pc) && pc != 0)
+ || get_frame_pc_if_available (fi, &pc))
+ fi->this_id.value = frame_id_build_unavailable_stack (pc);
+ }
+ else
+ throw_exception (ex);
+ }
+ END_CATCH
+
gdb_assert (frame_id_p (fi->this_id.value));
fi->this_id.p = 1;
if (frame_debug)
@@ -1196,8 +1221,25 @@ frame_unwind_register_value (struct frame_info *frame, int regnum)
if (frame->unwind == NULL)
frame_unwind_find_by_frame (frame, &frame->prologue_cache);
- /* Ask this frame to unwind its register. */
- value = frame->unwind->prev_register (frame, &frame->prologue_cache, regnum);
+ TRY
+ {
+ /* Ask this frame to unwind its register. */
+ value = frame->unwind->prev_register (frame, &frame->prologue_cache,
+ regnum);
+ }
+ CATCH (ex, RETURN_MASK_ERROR)
+ {
+ if (ex.error == NOT_AVAILABLE_ERROR)
+ {
+ value = frame_unwind_got_register (frame, regnum, regnum);
+ set_value_lazy (value, 0);
+ mark_value_bytes_unavailable (value, 0,
+ TYPE_LENGTH (value_type (value)));
+ }
+ else
+ throw_exception (ex);
+ }
+ END_CATCH
if (frame_debug)
{
@@ -2009,9 +2051,20 @@ get_prev_frame_always_1 (struct frame_info *this_frame)
/* Check that this frame is unwindable. If it isn't, don't try to
unwind to the prev frame. */
- this_frame->stop_reason
- = this_frame->unwind->stop_reason (this_frame,
- &this_frame->prologue_cache);
+ TRY
+ {
+ this_frame->stop_reason
+ = this_frame->unwind->stop_reason (this_frame,
+ &this_frame->prologue_cache);
+ }
+ CATCH (ex, RETURN_MASK_ERROR)
+ {
+ if (ex.error == NOT_AVAILABLE_ERROR)
+ this_frame->stop_reason = UNWIND_UNAVAILABLE;
+ else
+ throw_exception (ex);
+ }
+ END_CATCH
if (this_frame->stop_reason != UNWIND_NO_REASON)
{
@@ -2596,11 +2649,23 @@ get_frame_locals_address (struct frame_info *fi)
/* If there isn't a frame address method, find it. */
if (fi->base == NULL)
fi->base = frame_base_find_by_frame (fi);
- /* Sneaky: If the low-level unwind and high-level base code share a
- common unwinder, let them share the prologue cache. */
- if (fi->base->unwind == fi->unwind)
- return fi->base->this_locals (fi, &fi->prologue_cache);
- return fi->base->this_locals (fi, &fi->base_cache);
+
+ TRY
+ {
+ /* Sneaky: If the low-level unwind and high-level base code share a
+ common unwinder, let them share the prologue cache. */
+ if (fi->base->unwind == fi->unwind)
+ return fi->base->this_locals (fi, &fi->prologue_cache);
+ return fi->base->this_locals (fi, &fi->base_cache);
+ }
+ CATCH (ex, RETURN_MASK_ERROR)
+ {
+ if (ex.error != NOT_AVAILABLE_ERROR)
+ throw_exception (ex);
+ }
+ END_CATCH
+
+ return 0;
}
CORE_ADDR
@@ -2611,11 +2676,23 @@ get_frame_args_address (struct frame_info *fi)
/* If there isn't a frame address method, find it. */
if (fi->base == NULL)
fi->base = frame_base_find_by_frame (fi);
- /* Sneaky: If the low-level unwind and high-level base code share a
- common unwinder, let them share the prologue cache. */
- if (fi->base->unwind == fi->unwind)
- return fi->base->this_args (fi, &fi->prologue_cache);
- return fi->base->this_args (fi, &fi->base_cache);
+
+ TRY
+ {
+ /* Sneaky: If the low-level unwind and high-level base code share a
+ common unwinder, let them share the prologue cache. */
+ if (fi->base->unwind == fi->unwind)
+ return fi->base->this_args (fi, &fi->prologue_cache);
+ return fi->base->this_args (fi, &fi->base_cache);
+ }
+ CATCH (ex, RETURN_MASK_ERROR)
+ {
+ if (ex.error != NOT_AVAILABLE_ERROR)
+ throw_exception (ex);
+ }
+ END_CATCH
+
+ return 0;
}
/* Return true if the frame unwinder for frame FI is UNWINDER; false
--
1.9.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 8/9] Throw exception in i386 unwinders
2017-07-31 22:22 [PATCH 0/9] Centralize unwinder api exceptions handling Yao Qi
` (6 preceding siblings ...)
2017-07-31 22:22 ` [PATCH 6/9] Throw exception in dwarf2 unwinders Yao Qi
@ 2017-07-31 22:22 ` Yao Qi
2017-07-31 22:22 ` [PATCH 1/9] Move dwarf2_frame_state_reg.exp_len to union .loc Yao Qi
2017-08-11 8:35 ` [PATCH 0/9] Centralize unwinder api exceptions handling Yao Qi
9 siblings, 0 replies; 13+ messages in thread
From: Yao Qi @ 2017-07-31 22:22 UTC (permalink / raw)
To: gdb-patches
This patch changes i386 unwinders not to throw exceptions.
gdb:
2017-07-31 Yao Qi <yao.qi@linaro.org>
* i386-tdep.c (i386_frame_cache) <base_p>: Remove.
(i386_alloc_frame_cache): Update.
(i386_frame_cache_1): Likewise.
(i386_frame_cache): Don't catch exceptions.
(i386_frame_this_id): Don't check cache->base_p.
(i386_frame_unwind_stop_reason): Likewise.
(i386_epilogue_frame_cache): Don't catch exceptions.
(i386_epilogue_frame_unwind_stop_reason): Don't check
cache->base_p.
(i386_epilogue_frame_this_id): Likewise.
(i386_sigtramp_frame_cache): Don't catch exceptions.
(i386_sigtramp_frame_unwind_stop_reason): Don't check
cache->base_p.
(i386_sigtramp_frame_this_id): Likewise.
---
gdb/i386-tdep.c | 111 ++++++++++++++------------------------------------------
1 file changed, 27 insertions(+), 84 deletions(-)
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index bd728f0..c09130e 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -1074,7 +1074,6 @@ struct i386_frame_cache
{
/* Base address. */
CORE_ADDR base;
- int base_p;
LONGEST sp_offset;
CORE_ADDR pc;
@@ -1099,7 +1098,6 @@ i386_alloc_frame_cache (void)
cache = FRAME_OBSTACK_ZALLOC (struct i386_frame_cache);
/* Base address. */
- cache->base_p = 0;
cache->base = 0;
cache->sp_offset = -4;
cache->pc = 0;
@@ -1997,10 +1995,7 @@ i386_frame_cache_1 (struct frame_info *this_frame,
get_frame_register (this_frame, I386_EBP_REGNUM, buf);
cache->base = extract_unsigned_integer (buf, 4, byte_order);
if (cache->base == 0)
- {
- cache->base_p = 1;
- return;
- }
+ return;
/* For normal frames, %eip is stored at 4(%ebp). */
cache->saved_regs[I386_EIP_REGNUM] = 4;
@@ -2071,8 +2066,6 @@ i386_frame_cache_1 (struct frame_info *this_frame,
for (i = 0; i < I386_NUM_SAVED_REGS; i++)
if (cache->saved_regs[i] != -1)
cache->saved_regs[i] += cache->base;
-
- cache->base_p = 1;
}
static struct i386_frame_cache *
@@ -2086,16 +2079,7 @@ i386_frame_cache (struct frame_info *this_frame, void **this_cache)
cache = i386_alloc_frame_cache ();
*this_cache = cache;
- TRY
- {
- i386_frame_cache_1 (this_frame, cache);
- }
- CATCH (ex, RETURN_MASK_ERROR)
- {
- if (ex.error != NOT_AVAILABLE_ERROR)
- throw_exception (ex);
- }
- END_CATCH
+ i386_frame_cache_1 (this_frame, cache);
return cache;
}
@@ -2106,9 +2090,7 @@ i386_frame_this_id (struct frame_info *this_frame, void **this_cache,
{
struct i386_frame_cache *cache = i386_frame_cache (this_frame, this_cache);
- if (!cache->base_p)
- (*this_id) = frame_id_build_unavailable_stack (cache->pc);
- else if (cache->base == 0)
+ if (cache->base == 0)
{
/* This marks the outermost frame. */
}
@@ -2125,9 +2107,6 @@ i386_frame_unwind_stop_reason (struct frame_info *this_frame,
{
struct i386_frame_cache *cache = i386_frame_cache (this_frame, this_cache);
- if (!cache->base_p)
- return UNWIND_UNAVAILABLE;
-
/* This marks the outermost frame. */
if (cache->base == 0)
return UNWIND_OUTERMOST;
@@ -2256,26 +2235,15 @@ i386_epilogue_frame_cache (struct frame_info *this_frame, void **this_cache)
cache = i386_alloc_frame_cache ();
*this_cache = cache;
- TRY
- {
- cache->pc = get_frame_func (this_frame);
+ cache->pc = get_frame_func (this_frame);
- /* At this point the stack looks as if we just entered the
- function, with the return address at the top of the
- stack. */
- sp = get_frame_register_unsigned (this_frame, I386_ESP_REGNUM);
- cache->base = sp + cache->sp_offset;
- cache->saved_sp = cache->base + 8;
- cache->saved_regs[I386_EIP_REGNUM] = cache->base + 4;
-
- cache->base_p = 1;
- }
- CATCH (ex, RETURN_MASK_ERROR)
- {
- if (ex.error != NOT_AVAILABLE_ERROR)
- throw_exception (ex);
- }
- END_CATCH
+ /* At this point the stack looks as if we just entered the
+ function, with the return address at the top of the
+ stack. */
+ sp = get_frame_register_unsigned (this_frame, I386_ESP_REGNUM);
+ cache->base = sp + cache->sp_offset;
+ cache->saved_sp = cache->base + 8;
+ cache->saved_regs[I386_EIP_REGNUM] = cache->base + 4;
return cache;
}
@@ -2287,9 +2255,6 @@ i386_epilogue_frame_unwind_stop_reason (struct frame_info *this_frame,
struct i386_frame_cache *cache =
i386_epilogue_frame_cache (this_frame, this_cache);
- if (!cache->base_p)
- return UNWIND_UNAVAILABLE;
-
return UNWIND_NO_REASON;
}
@@ -2301,10 +2266,7 @@ i386_epilogue_frame_this_id (struct frame_info *this_frame,
struct i386_frame_cache *cache =
i386_epilogue_frame_cache (this_frame, this_cache);
- if (!cache->base_p)
- (*this_id) = frame_id_build_unavailable_stack (cache->pc);
- else
- (*this_id) = frame_id_build (cache->base + 8, cache->pc);
+ (*this_id) = frame_id_build (cache->base + 8, cache->pc);
}
static struct value *
@@ -2442,36 +2404,25 @@ i386_sigtramp_frame_cache (struct frame_info *this_frame, void **this_cache)
cache = i386_alloc_frame_cache ();
- TRY
- {
- get_frame_register (this_frame, I386_ESP_REGNUM, buf);
- cache->base = extract_unsigned_integer (buf, 4, byte_order) - 4;
+ get_frame_register (this_frame, I386_ESP_REGNUM, buf);
+ cache->base = extract_unsigned_integer (buf, 4, byte_order) - 4;
- addr = tdep->sigcontext_addr (this_frame);
- if (tdep->sc_reg_offset)
- {
- int i;
+ addr = tdep->sigcontext_addr (this_frame);
+ if (tdep->sc_reg_offset)
+ {
+ int i;
- gdb_assert (tdep->sc_num_regs <= I386_NUM_SAVED_REGS);
+ gdb_assert (tdep->sc_num_regs <= I386_NUM_SAVED_REGS);
- for (i = 0; i < tdep->sc_num_regs; i++)
- if (tdep->sc_reg_offset[i] != -1)
- cache->saved_regs[i] = addr + tdep->sc_reg_offset[i];
- }
- else
- {
- cache->saved_regs[I386_EIP_REGNUM] = addr + tdep->sc_pc_offset;
- cache->saved_regs[I386_ESP_REGNUM] = addr + tdep->sc_sp_offset;
- }
-
- cache->base_p = 1;
+ for (i = 0; i < tdep->sc_num_regs; i++)
+ if (tdep->sc_reg_offset[i] != -1)
+ cache->saved_regs[i] = addr + tdep->sc_reg_offset[i];
}
- CATCH (ex, RETURN_MASK_ERROR)
+ else
{
- if (ex.error != NOT_AVAILABLE_ERROR)
- throw_exception (ex);
+ cache->saved_regs[I386_EIP_REGNUM] = addr + tdep->sc_pc_offset;
+ cache->saved_regs[I386_ESP_REGNUM] = addr + tdep->sc_sp_offset;
}
- END_CATCH
*this_cache = cache;
return cache;
@@ -2484,9 +2435,6 @@ i386_sigtramp_frame_unwind_stop_reason (struct frame_info *this_frame,
struct i386_frame_cache *cache =
i386_sigtramp_frame_cache (this_frame, this_cache);
- if (!cache->base_p)
- return UNWIND_UNAVAILABLE;
-
return UNWIND_NO_REASON;
}
@@ -2497,13 +2445,8 @@ i386_sigtramp_frame_this_id (struct frame_info *this_frame, void **this_cache,
struct i386_frame_cache *cache =
i386_sigtramp_frame_cache (this_frame, this_cache);
- if (!cache->base_p)
- (*this_id) = frame_id_build_unavailable_stack (get_frame_pc (this_frame));
- else
- {
- /* See the end of i386_push_dummy_call. */
- (*this_id) = frame_id_build (cache->base + 8, get_frame_pc (this_frame));
- }
+ /* See the end of i386_push_dummy_call. */
+ (*this_id) = frame_id_build (cache->base + 8, get_frame_pc (this_frame));
}
static struct value *
--
1.9.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/9] Class-fy dwarf2_frame_state
2017-07-31 22:22 [PATCH 0/9] Centralize unwinder api exceptions handling Yao Qi
@ 2017-07-31 22:22 ` Yao Qi
2017-07-31 22:22 ` [PATCH 4/9] Reset *THIS_CACHE in frame_unwind_try_unwinder in case of exception Yao Qi
` (8 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Yao Qi @ 2017-07-31 22:22 UTC (permalink / raw)
To: gdb-patches
This patch adds ctor and dtor to dwarf2_frame_state, so that we can
remove one cleanup "old_chain".
gdb:
2017-07-20 Yao Qi <yao.qi@linaro.org>
* dwarf2-frame.c (dwarf2_frame_state_free): Remove.
(dwarf2_frame_state::dwarf2_frame_state): New.
(dwarf2_frame_state::~dwarf2_frame_state): New.
(dwarf2_fetch_cfa_info): Update.
(dwarf2_frame_cache): Remove old_chain. Change 'fs' to an object
rather than a pointer. Update code.
* dwarf2-frame.h (struct dwarf2_frame_state): Declare ctor and
dtor.
<data_align, code_align, retaddr_column>: Change them to const.
<armcc_cfa_offsets_sf, armcc_cfa_offsets_reversed>: Change them
to bool.
---
gdb/dwarf2-frame.c | 104 +++++++++++++++++++++++------------------------------
gdb/dwarf2-frame.h | 19 ++++++----
2 files changed, 57 insertions(+), 66 deletions(-)
diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index 191f60a..486d60b 100644
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -218,18 +218,19 @@ dwarf2_frame_state_free_regs (struct dwarf2_frame_state_reg_info *rs)
}
}
-/* Release the memory allocated to the frame state FS. */
-
-static void
-dwarf2_frame_state_free (void *p)
+dwarf2_frame_state::dwarf2_frame_state (CORE_ADDR pc_, struct dwarf2_cie *cie)
+ : pc (pc_), data_align (cie->data_alignment_factor),
+ code_align (cie->code_alignment_factor),
+ retaddr_column (cie->return_address_register)
{
- struct dwarf2_frame_state *fs = (struct dwarf2_frame_state *) p;
+}
- dwarf2_frame_state_free_regs (fs->initial.prev);
- dwarf2_frame_state_free_regs (fs->regs.prev);
- xfree (fs->initial.reg);
- xfree (fs->regs.reg);
- xfree (fs);
+dwarf2_frame_state::~dwarf2_frame_state ()
+{
+ dwarf2_frame_state_free_regs (initial.prev);
+ dwarf2_frame_state_free_regs (regs.prev);
+ xfree (initial.reg);
+ xfree (regs.reg);
}
\f
@@ -865,21 +866,14 @@ dwarf2_fetch_cfa_info (struct gdbarch *gdbarch, CORE_ADDR pc,
{
struct dwarf2_fde *fde;
CORE_ADDR text_offset;
- struct dwarf2_frame_state fs;
-
- memset (&fs, 0, sizeof (struct dwarf2_frame_state));
-
- fs.pc = pc;
+ CORE_ADDR pc1 = pc;
/* Find the correct FDE. */
- fde = dwarf2_frame_find_fde (&fs.pc, &text_offset);
+ fde = dwarf2_frame_find_fde (&pc1, &text_offset);
if (fde == NULL)
error (_("Could not compute CFA; needed to translate this expression"));
- /* Extract any interesting information from the CIE. */
- fs.data_align = fde->cie->data_alignment_factor;
- fs.code_align = fde->cie->code_alignment_factor;
- fs.retaddr_column = fde->cie->return_address_register;
+ dwarf2_frame_state fs (pc1, fde->cie);
/* Check for "quirks" - known bugs in producers. */
dwarf2_frame_find_quirks (&fs, fde);
@@ -978,12 +972,11 @@ clear_pointer_cleanup (void *arg)
static struct dwarf2_frame_cache *
dwarf2_frame_cache (struct frame_info *this_frame, void **this_cache)
{
- struct cleanup *reset_cache_cleanup, *old_chain;
+ struct cleanup *reset_cache_cleanup;
struct gdbarch *gdbarch = get_frame_arch (this_frame);
const int num_regs = gdbarch_num_regs (gdbarch)
+ gdbarch_num_pseudo_regs (gdbarch);
struct dwarf2_frame_cache *cache;
- struct dwarf2_frame_state *fs;
struct dwarf2_fde *fde;
CORE_ADDR entry_pc;
const gdb_byte *instr;
@@ -997,10 +990,6 @@ dwarf2_frame_cache (struct frame_info *this_frame, void **this_cache)
*this_cache = cache;
reset_cache_cleanup = make_cleanup (clear_pointer_cleanup, this_cache);
- /* Allocate and initialize the frame state. */
- fs = XCNEW (struct dwarf2_frame_state);
- old_chain = make_cleanup (dwarf2_frame_state_free, fs);
-
/* Unwind the PC.
Note that if the next frame is never supposed to return (i.e. a call
@@ -1016,41 +1005,40 @@ dwarf2_frame_cache (struct frame_info *this_frame, void **this_cache)
get_frame_address_in_block does just this. It's not clear how
reliable the method is though; there is the potential for the
register state pre-call being different to that on return. */
- fs->pc = get_frame_address_in_block (this_frame);
+ CORE_ADDR pc1 = get_frame_address_in_block (this_frame);
/* Find the correct FDE. */
- fde = dwarf2_frame_find_fde (&fs->pc, &cache->text_offset);
+ fde = dwarf2_frame_find_fde (&pc1, &cache->text_offset);
gdb_assert (fde != NULL);
- /* Extract any interesting information from the CIE. */
- fs->data_align = fde->cie->data_alignment_factor;
- fs->code_align = fde->cie->code_alignment_factor;
- fs->retaddr_column = fde->cie->return_address_register;
+ /* Allocate and initialize the frame state. */
+ struct dwarf2_frame_state fs (pc1, fde->cie);
+
cache->addr_size = fde->cie->addr_size;
/* Check for "quirks" - known bugs in producers. */
- dwarf2_frame_find_quirks (fs, fde);
+ dwarf2_frame_find_quirks (&fs, fde);
/* First decode all the insns in the CIE. */
execute_cfa_program (fde, fde->cie->initial_instructions,
fde->cie->end, gdbarch,
- get_frame_address_in_block (this_frame), fs);
+ get_frame_address_in_block (this_frame), &fs);
/* Save the initialized register set. */
- fs->initial = fs->regs;
- fs->initial.reg = dwarf2_frame_state_copy_regs (&fs->regs);
+ fs.initial = fs.regs;
+ fs.initial.reg = dwarf2_frame_state_copy_regs (&fs.regs);
if (get_frame_func_if_available (this_frame, &entry_pc))
{
/* Decode the insns in the FDE up to the entry PC. */
instr = execute_cfa_program (fde, fde->instructions, fde->end, gdbarch,
- entry_pc, fs);
+ entry_pc, &fs);
- if (fs->regs.cfa_how == CFA_REG_OFFSET
- && (dwarf_reg_to_regnum (gdbarch, fs->regs.cfa_reg)
+ if (fs.regs.cfa_how == CFA_REG_OFFSET
+ && (dwarf_reg_to_regnum (gdbarch, fs.regs.cfa_reg)
== gdbarch_sp_regnum (gdbarch)))
{
- cache->entry_cfa_sp_offset = fs->regs.cfa_offset;
+ cache->entry_cfa_sp_offset = fs.regs.cfa_offset;
cache->entry_cfa_sp_offset_p = 1;
}
}
@@ -1059,24 +1047,24 @@ dwarf2_frame_cache (struct frame_info *this_frame, void **this_cache)
/* Then decode the insns in the FDE up to our target PC. */
execute_cfa_program (fde, instr, fde->end, gdbarch,
- get_frame_address_in_block (this_frame), fs);
+ get_frame_address_in_block (this_frame), &fs);
TRY
{
/* Calculate the CFA. */
- switch (fs->regs.cfa_how)
+ switch (fs.regs.cfa_how)
{
case CFA_REG_OFFSET:
- cache->cfa = read_addr_from_reg (this_frame, fs->regs.cfa_reg);
- if (fs->armcc_cfa_offsets_reversed)
- cache->cfa -= fs->regs.cfa_offset;
+ cache->cfa = read_addr_from_reg (this_frame, fs.regs.cfa_reg);
+ if (fs.armcc_cfa_offsets_reversed)
+ cache->cfa -= fs.regs.cfa_offset;
else
- cache->cfa += fs->regs.cfa_offset;
+ cache->cfa += fs.regs.cfa_offset;
break;
case CFA_EXP:
cache->cfa =
- execute_stack_op (fs->regs.cfa_exp, fs->regs.cfa_exp_len,
+ execute_stack_op (fs.regs.cfa_exp, fs.regs.cfa_exp_len,
cache->addr_size, cache->text_offset,
this_frame, 0, 0);
break;
@@ -1090,7 +1078,6 @@ dwarf2_frame_cache (struct frame_info *this_frame, void **this_cache)
if (ex.error == NOT_AVAILABLE_ERROR)
{
cache->unavailable_retaddr = 1;
- do_cleanups (old_chain);
discard_cleanups (reset_cache_cleanup);
return cache;
}
@@ -1114,7 +1101,7 @@ dwarf2_frame_cache (struct frame_info *this_frame, void **this_cache)
{
int column; /* CFI speak for "register number". */
- for (column = 0; column < fs->regs.num_regs; column++)
+ for (column = 0; column < fs.regs.num_regs; column++)
{
/* Use the GDB register number as the destination index. */
int regnum = dwarf_reg_to_regnum (gdbarch, column);
@@ -1134,16 +1121,16 @@ dwarf2_frame_cache (struct frame_info *this_frame, void **this_cache)
problems when a debug info register falls outside of the
table. We need a way of iterating through all the valid
DWARF2 register numbers. */
- if (fs->regs.reg[column].how == DWARF2_FRAME_REG_UNSPECIFIED)
+ if (fs.regs.reg[column].how == DWARF2_FRAME_REG_UNSPECIFIED)
{
if (cache->reg[regnum].how == DWARF2_FRAME_REG_UNSPECIFIED)
complaint (&symfile_complaints, _("\
incomplete CFI data; unspecified registers (e.g., %s) at %s"),
gdbarch_register_name (gdbarch, regnum),
- paddress (gdbarch, fs->pc));
+ paddress (gdbarch, fs.pc));
}
else
- cache->reg[regnum] = fs->regs.reg[column];
+ cache->reg[regnum] = fs.regs.reg[column];
}
}
@@ -1158,7 +1145,7 @@ incomplete CFI data; unspecified registers (e.g., %s) at %s"),
|| cache->reg[regnum].how == DWARF2_FRAME_REG_RA_OFFSET)
{
struct dwarf2_frame_state_reg *retaddr_reg =
- &fs->regs.reg[fs->retaddr_column];
+ &fs.regs.reg[fs.retaddr_column];
/* It seems rather bizarre to specify an "empty" column as
the return adress column. However, this is exactly
@@ -1167,7 +1154,7 @@ incomplete CFI data; unspecified registers (e.g., %s) at %s"),
register corresponding to the return address column.
Incidentally, that's how we should treat a return
address column specifying "same value" too. */
- if (fs->retaddr_column < fs->regs.num_regs
+ if (fs.retaddr_column < fs.regs.num_regs
&& retaddr_reg->how != DWARF2_FRAME_REG_UNSPECIFIED
&& retaddr_reg->how != DWARF2_FRAME_REG_SAME_VALUE)
{
@@ -1180,12 +1167,12 @@ incomplete CFI data; unspecified registers (e.g., %s) at %s"),
{
if (cache->reg[regnum].how == DWARF2_FRAME_REG_RA)
{
- cache->reg[regnum].loc.reg = fs->retaddr_column;
+ cache->reg[regnum].loc.reg = fs.retaddr_column;
cache->reg[regnum].how = DWARF2_FRAME_REG_SAVED_REG;
}
else
{
- cache->retaddr_reg.loc.reg = fs->retaddr_column;
+ cache->retaddr_reg.loc.reg = fs.retaddr_column;
cache->retaddr_reg.how = DWARF2_FRAME_REG_SAVED_REG;
}
}
@@ -1193,11 +1180,10 @@ incomplete CFI data; unspecified registers (e.g., %s) at %s"),
}
}
- if (fs->retaddr_column < fs->regs.num_regs
- && fs->regs.reg[fs->retaddr_column].how == DWARF2_FRAME_REG_UNDEFINED)
+ if (fs.retaddr_column < fs.regs.num_regs
+ && fs.regs.reg[fs.retaddr_column].how == DWARF2_FRAME_REG_UNDEFINED)
cache->undefined_retaddr = 1;
- do_cleanups (old_chain);
discard_cleanups (reset_cache_cleanup);
return cache;
}
diff --git a/gdb/dwarf2-frame.h b/gdb/dwarf2-frame.h
index 69a7a20..c952a20 100644
--- a/gdb/dwarf2-frame.h
+++ b/gdb/dwarf2-frame.h
@@ -106,35 +106,40 @@ struct dwarf2_frame_state_reg_info
struct dwarf2_frame_state_reg_info *prev;
};
+struct dwarf2_cie;
+
/* Structure describing a frame state. */
struct dwarf2_frame_state
{
+ dwarf2_frame_state (CORE_ADDR pc, struct dwarf2_cie *cie);
+ ~dwarf2_frame_state ();
+
/* Each register save state can be described in terms of a CFA slot,
another register, or a location expression. */
- struct dwarf2_frame_state_reg_info regs;
+ struct dwarf2_frame_state_reg_info regs {};
/* The PC described by the current frame state. */
CORE_ADDR pc;
/* Initial register set from the CIE.
Used to implement DW_CFA_restore. */
- struct dwarf2_frame_state_reg_info initial;
+ struct dwarf2_frame_state_reg_info initial {};
/* The information we care about from the CIE. */
- LONGEST data_align;
- ULONGEST code_align;
- ULONGEST retaddr_column;
+ const LONGEST data_align;
+ const ULONGEST code_align;
+ const ULONGEST retaddr_column;
/* Flags for known producer quirks. */
/* The ARM compilers, in DWARF2 mode, assume that DW_CFA_def_cfa
and DW_CFA_def_cfa_offset takes a factored offset. */
- int armcc_cfa_offsets_sf;
+ bool armcc_cfa_offsets_sf = false;
/* The ARM compilers, in DWARF2 or DWARF3 mode, may assume that
the CFA is defined as REG - OFFSET rather than REG + OFFSET. */
- int armcc_cfa_offsets_reversed;
+ bool armcc_cfa_offsets_reversed = false;
};
/* Set the architecture-specific register state initialization
--
1.9.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/9] Move dwarf2_frame_state_reg.exp_len to union .loc
2017-07-31 22:22 [PATCH 0/9] Centralize unwinder api exceptions handling Yao Qi
` (7 preceding siblings ...)
2017-07-31 22:22 ` [PATCH 8/9] Throw exception in i386 unwinders Yao Qi
@ 2017-07-31 22:22 ` Yao Qi
2017-08-11 8:35 ` [PATCH 0/9] Centralize unwinder api exceptions handling Yao Qi
9 siblings, 0 replies; 13+ messages in thread
From: Yao Qi @ 2017-07-31 22:22 UTC (permalink / raw)
To: gdb-patches
dwarf2_frame_state_reg.exp_len is only used together with .loc.exp, so
it makes more sense to exp_len to the union as well.
gdb:
2017-07-24 Yao Qi <yao.qi@linaro.org>
* dwarf2-frame.h (struct dwarf2_frame_state_reg) <exp_len>: Remove.
<loc.exp>: New field.
* dwarf2-frame.c (execute_cfa_program): Update.
(dwarf2_frame_prev_register): Update.
---
gdb/dwarf2-frame.c | 16 ++++++++--------
gdb/dwarf2-frame.h | 7 +++++--
2 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index d7f985b..191f60a 100644
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -561,8 +561,8 @@ bad CFI data; mismatched DW_CFA_restore_state at %s"),
reg = dwarf2_frame_adjust_regnum (gdbarch, reg, eh_frame_p);
dwarf2_frame_state_alloc_regs (&fs->regs, reg + 1);
insn_ptr = safe_read_uleb128 (insn_ptr, insn_end, &utmp);
- fs->regs.reg[reg].loc.exp = insn_ptr;
- fs->regs.reg[reg].exp_len = utmp;
+ fs->regs.reg[reg].loc.exp.start = insn_ptr;
+ fs->regs.reg[reg].loc.exp.len = utmp;
fs->regs.reg[reg].how = DWARF2_FRAME_REG_SAVED_EXP;
insn_ptr += utmp;
break;
@@ -599,8 +599,8 @@ bad CFI data; mismatched DW_CFA_restore_state at %s"),
insn_ptr = safe_read_uleb128 (insn_ptr, insn_end, ®);
dwarf2_frame_state_alloc_regs (&fs->regs, reg + 1);
insn_ptr = safe_read_uleb128 (insn_ptr, insn_end, &utmp);
- fs->regs.reg[reg].loc.exp = insn_ptr;
- fs->regs.reg[reg].exp_len = utmp;
+ fs->regs.reg[reg].loc.exp.start = insn_ptr;
+ fs->regs.reg[reg].loc.exp.len = utmp;
fs->regs.reg[reg].how = DWARF2_FRAME_REG_SAVED_VAL_EXP;
insn_ptr += utmp;
break;
@@ -1286,8 +1286,8 @@ dwarf2_frame_prev_register (struct frame_info *this_frame, void **this_cache,
return frame_unwind_got_register (this_frame, regnum, realnum);
case DWARF2_FRAME_REG_SAVED_EXP:
- addr = execute_stack_op (cache->reg[regnum].loc.exp,
- cache->reg[regnum].exp_len,
+ addr = execute_stack_op (cache->reg[regnum].loc.exp.start,
+ cache->reg[regnum].loc.exp.len,
cache->addr_size, cache->text_offset,
this_frame, cache->cfa, 1);
return frame_unwind_got_memory (this_frame, regnum, addr);
@@ -1297,8 +1297,8 @@ dwarf2_frame_prev_register (struct frame_info *this_frame, void **this_cache,
return frame_unwind_got_constant (this_frame, regnum, addr);
case DWARF2_FRAME_REG_SAVED_VAL_EXP:
- addr = execute_stack_op (cache->reg[regnum].loc.exp,
- cache->reg[regnum].exp_len,
+ addr = execute_stack_op (cache->reg[regnum].loc.exp.start,
+ cache->reg[regnum].loc.exp.len,
cache->addr_size, cache->text_offset,
this_frame, cache->cfa, 1);
return frame_unwind_got_constant (this_frame, regnum, addr);
diff --git a/gdb/dwarf2-frame.h b/gdb/dwarf2-frame.h
index 9e8668e..69a7a20 100644
--- a/gdb/dwarf2-frame.h
+++ b/gdb/dwarf2-frame.h
@@ -74,11 +74,14 @@ struct dwarf2_frame_state_reg
union {
LONGEST offset;
ULONGEST reg;
- const gdb_byte *exp;
+ struct
+ {
+ const gdb_byte *start;
+ ULONGEST len;
+ } exp;
struct value *(*fn) (struct frame_info *this_frame, void **this_cache,
int regnum);
} loc;
- ULONGEST exp_len;
enum dwarf2_frame_reg_rule how;
};
--
1.9.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/9] Class-fy dwarf2_frame_state_reg_info
2017-07-31 22:22 [PATCH 0/9] Centralize unwinder api exceptions handling Yao Qi
` (2 preceding siblings ...)
2017-07-31 22:22 ` [PATCH 7/9] Throw exception in amd64 unwinders Yao Qi
@ 2017-07-31 22:22 ` Yao Qi
2017-07-31 22:22 ` [PATCH 9/9] Throw exception in aarch64 unwinder Yao Qi
` (5 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Yao Qi @ 2017-07-31 22:22 UTC (permalink / raw)
To: gdb-patches
This patch adds dwarf2_frame_state_reg_info ctor, dtor, copy ctor,
assignment operator, and move assignment. This patch also adds unit test
to execute_cfa_program to cover the changes.
gdb:
2017-07-21 Yao Qi <yao.qi@linaro.org>
* dwarf2-frame.c (dwarf2_frame_state_alloc_regs): Remove.
(dwarf2_frame_state_copy_regs): Remove.
(dwarf2_frame_state_free_regs): Remove.
(dwarf2_frame_state::~dwarf2_frame_state): Remove.
(dwarf2_restore_rule): Call method .alloc_regs instead of
dwarf2_frame_state_alloc_regs.
(execute_cfa_program): Likewise. Call dwarf2_frame_state_reg_info
constructor. Call std::move.
(dwarf2_fetch_cfa_info): Don't call dwarf2_frame_state_copy_regs.
(dwarf2_frame_cache): Likewise.
[GDB_SELF_TEST]: Include selftest.h and
selftest-arch.h.
[GDB_SELF_TEST] (execute_cfa_program_test): New function.
(_initialize_dwarf2_frame) [GDB_SELF_TEST]: Register
execute_cfa_program_test.
* dwarf2-frame.h (dwarf2_frame_state_reg_info): Add ctor, dtor,
copy ctor, assignment operator, move assignment.
<alloc_regs>: New method.
<swap>: New method.
(struct dwarf2_frame_state): Delete dtor.
(dwarf2_frame_state_alloc_regs): Remove declaration.
* sparc-tdep.c (sparc_execute_dwarf_cfa_vendor_op): Don't call
dwarf2_frame_state_alloc_regs, use .alloc_regs instead.
---
gdb/dwarf2-frame.c | 164 +++++++++++++++++++++++++++--------------------------
gdb/dwarf2-frame.h | 90 ++++++++++++++++++++++++-----
gdb/sparc-tdep.c | 2 +-
3 files changed, 161 insertions(+), 95 deletions(-)
diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index 486d60b..0cb40d9 100644
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -38,6 +38,10 @@
#include "ax.h"
#include "dwarf2loc.h"
#include "dwarf2-frame-tailcall.h"
+#if GDB_SELF_TEST
+#include "selftest.h"
+#include "selftest-arch.h"
+#endif
struct comp_unit;
@@ -169,69 +173,12 @@ static CORE_ADDR read_encoded_value (struct comp_unit *unit, gdb_byte encoding,
which is unused in that case. */
#define cfa_exp_len cfa_reg
-/* Assert that the register set RS is large enough to store gdbarch_num_regs
- columns. If necessary, enlarge the register set. */
-
-void
-dwarf2_frame_state_alloc_regs (struct dwarf2_frame_state_reg_info *rs,
- int num_regs)
-{
- size_t size = sizeof (struct dwarf2_frame_state_reg);
-
- if (num_regs <= rs->num_regs)
- return;
-
- rs->reg = (struct dwarf2_frame_state_reg *)
- xrealloc (rs->reg, num_regs * size);
-
- /* Initialize newly allocated registers. */
- memset (rs->reg + rs->num_regs, 0, (num_regs - rs->num_regs) * size);
- rs->num_regs = num_regs;
-}
-
-/* Copy the register columns in register set RS into newly allocated
- memory and return a pointer to this newly created copy. */
-
-static struct dwarf2_frame_state_reg *
-dwarf2_frame_state_copy_regs (struct dwarf2_frame_state_reg_info *rs)
-{
- size_t size = rs->num_regs * sizeof (struct dwarf2_frame_state_reg);
- struct dwarf2_frame_state_reg *reg;
-
- reg = (struct dwarf2_frame_state_reg *) xmalloc (size);
- memcpy (reg, rs->reg, size);
-
- return reg;
-}
-
-/* Release the memory allocated to register set RS. */
-
-static void
-dwarf2_frame_state_free_regs (struct dwarf2_frame_state_reg_info *rs)
-{
- if (rs)
- {
- dwarf2_frame_state_free_regs (rs->prev);
-
- xfree (rs->reg);
- xfree (rs);
- }
-}
-
dwarf2_frame_state::dwarf2_frame_state (CORE_ADDR pc_, struct dwarf2_cie *cie)
: pc (pc_), data_align (cie->data_alignment_factor),
code_align (cie->code_alignment_factor),
retaddr_column (cie->return_address_register)
{
}
-
-dwarf2_frame_state::~dwarf2_frame_state ()
-{
- dwarf2_frame_state_free_regs (initial.prev);
- dwarf2_frame_state_free_regs (regs.prev);
- xfree (initial.reg);
- xfree (regs.reg);
-}
\f
/* Helper functions for execute_stack_op. */
@@ -255,7 +202,7 @@ dwarf2_restore_rule (struct gdbarch *gdbarch, ULONGEST reg_num,
gdb_assert (fs->initial.reg);
reg = dwarf2_frame_adjust_regnum (gdbarch, reg_num, eh_frame_p);
- dwarf2_frame_state_alloc_regs (&fs->regs, reg + 1);
+ fs->regs.alloc_regs (reg + 1);
/* Check if this register was explicitly initialized in the
CIE initial instructions. If not, default the rule to
@@ -409,7 +356,7 @@ execute_cfa_program (struct dwarf2_fde *fde, const gdb_byte *insn_ptr,
reg = dwarf2_frame_adjust_regnum (gdbarch, reg, eh_frame_p);
insn_ptr = safe_read_uleb128 (insn_ptr, insn_end, &utmp);
offset = utmp * fs->data_align;
- dwarf2_frame_state_alloc_regs (&fs->regs, reg + 1);
+ fs->regs.alloc_regs (reg + 1);
fs->regs.reg[reg].how = DWARF2_FRAME_REG_SAVED_OFFSET;
fs->regs.reg[reg].loc.offset = offset;
}
@@ -453,7 +400,7 @@ execute_cfa_program (struct dwarf2_fde *fde, const gdb_byte *insn_ptr,
reg = dwarf2_frame_adjust_regnum (gdbarch, reg, eh_frame_p);
insn_ptr = safe_read_uleb128 (insn_ptr, insn_end, &utmp);
offset = utmp * fs->data_align;
- dwarf2_frame_state_alloc_regs (&fs->regs, reg + 1);
+ fs->regs.alloc_regs (reg + 1);
fs->regs.reg[reg].how = DWARF2_FRAME_REG_SAVED_OFFSET;
fs->regs.reg[reg].loc.offset = offset;
break;
@@ -466,14 +413,14 @@ execute_cfa_program (struct dwarf2_fde *fde, const gdb_byte *insn_ptr,
case DW_CFA_undefined:
insn_ptr = safe_read_uleb128 (insn_ptr, insn_end, ®);
reg = dwarf2_frame_adjust_regnum (gdbarch, reg, eh_frame_p);
- dwarf2_frame_state_alloc_regs (&fs->regs, reg + 1);
+ fs->regs.alloc_regs (reg + 1);
fs->regs.reg[reg].how = DWARF2_FRAME_REG_UNDEFINED;
break;
case DW_CFA_same_value:
insn_ptr = safe_read_uleb128 (insn_ptr, insn_end, ®);
reg = dwarf2_frame_adjust_regnum (gdbarch, reg, eh_frame_p);
- dwarf2_frame_state_alloc_regs (&fs->regs, reg + 1);
+ fs->regs.alloc_regs (reg + 1);
fs->regs.reg[reg].how = DWARF2_FRAME_REG_SAME_VALUE;
break;
@@ -482,7 +429,7 @@ execute_cfa_program (struct dwarf2_fde *fde, const gdb_byte *insn_ptr,
reg = dwarf2_frame_adjust_regnum (gdbarch, reg, eh_frame_p);
insn_ptr = safe_read_uleb128 (insn_ptr, insn_end, &utmp);
utmp = dwarf2_frame_adjust_regnum (gdbarch, utmp, eh_frame_p);
- dwarf2_frame_state_alloc_regs (&fs->regs, reg + 1);
+ fs->regs.alloc_regs (reg + 1);
fs->regs.reg[reg].how = DWARF2_FRAME_REG_SAVED_REG;
fs->regs.reg[reg].loc.reg = utmp;
break;
@@ -491,9 +438,7 @@ execute_cfa_program (struct dwarf2_fde *fde, const gdb_byte *insn_ptr,
{
struct dwarf2_frame_state_reg_info *new_rs;
- new_rs = XNEW (struct dwarf2_frame_state_reg_info);
- *new_rs = fs->regs;
- fs->regs.reg = dwarf2_frame_state_copy_regs (&fs->regs);
+ new_rs = new dwarf2_frame_state_reg_info (fs->regs);
fs->regs.prev = new_rs;
}
break;
@@ -509,11 +454,7 @@ bad CFI data; mismatched DW_CFA_restore_state at %s"),
paddress (gdbarch, fs->pc));
}
else
- {
- xfree (fs->regs.reg);
- fs->regs = *old_rs;
- xfree (old_rs);
- }
+ fs->regs = std::move (*old_rs);
}
break;
@@ -560,7 +501,7 @@ bad CFI data; mismatched DW_CFA_restore_state at %s"),
case DW_CFA_expression:
insn_ptr = safe_read_uleb128 (insn_ptr, insn_end, ®);
reg = dwarf2_frame_adjust_regnum (gdbarch, reg, eh_frame_p);
- dwarf2_frame_state_alloc_regs (&fs->regs, reg + 1);
+ fs->regs.alloc_regs (reg + 1);
insn_ptr = safe_read_uleb128 (insn_ptr, insn_end, &utmp);
fs->regs.reg[reg].loc.exp.start = insn_ptr;
fs->regs.reg[reg].loc.exp.len = utmp;
@@ -573,14 +514,14 @@ bad CFI data; mismatched DW_CFA_restore_state at %s"),
reg = dwarf2_frame_adjust_regnum (gdbarch, reg, eh_frame_p);
insn_ptr = safe_read_sleb128 (insn_ptr, insn_end, &offset);
offset *= fs->data_align;
- dwarf2_frame_state_alloc_regs (&fs->regs, reg + 1);
+ fs->regs.alloc_regs (reg + 1);
fs->regs.reg[reg].how = DWARF2_FRAME_REG_SAVED_OFFSET;
fs->regs.reg[reg].loc.offset = offset;
break;
case DW_CFA_val_offset:
insn_ptr = safe_read_uleb128 (insn_ptr, insn_end, ®);
- dwarf2_frame_state_alloc_regs (&fs->regs, reg + 1);
+ fs->regs.alloc_regs (reg + 1);
insn_ptr = safe_read_uleb128 (insn_ptr, insn_end, &utmp);
offset = utmp * fs->data_align;
fs->regs.reg[reg].how = DWARF2_FRAME_REG_SAVED_VAL_OFFSET;
@@ -589,7 +530,7 @@ bad CFI data; mismatched DW_CFA_restore_state at %s"),
case DW_CFA_val_offset_sf:
insn_ptr = safe_read_uleb128 (insn_ptr, insn_end, ®);
- dwarf2_frame_state_alloc_regs (&fs->regs, reg + 1);
+ fs->regs.alloc_regs (reg + 1);
insn_ptr = safe_read_sleb128 (insn_ptr, insn_end, &offset);
offset *= fs->data_align;
fs->regs.reg[reg].how = DWARF2_FRAME_REG_SAVED_VAL_OFFSET;
@@ -598,7 +539,7 @@ bad CFI data; mismatched DW_CFA_restore_state at %s"),
case DW_CFA_val_expression:
insn_ptr = safe_read_uleb128 (insn_ptr, insn_end, ®);
- dwarf2_frame_state_alloc_regs (&fs->regs, reg + 1);
+ fs->regs.alloc_regs (reg + 1);
insn_ptr = safe_read_uleb128 (insn_ptr, insn_end, &utmp);
fs->regs.reg[reg].loc.exp.start = insn_ptr;
fs->regs.reg[reg].loc.exp.len = utmp;
@@ -631,7 +572,7 @@ bad CFI data; mismatched DW_CFA_restore_state at %s"),
reg = dwarf2_frame_adjust_regnum (gdbarch, reg, eh_frame_p);
insn_ptr = safe_read_uleb128 (insn_ptr, insn_end, &utmp);
offset = utmp * fs->data_align;
- dwarf2_frame_state_alloc_regs (&fs->regs, reg + 1);
+ fs->regs.alloc_regs (reg + 1);
fs->regs.reg[reg].how = DWARF2_FRAME_REG_SAVED_OFFSET;
fs->regs.reg[reg].loc.offset = -offset;
break;
@@ -655,12 +596,73 @@ bad CFI data; mismatched DW_CFA_restore_state at %s"),
if (fs->initial.reg == NULL)
{
/* Don't allow remember/restore between CIE and FDE programs. */
- dwarf2_frame_state_free_regs (fs->regs.prev);
+ delete fs->regs.prev;
fs->regs.prev = NULL;
}
return insn_ptr;
}
+
+#if GDB_SELF_TEST
+
+namespace selftests {
+
+/* Unit test to function execute_cfa_program. */
+
+static void
+execute_cfa_program_test (struct gdbarch *gdbarch)
+{
+ struct dwarf2_fde fde;
+ struct dwarf2_cie cie;
+
+ memset (&fde, 0, sizeof fde);
+ memset (&cie, 0, sizeof cie);
+
+ cie.data_alignment_factor = -4;
+ cie.code_alignment_factor = 2;
+ fde.cie = &cie;
+
+ dwarf2_frame_state fs (0, fde.cie);
+
+ gdb_byte insns[] =
+ {
+ DW_CFA_def_cfa, 1, 4, /* DW_CFA_def_cfa: r1 ofs 4 */
+ DW_CFA_offset | 0x2, 1, /* DW_CFA_offset: r2 at cfa-4 */
+ DW_CFA_remember_state,
+ DW_CFA_restore_state,
+ };
+
+ const gdb_byte *insn_end = insns + sizeof (insns);
+ const gdb_byte *out = execute_cfa_program (&fde, insns, insn_end, gdbarch,
+ 0, &fs);
+
+ SELF_CHECK (out == insn_end);
+ SELF_CHECK (fs.pc == 0);
+
+ /* The instructions above only use r1 and r2, but the register numbers
+ used are adjusted by dwarf2_frame_adjust_regnum. */
+ auto r1 = dwarf2_frame_adjust_regnum (gdbarch, 1, fde.eh_frame_p);
+ auto r2 = dwarf2_frame_adjust_regnum (gdbarch, 2, fde.eh_frame_p);
+
+ SELF_CHECK (fs.regs.num_regs == (std::max (r1, r2) + 1));
+
+ SELF_CHECK (fs.regs.reg[r2].how == DWARF2_FRAME_REG_SAVED_OFFSET);
+ SELF_CHECK (fs.regs.reg[r2].loc.offset == -4);
+
+ for (auto i = 0; i < fs.regs.num_regs; i++)
+ if (i != r2)
+ SELF_CHECK (fs.regs.reg[i].how == DWARF2_FRAME_REG_UNSPECIFIED);
+
+ SELF_CHECK (fs.regs.cfa_reg == 1);
+ SELF_CHECK (fs.regs.cfa_offset == 4);
+ SELF_CHECK (fs.regs.cfa_how == CFA_REG_OFFSET);
+ SELF_CHECK (fs.regs.cfa_exp == NULL);
+ SELF_CHECK (fs.regs.prev == NULL);
+}
+
+} // namespace selftests
+#endif /* GDB_SELF_TEST */
+
\f
/* Architecture-specific operations. */
@@ -884,7 +886,6 @@ dwarf2_fetch_cfa_info (struct gdbarch *gdbarch, CORE_ADDR pc,
/* Save the initialized register set. */
fs.initial = fs.regs;
- fs.initial.reg = dwarf2_frame_state_copy_regs (&fs.regs);
/* Then decode the insns in the FDE up to our target PC. */
execute_cfa_program (fde, fde->instructions, fde->end, gdbarch, pc, &fs);
@@ -1026,7 +1027,6 @@ dwarf2_frame_cache (struct frame_info *this_frame, void **this_cache)
/* Save the initialized register set. */
fs.initial = fs.regs;
- fs.initial.reg = dwarf2_frame_state_copy_regs (&fs.regs);
if (get_frame_func_if_available (this_frame, &entry_pc))
{
@@ -2418,4 +2418,8 @@ _initialize_dwarf2_frame (void)
{
dwarf2_frame_data = gdbarch_data_register_pre_init (dwarf2_frame_init);
dwarf2_frame_objfile_data = register_objfile_data ();
+
+#if GDB_SELF_TEST
+ register_self_test_foreach_arch (selftests::execute_cfa_program_test);
+#endif
}
diff --git a/gdb/dwarf2-frame.h b/gdb/dwarf2-frame.h
index c952a20..2c3d14c 100644
--- a/gdb/dwarf2-frame.h
+++ b/gdb/dwarf2-frame.h
@@ -94,16 +94,85 @@ enum cfa_how_kind
struct dwarf2_frame_state_reg_info
{
- struct dwarf2_frame_state_reg *reg;
- int num_regs;
+ dwarf2_frame_state_reg_info () = default;
+ ~dwarf2_frame_state_reg_info ()
+ {
+ delete prev;
+ xfree (reg);
+ }
+
+ /* Copy constructor. */
+ dwarf2_frame_state_reg_info (const dwarf2_frame_state_reg_info &src)
+ : num_regs (src.num_regs), cfa_offset (src.cfa_offset),
+ cfa_reg (src.cfa_reg), cfa_how (src.cfa_how), cfa_exp (src.cfa_exp),
+ prev (src.prev)
+ {
+ size_t size = src.num_regs * sizeof (struct dwarf2_frame_state_reg);
+
+ reg = (struct dwarf2_frame_state_reg *) xmalloc (size);
+ memcpy (reg, src.reg, size);
+ }
+
+ /* Assignment operator for both move-assignment and copy-assignment. */
+ dwarf2_frame_state_reg_info&
+ operator= (dwarf2_frame_state_reg_info rhs)
+ {
+ swap (*this, rhs);
+ return *this;
+ }
+
+ /* Move constructor. */
+ dwarf2_frame_state_reg_info (dwarf2_frame_state_reg_info &&rhs) noexcept
+ : reg (rhs.reg), num_regs (rhs.num_regs), cfa_offset (rhs.cfa_offset),
+ cfa_reg (rhs.cfa_reg), cfa_how (rhs.cfa_how), cfa_exp (rhs.cfa_exp),
+ prev (rhs.prev)
+ {
+ rhs.prev = nullptr;
+ rhs.reg = nullptr;
+ }
- LONGEST cfa_offset;
- ULONGEST cfa_reg;
- enum cfa_how_kind cfa_how;
- const gdb_byte *cfa_exp;
+/* Assert that the register set RS is large enough to store gdbarch_num_regs
+ columns. If necessary, enlarge the register set. */
+ void alloc_regs (int num_regs_requested)
+ {
+ if (num_regs_requested <= num_regs)
+ return;
+
+ size_t size = sizeof (struct dwarf2_frame_state_reg);
+
+ reg = (struct dwarf2_frame_state_reg *)
+ xrealloc (reg, num_regs_requested * size);
+
+ /* Initialize newly allocated registers. */
+ memset (reg + num_regs, 0, (num_regs_requested - num_regs) * size);
+ num_regs = num_regs_requested;
+ }
+
+ struct dwarf2_frame_state_reg *reg = NULL;
+ int num_regs = 0;
+
+ LONGEST cfa_offset = 0;
+ ULONGEST cfa_reg = 0;
+ enum cfa_how_kind cfa_how = CFA_UNSET;
+ const gdb_byte *cfa_exp = NULL;
/* Used to implement DW_CFA_remember_state. */
- struct dwarf2_frame_state_reg_info *prev;
+ struct dwarf2_frame_state_reg_info *prev = NULL;
+
+private:
+ friend void swap (dwarf2_frame_state_reg_info& lhs,
+ dwarf2_frame_state_reg_info& rhs)
+ {
+ using std::swap;
+
+ swap (lhs.reg, rhs.reg);
+ swap (lhs.num_regs, rhs.num_regs);
+ swap (lhs.cfa_offset, rhs.cfa_offset);
+ swap (lhs.cfa_reg, rhs.cfa_reg);
+ swap (lhs.cfa_how, rhs.cfa_how);
+ swap (lhs.cfa_exp, rhs.cfa_exp);
+ swap (lhs.prev, rhs.prev);
+ }
};
struct dwarf2_cie;
@@ -113,7 +182,6 @@ struct dwarf2_cie;
struct dwarf2_frame_state
{
dwarf2_frame_state (CORE_ADDR pc, struct dwarf2_cie *cie);
- ~dwarf2_frame_state ();
/* Each register save state can be described in terms of a CFA slot,
another register, or a location expression. */
@@ -180,12 +248,6 @@ extern const struct frame_base *
CORE_ADDR dwarf2_frame_cfa (struct frame_info *this_frame);
-/* Assert that the register set RS is large enough to store gdbarch_num_regs
- columns. If necessary, enlarge the register set. */
-
-void dwarf2_frame_state_alloc_regs (struct dwarf2_frame_state_reg_info *rs,
- int num_regs);
-
/* Find the CFA information for PC.
Return 1 if a register is used for the CFA, or 0 if another
diff --git a/gdb/sparc-tdep.c b/gdb/sparc-tdep.c
index dce8527..d751669 100644
--- a/gdb/sparc-tdep.c
+++ b/gdb/sparc-tdep.c
@@ -1588,7 +1588,7 @@ sparc_execute_dwarf_cfa_vendor_op (struct gdbarch *gdbarch, gdb_byte op,
uint64_t reg;
int size = register_size (gdbarch, 0);
- dwarf2_frame_state_alloc_regs (&fs->regs, 32);
+ fs->regs.alloc_regs (32);
for (reg = 8; reg < 16; reg++)
{
fs->regs.reg[reg].how = DWARF2_FRAME_REG_SAVED_REG;
--
1.9.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 9/9] Throw exception in aarch64 unwinder
2017-07-31 22:22 [PATCH 0/9] Centralize unwinder api exceptions handling Yao Qi
` (3 preceding siblings ...)
2017-07-31 22:22 ` [PATCH 3/9] Class-fy dwarf2_frame_state_reg_info Yao Qi
@ 2017-07-31 22:22 ` Yao Qi
2017-07-31 22:22 ` [PATCH 5/9] Handle unwinding exceptions Yao Qi
` (4 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Yao Qi @ 2017-07-31 22:22 UTC (permalink / raw)
To: gdb-patches
gdb:
2017-07-31 Yao Qi <yao.qi@linaro.org>
* aarch64-tdep.c (aarch64_prologue_cache) <available_p>: Remove.
(aarch64_make_prologue_cache_1): Update.
(aarch64_make_prologue_cache): Don't catch exception.
(aarch64_prologue_frame_unwind_stop_reason): Don't check
cache->available_p.
(aarch64_prologue_this_id): Likewise.
(aarch64_make_stub_cache): Don't catch exceptions.
(aarch64_stub_frame_unwind_stop_reason): Don't check
cache->available_p.
(aarch64_stub_this_id): Likewise.
---
gdb/aarch64-tdep.c | 48 ++++++------------------------------------------
1 file changed, 6 insertions(+), 42 deletions(-)
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 75bb00f..19533af 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -174,9 +174,6 @@ struct aarch64_prologue_cache
to identify this frame. */
CORE_ADDR prev_sp;
- /* Is the target available to read from? */
- int available_p;
-
/* The frame base for this frame is just prev_sp - frame size.
FRAMESIZE is the distance from the frame pointer to the
initial stack pointer. */
@@ -744,8 +741,6 @@ aarch64_make_prologue_cache_1 (struct frame_info *this_frame,
cache->saved_regs[reg].addr += cache->prev_sp;
cache->func = get_frame_func (this_frame);
-
- cache->available_p = 1;
}
/* Allocate and fill in *THIS_CACHE with information about the prologue of
@@ -765,16 +760,7 @@ aarch64_make_prologue_cache (struct frame_info *this_frame, void **this_cache)
cache->saved_regs = trad_frame_alloc_saved_regs (this_frame);
*this_cache = cache;
- TRY
- {
- aarch64_make_prologue_cache_1 (this_frame, cache);
- }
- CATCH (ex, RETURN_MASK_ERROR)
- {
- if (ex.error != NOT_AVAILABLE_ERROR)
- throw_exception (ex);
- }
- END_CATCH
+ aarch64_make_prologue_cache_1 (this_frame, cache);
return cache;
}
@@ -788,9 +774,6 @@ aarch64_prologue_frame_unwind_stop_reason (struct frame_info *this_frame,
struct aarch64_prologue_cache *cache
= aarch64_make_prologue_cache (this_frame, this_cache);
- if (!cache->available_p)
- return UNWIND_UNAVAILABLE;
-
/* Halt the backtrace at "_start". */
if (cache->prev_pc <= gdbarch_tdep (get_frame_arch (this_frame))->lowest_pc)
return UNWIND_OUTERMOST;
@@ -812,10 +795,7 @@ aarch64_prologue_this_id (struct frame_info *this_frame,
struct aarch64_prologue_cache *cache
= aarch64_make_prologue_cache (this_frame, this_cache);
- if (!cache->available_p)
- *this_id = frame_id_build_unavailable_stack (cache->func);
- else
- *this_id = frame_id_build (cache->prev_sp, cache->func);
+ *this_id = frame_id_build (cache->prev_sp, cache->func);
}
/* Implement the "prev_register" frame_unwind method. */
@@ -889,19 +869,9 @@ aarch64_make_stub_cache (struct frame_info *this_frame, void **this_cache)
cache->saved_regs = trad_frame_alloc_saved_regs (this_frame);
*this_cache = cache;
- TRY
- {
- cache->prev_sp = get_frame_register_unsigned (this_frame,
- AARCH64_SP_REGNUM);
- cache->prev_pc = get_frame_pc (this_frame);
- cache->available_p = 1;
- }
- CATCH (ex, RETURN_MASK_ERROR)
- {
- if (ex.error != NOT_AVAILABLE_ERROR)
- throw_exception (ex);
- }
- END_CATCH
+ cache->prev_sp = get_frame_register_unsigned (this_frame,
+ AARCH64_SP_REGNUM);
+ cache->prev_pc = get_frame_pc (this_frame);
return cache;
}
@@ -915,9 +885,6 @@ aarch64_stub_frame_unwind_stop_reason (struct frame_info *this_frame,
struct aarch64_prologue_cache *cache
= aarch64_make_stub_cache (this_frame, this_cache);
- if (!cache->available_p)
- return UNWIND_UNAVAILABLE;
-
return UNWIND_NO_REASON;
}
@@ -930,10 +897,7 @@ aarch64_stub_this_id (struct frame_info *this_frame,
struct aarch64_prologue_cache *cache
= aarch64_make_stub_cache (this_frame, this_cache);
- if (cache->available_p)
- *this_id = frame_id_build (cache->prev_sp, cache->prev_pc);
- else
- *this_id = frame_id_build_unavailable_stack (cache->prev_pc);
+ *this_id = frame_id_build (cache->prev_sp, cache->prev_pc);
}
/* Implement the "sniffer" frame_unwind method. */
--
1.9.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/9] Reset *THIS_CACHE in frame_unwind_try_unwinder in case of exception
2017-07-31 22:22 [PATCH 0/9] Centralize unwinder api exceptions handling Yao Qi
2017-07-31 22:22 ` [PATCH 2/9] Class-fy dwarf2_frame_state Yao Qi
@ 2017-07-31 22:22 ` Yao Qi
2017-08-11 16:47 ` Pedro Alves
2017-07-31 22:22 ` [PATCH 7/9] Throw exception in amd64 unwinders Yao Qi
` (7 subsequent siblings)
9 siblings, 1 reply; 13+ messages in thread
From: Yao Qi @ 2017-07-31 22:22 UTC (permalink / raw)
To: gdb-patches
It is required that unwinder->sniffer should set *this_cache to NULL if
the unwinder is not applicable or exception is thrown, so
78ac5f831692f70b841044961069e50d4ba6a76f adds clear_pointer_cleanup to set
*this_cache to NULL in case of exception in order to fix PR 14100.
https://sourceware.org/ml/gdb-patches/2012-08/msg00075.html
This patch removes that clear_pointer_cleanup, and catch all exception in
the caller of unwinder->sniffer. In case of exception, reset *this_case.
gdb:
2017-07-20 Yao Qi <yao.qi@linaro.org>
* dwarf2-frame.c (clear_pointer_cleanup): Remove.
(dwarf2_frame_cache): Remove reset_cache_cleanup.
(dwarf2_frame_cache):
* frame-unwind.c (frame_unwind_try_unwinder): Catch
RETURN_MASK_ALL and set *this_case to NULL.
* frame-unwind.h: Update comments.
---
gdb/dwarf2-frame.c | 14 --------------
gdb/frame-unwind.c | 7 ++++++-
gdb/frame-unwind.h | 3 ++-
3 files changed, 8 insertions(+), 16 deletions(-)
diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index 0cb40d9..f8e6522 100644
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -960,20 +960,9 @@ struct dwarf2_frame_cache
int entry_cfa_sp_offset_p;
};
-/* A cleanup that sets a pointer to NULL. */
-
-static void
-clear_pointer_cleanup (void *arg)
-{
- void **ptr = (void **) arg;
-
- *ptr = NULL;
-}
-
static struct dwarf2_frame_cache *
dwarf2_frame_cache (struct frame_info *this_frame, void **this_cache)
{
- struct cleanup *reset_cache_cleanup;
struct gdbarch *gdbarch = get_frame_arch (this_frame);
const int num_regs = gdbarch_num_regs (gdbarch)
+ gdbarch_num_pseudo_regs (gdbarch);
@@ -989,7 +978,6 @@ dwarf2_frame_cache (struct frame_info *this_frame, void **this_cache)
cache = FRAME_OBSTACK_ZALLOC (struct dwarf2_frame_cache);
cache->reg = FRAME_OBSTACK_CALLOC (num_regs, struct dwarf2_frame_state_reg);
*this_cache = cache;
- reset_cache_cleanup = make_cleanup (clear_pointer_cleanup, this_cache);
/* Unwind the PC.
@@ -1078,7 +1066,6 @@ dwarf2_frame_cache (struct frame_info *this_frame, void **this_cache)
if (ex.error == NOT_AVAILABLE_ERROR)
{
cache->unavailable_retaddr = 1;
- discard_cleanups (reset_cache_cleanup);
return cache;
}
@@ -1184,7 +1171,6 @@ incomplete CFI data; unspecified registers (e.g., %s) at %s"),
&& fs.regs.reg[fs.retaddr_column].how == DWARF2_FRAME_REG_UNDEFINED)
cache->undefined_retaddr = 1;
- discard_cleanups (reset_cache_cleanup);
return cache;
}
diff --git a/gdb/frame-unwind.c b/gdb/frame-unwind.c
index 4d21349..3a75013 100644
--- a/gdb/frame-unwind.c
+++ b/gdb/frame-unwind.c
@@ -106,8 +106,11 @@ frame_unwind_try_unwinder (struct frame_info *this_frame, void **this_cache,
{
res = unwinder->sniffer (unwinder, this_frame, this_cache);
}
- CATCH (ex, RETURN_MASK_ERROR)
+ CATCH (ex, RETURN_MASK_ALL)
{
+ /* Catch all exceptions, caused by either interrupt or error.
+ Reset *THIS_CACHE. */
+ *this_cache = NULL;
if (ex.error == NOT_AVAILABLE_ERROR)
{
/* This usually means that not even the PC is available,
@@ -128,6 +131,8 @@ frame_unwind_try_unwinder (struct frame_info *this_frame, void **this_cache,
}
else
{
+ /* Don't set *THIS_CACHE to NULL here, because sniffer has to do
+ so. */
do_cleanups (old_cleanup);
return 0;
}
diff --git a/gdb/frame-unwind.h b/gdb/frame-unwind.h
index 4588cef..8226b6d 100644
--- a/gdb/frame-unwind.h
+++ b/gdb/frame-unwind.h
@@ -46,7 +46,8 @@ struct value;
the PC and attributes) and if SELF is the applicable unwinder,
return non-zero. Possibly also initialize THIS_PROLOGUE_CACHE; but
only if returning 1. Initializing THIS_PROLOGUE_CACHE in other
- cases (0 return, or exception) is invalid. */
+ cases (0 return) is invalid. In case of exception, the caller has
+ to set *THIS_PROLOGUE_CACHE to NULL. */
typedef int (frame_sniffer_ftype) (const struct frame_unwind *self,
struct frame_info *this_frame,
--
1.9.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/9] Centralize unwinder api exceptions handling
2017-07-31 22:22 [PATCH 0/9] Centralize unwinder api exceptions handling Yao Qi
` (8 preceding siblings ...)
2017-07-31 22:22 ` [PATCH 1/9] Move dwarf2_frame_state_reg.exp_len to union .loc Yao Qi
@ 2017-08-11 8:35 ` Yao Qi
9 siblings, 0 replies; 13+ messages in thread
From: Yao Qi @ 2017-08-11 8:35 UTC (permalink / raw)
To: gdb-patches
On 17-07-31 23:21:46, Yao Qi wrote:
> unwinder has to handle NOT_AVAILABLE_ERROR. Patches 1 ~ 4 are preparatory
> ones. They can go in independently.
I pushed patches 1 ~ 4 in.
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 4/9] Reset *THIS_CACHE in frame_unwind_try_unwinder in case of exception
2017-07-31 22:22 ` [PATCH 4/9] Reset *THIS_CACHE in frame_unwind_try_unwinder in case of exception Yao Qi
@ 2017-08-11 16:47 ` Pedro Alves
2017-08-17 15:27 ` Yao Qi
0 siblings, 1 reply; 13+ messages in thread
From: Pedro Alves @ 2017-08-11 16:47 UTC (permalink / raw)
To: Yao Qi, gdb-patches
On 07/31/2017 11:21 PM, Yao Qi wrote:
> It is required that unwinder->sniffer should set *this_cache to NULL if
> the unwinder is not applicable or exception is thrown, so
> 78ac5f831692f70b841044961069e50d4ba6a76f adds clear_pointer_cleanup to set
> *this_cache to NULL in case of exception in order to fix PR 14100.
> https://sourceware.org/ml/gdb-patches/2012-08/msg00075.html
I suspect that the tailcall sniffer issues Mark was alluding to
in that thread were meanwhile fixed by:
commit 1ec56e88aa9b052ab10b806d82fbdbc8d153d977
Author: Pedro Alves <palves@redhat.com>
AuthorDate: Fri Nov 22 13:17:46 2013 +0000
Eliminate dwarf2_frame_cache recursion, don't unwind from the dwarf2 sniffer (move dwarf2_tailcall_sniffer_first elsewhere).
However, Tom's 2nd approach still wouldn't work, because
dwarf2_frame_cache still has other kinds of (benign) recursion.
> This patch removes that clear_pointer_cleanup, and catch all exception in
> the caller of unwinder->sniffer. In case of exception, reset *this_case.
> @@ -106,8 +106,11 @@ frame_unwind_try_unwinder (struct frame_info *this_frame, void **this_cache,
> {
> res = unwinder->sniffer (unwinder, this_frame, this_cache);
> }
> - CATCH (ex, RETURN_MASK_ERROR)
> + CATCH (ex, RETURN_MASK_ALL)
> {
> + /* Catch all exceptions, caused by either interrupt or error.
> + Reset *THIS_CACHE. */
> + *this_cache = NULL;
If we always do this on error, then why not do it in:
static void
frame_cleanup_after_sniffer (void *arg)
{
struct frame_info *frame = (struct frame_info *) arg;
/* The sniffer should not allocate a prologue cache if it did not
match this frame. */
gdb_assert (frame->prologue_cache == NULL);
with the comment adjusted? I.e., replace the assertion with
frame->prologue_cache = NULL;
Still, there's something in the whole try-unwind mechanism that
isn't handled 100% nicely, that makes me wonder whether this
central this_cache clearing here is the right approach.
Ant that is the fact that we clear *this_cache, but the dwarf2_frame_cache
object is left in the frame chain obstack. Sure, it won't usually
be a problem, the memory will be reclaimed at some point later, but
it still feels like a design hole. It looks to me like we should
just wipe everything at was added to the obstack if the unwinder
fails. I gave it a try, see prototype patch below, to see if something
would break. It didn't -- the patch passes regtesting on x86-64,
at least. I made get_frame_cache a template since all unwinders
would do exactly the same.
A simpler, though maybe not as nice approach could
be to call obstack_free in frame_cleanup_after_sniffer instead:
if (frame->prologue_cache != NULL)
{
// assume
obstack_free (&frame_cache_obstack, frame->prologue_cache);
frame->prologue_cache = NULL;
}
From a6964a47796b5f0b4e9c40a3afa110409c26f668 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 11 Aug 2017 16:27:21 +0100
Subject: [PATCH] frame-obstack
---
gdb/dwarf2-frame.c | 31 ++++++++++---------------------
gdb/frame-unwind.h | 27 +++++++++++++++++++++++++++
gdb/frame.c | 2 +-
gdb/frame.h | 2 ++
gdb/gdb_obstack.h | 30 ++++++++++++++++++++++++++++++
5 files changed, 70 insertions(+), 22 deletions(-)
diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index f8e6522..89e574b 100644
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -919,6 +919,8 @@ dwarf2_fetch_cfa_info (struct gdbarch *gdbarch, CORE_ADDR pc,
\f
struct dwarf2_frame_cache
{
+ explicit dwarf2_frame_cache (frame_info *frame);
+
/* DWARF Call Frame Address. */
CORE_ADDR cfa;
@@ -960,24 +962,17 @@ struct dwarf2_frame_cache
int entry_cfa_sp_offset_p;
};
-static struct dwarf2_frame_cache *
-dwarf2_frame_cache (struct frame_info *this_frame, void **this_cache)
+dwarf2_frame_cache::dwarf2_frame_cache (struct frame_info *this_frame)
{
+ struct dwarf2_frame_cache *cache = this;
struct gdbarch *gdbarch = get_frame_arch (this_frame);
const int num_regs = gdbarch_num_regs (gdbarch)
+ gdbarch_num_pseudo_regs (gdbarch);
- struct dwarf2_frame_cache *cache;
struct dwarf2_fde *fde;
CORE_ADDR entry_pc;
const gdb_byte *instr;
- if (*this_cache)
- return (struct dwarf2_frame_cache *) *this_cache;
-
- /* Allocate a new cache. */
- cache = FRAME_OBSTACK_ZALLOC (struct dwarf2_frame_cache);
cache->reg = FRAME_OBSTACK_CALLOC (num_regs, struct dwarf2_frame_state_reg);
- *this_cache = cache;
/* Unwind the PC.
@@ -1066,7 +1061,7 @@ dwarf2_frame_cache (struct frame_info *this_frame, void **this_cache)
if (ex.error == NOT_AVAILABLE_ERROR)
{
cache->unavailable_retaddr = 1;
- return cache;
+ return;
}
throw_exception (ex);
@@ -1170,16 +1165,13 @@ incomplete CFI data; unspecified registers (e.g., %s) at %s"),
if (fs.retaddr_column < fs.regs.num_regs
&& fs.regs.reg[fs.retaddr_column].how == DWARF2_FRAME_REG_UNDEFINED)
cache->undefined_retaddr = 1;
-
- return cache;
}
static enum unwind_stop_reason
dwarf2_frame_unwind_stop_reason (struct frame_info *this_frame,
void **this_cache)
{
- struct dwarf2_frame_cache *cache
- = dwarf2_frame_cache (this_frame, this_cache);
+ auto *cache = get_frame_cache<dwarf2_frame_cache> (this_frame, this_cache);
if (cache->unavailable_retaddr)
return UNWIND_UNAVAILABLE;
@@ -1194,8 +1186,7 @@ static void
dwarf2_frame_this_id (struct frame_info *this_frame, void **this_cache,
struct frame_id *this_id)
{
- struct dwarf2_frame_cache *cache =
- dwarf2_frame_cache (this_frame, this_cache);
+ auto *cache = get_frame_cache<dwarf2_frame_cache> (this_frame, this_cache);
if (cache->unavailable_retaddr)
(*this_id) = frame_id_build_unavailable_stack (get_frame_func (this_frame));
@@ -1210,8 +1201,7 @@ dwarf2_frame_prev_register (struct frame_info *this_frame, void **this_cache,
int regnum)
{
struct gdbarch *gdbarch = get_frame_arch (this_frame);
- struct dwarf2_frame_cache *cache =
- dwarf2_frame_cache (this_frame, this_cache);
+ auto *cache = get_frame_cache<dwarf2_frame_cache> (this_frame, this_cache);
CORE_ADDR addr;
int realnum;
@@ -1316,7 +1306,7 @@ dwarf2_frame_prev_register (struct frame_info *this_frame, void **this_cache,
static void
dwarf2_frame_dealloc_cache (struct frame_info *self, void *this_cache)
{
- struct dwarf2_frame_cache *cache = dwarf2_frame_cache (self, &this_cache);
+ auto *cache = get_frame_cache<dwarf2_frame_cache> (self, &this_cache);
if (cache->tailcall_cache)
dwarf2_tailcall_frame_unwind.dealloc_cache (self, cache->tailcall_cache);
@@ -1401,8 +1391,7 @@ dwarf2_append_unwinders (struct gdbarch *gdbarch)
static CORE_ADDR
dwarf2_frame_base_address (struct frame_info *this_frame, void **this_cache)
{
- struct dwarf2_frame_cache *cache =
- dwarf2_frame_cache (this_frame, this_cache);
+ auto *cache = get_frame_cache<dwarf2_frame_cache> (this_frame, this_cache);
return cache->cfa;
}
diff --git a/gdb/frame-unwind.h b/gdb/frame-unwind.h
index 8226b6d..b2c65d0 100644
--- a/gdb/frame-unwind.h
+++ b/gdb/frame-unwind.h
@@ -29,6 +29,7 @@ struct regcache;
struct value;
#include "frame.h" /* For enum frame_type. */
+#include "gdb_obstack.h"
/* The following unwind functions assume a chain of frames forming the
sequence: (outer) prev <-> this <-> next (inner). All the
@@ -220,4 +221,30 @@ struct value *frame_unwind_got_bytes (struct frame_info *frame, int regnum,
struct value *frame_unwind_got_address (struct frame_info *frame, int regnum,
CORE_ADDR addr);
+template<typename Cache>
+Cache *
+get_frame_cache (struct frame_info *this_frame, void **this_cache)
+{
+ if (*this_cache)
+ return (Cache *) *this_cache;
+
+ /* Allocate a new cache. */
+ Cache *cache = FRAME_OBSTACK_ZALLOC (Cache);
+
+ /* If an exception happens here, free everything on the obstack up
+ to CACHE. */
+ scoped_obstack_freer obstack_freer (&frame_cache_obstack, cache);
+
+ /* Install CACHE in *THIS_CACHE, and set up to clear it if something
+ goes wrong. */
+ scoped_restore restore_this_cache = make_scoped_restore (this_cache);
+ *this_cache = cache;
+
+ cache = new (cache) Cache (this_frame);
+
+ obstack_freer.dont_free ();
+ restore_this_cache.release ();
+ return cache;
+}
+
#endif
diff --git a/gdb/frame.c b/gdb/frame.c
index 30e4aea..f4f7a5d 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -1552,7 +1552,7 @@ create_sentinel_frame (struct program_space *pspace, struct regcache *regcache)
inferior is stopped. Control variables for the frame cache should
be local to this module. */
-static struct obstack frame_cache_obstack;
+struct obstack frame_cache_obstack;
void *
frame_obstack_zalloc (unsigned long size)
diff --git a/gdb/frame.h b/gdb/frame.h
index 56cbd44..1694241 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -679,6 +679,8 @@ extern void *frame_obstack_zalloc (unsigned long size);
#define FRAME_OBSTACK_CALLOC(NUMBER,TYPE) \
((TYPE *) frame_obstack_zalloc ((NUMBER) * sizeof (TYPE)))
+extern struct obstack frame_cache_obstack;
+
/* Create a regcache, and copy the frame's registers into it. */
struct regcache *frame_save_as_regcache (struct frame_info *this_frame);
diff --git a/gdb/gdb_obstack.h b/gdb/gdb_obstack.h
index b241c82..b481396 100644
--- a/gdb/gdb_obstack.h
+++ b/gdb/gdb_obstack.h
@@ -78,4 +78,34 @@ struct auto_obstack : obstack
{ obstack_free (this, obstack_base (this)); }
};
+/* RAII object that automatically frees all objects allocated in an
+ obstack starting at a specified object. Since the obstack is a
+ stack of objects, freeing one object automatically frees all other
+ objects allocated more recently in the same obstack. */
+class scoped_obstack_freer
+{
+public:
+ scoped_obstack_freer (struct obstack *obstack, void *from_obj)
+ ATTRIBUTE_NONNULL (2) ATTRIBUTE_NONNULL (3)
+ : m_obstack (obstack),
+ m_from_obj (from_obj)
+ {
+ gdb_assert (m_obstack != NULL);
+ gdb_assert (m_from_obj != NULL);
+ }
+
+ ~scoped_obstack_freer ()
+ {
+ if (m_from_obj != NULL)
+ obstack_free (m_obstack, m_from_obj);
+ }
+
+ /* Don't free objects. */
+ void dont_free () { m_from_obj = NULL; }
+
+private:
+ struct obstack *m_obstack;
+ void *m_from_obj;
+};
+
#endif
--
2.5.5
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 4/9] Reset *THIS_CACHE in frame_unwind_try_unwinder in case of exception
2017-08-11 16:47 ` Pedro Alves
@ 2017-08-17 15:27 ` Yao Qi
0 siblings, 0 replies; 13+ messages in thread
From: Yao Qi @ 2017-08-17 15:27 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Pedro Alves <palves@redhat.com> writes:
> If we always do this on error, then why not do it in:
>
> static void
> frame_cleanup_after_sniffer (void *arg)
> {
> struct frame_info *frame = (struct frame_info *) arg;
>
> /* The sniffer should not allocate a prologue cache if it did not
> match this frame. */
> gdb_assert (frame->prologue_cache == NULL);
>
> with the comment adjusted? I.e., replace the assertion with
> frame->prologue_cache = NULL;
Each frame sniffer still has to de-allocate frame->prologue_cache if the
unwinder isn't applicable. My patch touches the patch on
error/exception, and the assert is to check each unwinder's sniffer has
to release frame->prologue_cache on no-error return.
>
> Still, there's something in the whole try-unwind mechanism that
> isn't handled 100% nicely, that makes me wonder whether this
> central this_cache clearing here is the right approach.
>
> Ant that is the fact that we clear *this_cache, but the dwarf2_frame_cache
> object is left in the frame chain obstack. Sure, it won't usually
We don't allocate dwarf2_frame_cache in dwarf2_frame_sniffer, so
dwarf2_frame_cache object won't be left in obstack. However,
- dwarf2_frame_cache created in dwarf2_frame_{unwinder_stop_reason,
this_id, prev_register} may be left in obstack in case of exception.
- other unwinders' sniffer may allocate frame cache, and its frame
cache may be left on obstack in case of exception.
> be a problem, the memory will be reclaimed at some point later, but
> it still feels like a design hole. It looks to me like we should
> just wipe everything at was added to the obstack if the unwinder
Such design hole exists for several years. If we want to fix this
design hole, as you did, we should somehow call obstack_free. On the
contrary, this shows the benefit of this patch series, that is, we
centralize the exception handling, so we can call obstack_free when
exception is thrown in unwinder apis (see patch #5). Otherwise (we
catch exceptions in each unwinder) we have to call obstack_free in each
unwinder.
> fails. I gave it a try, see prototype patch below, to see if something
> would break. It didn't -- the patch passes regtesting on x86-64,
> at least. I made get_frame_cache a template since all unwinders
> would do exactly the same.
>
> A simpler, though maybe not as nice approach could
> be to call obstack_free in frame_cleanup_after_sniffer instead:
>
> if (frame->prologue_cache != NULL)
> {
> // assume
> obstack_free (&frame_cache_obstack, frame->prologue_cache);
> frame->prologue_cache = NULL;
> }
Your patch get_frame_cache can guarantee the obstack space can be
released when an exception is thrown during initialization. We need it
for every unwinder. The simpler approach above looks better, because we
can do it after every unwinder apis call.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2017-08-17 15:27 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-31 22:22 [PATCH 0/9] Centralize unwinder api exceptions handling Yao Qi
2017-07-31 22:22 ` [PATCH 2/9] Class-fy dwarf2_frame_state Yao Qi
2017-07-31 22:22 ` [PATCH 4/9] Reset *THIS_CACHE in frame_unwind_try_unwinder in case of exception Yao Qi
2017-08-11 16:47 ` Pedro Alves
2017-08-17 15:27 ` Yao Qi
2017-07-31 22:22 ` [PATCH 7/9] Throw exception in amd64 unwinders Yao Qi
2017-07-31 22:22 ` [PATCH 3/9] Class-fy dwarf2_frame_state_reg_info Yao Qi
2017-07-31 22:22 ` [PATCH 9/9] Throw exception in aarch64 unwinder Yao Qi
2017-07-31 22:22 ` [PATCH 5/9] Handle unwinding exceptions Yao Qi
2017-07-31 22:22 ` [PATCH 6/9] Throw exception in dwarf2 unwinders Yao Qi
2017-07-31 22:22 ` [PATCH 8/9] Throw exception in i386 unwinders Yao Qi
2017-07-31 22:22 ` [PATCH 1/9] Move dwarf2_frame_state_reg.exp_len to union .loc Yao Qi
2017-08-11 8:35 ` [PATCH 0/9] Centralize unwinder api exceptions handling Yao Qi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox