Fix double free when running gdb.linespec/ls-errs.exp (PR breakpoints/21553)

The problem is that b->extra_string is free'ed twice: Once in the
breakpoint's dtor, and another time via make_cleanup (xfree).

This patch gets rid of the cleanups, fixing the problem.

Tested on x86_64 GNU/Linux.

gdb/ChangeLog:
2017-06-06  Pedro Alves  <palves@redhat.com>

	PR breakpoints/21553
	* breakpoint.c (create_breakpoints_sal_default)
	(init_breakpoint_sal, create_breakpoint_sal): Use
	gdb::unique_xmalloc_ptr for string parameters.
	(create_breakpoint): Constify 'extra_string' and 'cond_string'
	parameters.  Replace cleanups with gdb::unique_xmalloc_ptr.
	(base_breakpoint_create_breakpoints_sal)
	(bkpt_create_breakpoints_sal, tracepoint_create_breakpoints_sal)
	(strace_marker_create_breakpoints_sal)
	(create_breakpoints_sal_default): Use gdb::unique_xmalloc_ptr for
	string parameters.
	* breakpoint.h (breakpoint_ops::create_breakpoints_sal): Use
	gdb::unique_xmalloc_ptr for string parameters.
	(create_breakpoint): Constify 'extra_string' and 'cond_string'
	parameters.
This commit is contained in:
Pedro Alves 2017-06-06 15:53:59 +01:00
parent fbe654c8bc
commit e1e01040aa
3 changed files with 80 additions and 64 deletions

View file

@ -1,3 +1,21 @@
2017-06-06 Pedro Alves <palves@redhat.com>
PR breakpoints/21553
* breakpoint.c (create_breakpoints_sal_default)
(init_breakpoint_sal, create_breakpoint_sal): Use
gdb::unique_xmalloc_ptr for string parameters.
(create_breakpoint): Constify 'extra_string' and 'cond_string'
parameters. Replace cleanups with gdb::unique_xmalloc_ptr.
(base_breakpoint_create_breakpoints_sal)
(bkpt_create_breakpoints_sal, tracepoint_create_breakpoints_sal)
(strace_marker_create_breakpoints_sal)
(create_breakpoints_sal_default): Use gdb::unique_xmalloc_ptr for
string parameters.
* breakpoint.h (breakpoint_ops::create_breakpoints_sal): Use
gdb::unique_xmalloc_ptr for string parameters.
(create_breakpoint): Constify 'extra_string' and 'cond_string'
parameters.
2017-06-06 Alan Hayward <alan.hayward@arm.com> 2017-06-06 Alan Hayward <alan.hayward@arm.com>
* alpha-tdep.c (alpha_register_to_value): Use * alpha-tdep.c (alpha_register_to_value): Use

View file

@ -120,7 +120,9 @@ static void
static void create_breakpoints_sal_default (struct gdbarch *, static void create_breakpoints_sal_default (struct gdbarch *,
struct linespec_result *, struct linespec_result *,
char *, char *, enum bptype, gdb::unique_xmalloc_ptr<char>,
gdb::unique_xmalloc_ptr<char>,
enum bptype,
enum bpdisp, int, int, enum bpdisp, int, int,
int, int,
const struct breakpoint_ops *, const struct breakpoint_ops *,
@ -9167,8 +9169,9 @@ static void
init_breakpoint_sal (struct breakpoint *b, struct gdbarch *gdbarch, init_breakpoint_sal (struct breakpoint *b, struct gdbarch *gdbarch,
struct symtabs_and_lines sals, struct symtabs_and_lines sals,
event_location_up &&location, event_location_up &&location,
char *filter, char *cond_string, gdb::unique_xmalloc_ptr<char> filter,
char *extra_string, gdb::unique_xmalloc_ptr<char> cond_string,
gdb::unique_xmalloc_ptr<char> extra_string,
enum bptype type, enum bpdisp disposition, enum bptype type, enum bpdisp disposition,
int thread, int task, int ignore_count, int thread, int task, int ignore_count,
const struct breakpoint_ops *ops, int from_tty, const struct breakpoint_ops *ops, int from_tty,
@ -9214,8 +9217,8 @@ init_breakpoint_sal (struct breakpoint *b, struct gdbarch *gdbarch,
b->thread = thread; b->thread = thread;
b->task = task; b->task = task;
b->cond_string = cond_string; b->cond_string = cond_string.release ();
b->extra_string = extra_string; b->extra_string = extra_string.release ();
b->ignore_count = ignore_count; b->ignore_count = ignore_count;
b->enable_state = enabled ? bp_enabled : bp_disabled; b->enable_state = enabled ? bp_enabled : bp_disabled;
b->disposition = disposition; b->disposition = disposition;
@ -9299,15 +9302,16 @@ init_breakpoint_sal (struct breakpoint *b, struct gdbarch *gdbarch,
b->location = std::move (location); b->location = std::move (location);
else else
b->location = new_address_location (b->loc->address, NULL, 0); b->location = new_address_location (b->loc->address, NULL, 0);
b->filter = filter; b->filter = filter.release ();
} }
static void static void
create_breakpoint_sal (struct gdbarch *gdbarch, create_breakpoint_sal (struct gdbarch *gdbarch,
struct symtabs_and_lines sals, struct symtabs_and_lines sals,
event_location_up &&location, event_location_up &&location,
char *filter, char *cond_string, gdb::unique_xmalloc_ptr<char> filter,
char *extra_string, gdb::unique_xmalloc_ptr<char> cond_string,
gdb::unique_xmalloc_ptr<char> extra_string,
enum bptype type, enum bpdisp disposition, enum bptype type, enum bpdisp disposition,
int thread, int task, int ignore_count, int thread, int task, int ignore_count,
const struct breakpoint_ops *ops, int from_tty, const struct breakpoint_ops *ops, int from_tty,
@ -9318,7 +9322,9 @@ create_breakpoint_sal (struct gdbarch *gdbarch,
init_breakpoint_sal (b.get (), gdbarch, init_breakpoint_sal (b.get (), gdbarch,
sals, std::move (location), sals, std::move (location),
filter, cond_string, extra_string, std::move (filter),
std::move (cond_string),
std::move (extra_string),
type, disposition, type, disposition,
thread, task, ignore_count, thread, task, ignore_count,
ops, from_tty, ops, from_tty,
@ -9346,7 +9352,8 @@ create_breakpoint_sal (struct gdbarch *gdbarch,
static void static void
create_breakpoints_sal (struct gdbarch *gdbarch, create_breakpoints_sal (struct gdbarch *gdbarch,
struct linespec_result *canonical, struct linespec_result *canonical,
char *cond_string, char *extra_string, gdb::unique_xmalloc_ptr<char> cond_string,
gdb::unique_xmalloc_ptr<char> extra_string,
enum bptype type, enum bpdisp disposition, enum bptype type, enum bpdisp disposition,
int thread, int task, int ignore_count, int thread, int task, int ignore_count,
const struct breakpoint_ops *ops, int from_tty, const struct breakpoint_ops *ops, int from_tty,
@ -9365,13 +9372,14 @@ create_breakpoints_sal (struct gdbarch *gdbarch,
event_location_up location event_location_up location
= (canonical->location != NULL = (canonical->location != NULL
? copy_event_location (canonical->location.get ()) : NULL); ? copy_event_location (canonical->location.get ()) : NULL);
char *filter_string = lsal->canonical ? xstrdup (lsal->canonical) : NULL; gdb::unique_xmalloc_ptr<char> filter_string
(lsal->canonical != NULL ? xstrdup (lsal->canonical) : NULL);
make_cleanup (xfree, filter_string);
create_breakpoint_sal (gdbarch, lsal->sals, create_breakpoint_sal (gdbarch, lsal->sals,
std::move (location), std::move (location),
filter_string, std::move (filter_string),
cond_string, extra_string, std::move (cond_string),
std::move (extra_string),
type, disposition, type, disposition,
thread, task, ignore_count, ops, thread, task, ignore_count, ops,
from_tty, enabled, internal, flags, from_tty, enabled, internal, flags,
@ -9650,8 +9658,9 @@ decode_static_tracepoint_spec (const char **arg_p)
int int
create_breakpoint (struct gdbarch *gdbarch, create_breakpoint (struct gdbarch *gdbarch,
const struct event_location *location, char *cond_string, const struct event_location *location,
int thread, char *extra_string, const char *cond_string,
int thread, const char *extra_string,
int parse_extra, int parse_extra,
int tempflag, enum bptype type_wanted, int tempflag, enum bptype type_wanted,
int ignore_count, int ignore_count,
@ -9743,9 +9752,13 @@ create_breakpoint (struct gdbarch *gdbarch,
breakpoint. */ breakpoint. */
if (!pending) if (!pending)
{ {
gdb::unique_xmalloc_ptr<char> cond_string_copy;
gdb::unique_xmalloc_ptr<char> extra_string_copy;
if (parse_extra) if (parse_extra)
{ {
char *rest; char *rest;
char *cond;
struct linespec_sals *lsal; struct linespec_sals *lsal;
lsal = VEC_index (linespec_sals, canonical.sals, 0); lsal = VEC_index (linespec_sals, canonical.sals, 0);
@ -9756,15 +9769,9 @@ create_breakpoint (struct gdbarch *gdbarch,
re-parse it in context of each sal. */ re-parse it in context of each sal. */
find_condition_and_thread (extra_string, lsal->sals.sals[0].pc, find_condition_and_thread (extra_string, lsal->sals.sals[0].pc,
&cond_string, &thread, &task, &rest); &cond, &thread, &task, &rest);
if (cond_string) cond_string_copy.reset (cond);
make_cleanup (xfree, cond_string); extra_string_copy.reset (rest);
if (rest)
make_cleanup (xfree, rest);
if (rest)
extra_string = rest;
else
extra_string = NULL;
} }
else else
{ {
@ -9774,20 +9781,16 @@ create_breakpoint (struct gdbarch *gdbarch,
/* Create a private copy of condition string. */ /* Create a private copy of condition string. */
if (cond_string) if (cond_string)
{ cond_string_copy.reset (xstrdup (cond_string));
cond_string = xstrdup (cond_string);
make_cleanup (xfree, cond_string);
}
/* Create a private copy of any extra string. */ /* Create a private copy of any extra string. */
if (extra_string) if (extra_string)
{ extra_string_copy.reset (xstrdup (extra_string));
extra_string = xstrdup (extra_string);
make_cleanup (xfree, extra_string);
}
} }
ops->create_breakpoints_sal (gdbarch, &canonical, ops->create_breakpoints_sal (gdbarch, &canonical,
cond_string, extra_string, type_wanted, std::move (cond_string_copy),
std::move (extra_string_copy),
type_wanted,
tempflag ? disp_del : disp_donttouch, tempflag ? disp_del : disp_donttouch,
thread, task, ignore_count, ops, thread, task, ignore_count, ops,
from_tty, enabled, internal, flags); from_tty, enabled, internal, flags);
@ -9804,22 +9807,12 @@ create_breakpoint (struct gdbarch *gdbarch,
else else
{ {
/* Create a private copy of condition string. */ /* Create a private copy of condition string. */
if (cond_string) b->cond_string = cond_string != NULL ? xstrdup (cond_string) : NULL;
{
cond_string = xstrdup (cond_string);
make_cleanup (xfree, cond_string);
}
b->cond_string = cond_string;
b->thread = thread; b->thread = thread;
} }
/* Create a private copy of any extra string. */ /* Create a private copy of any extra string. */
if (extra_string != NULL) b->extra_string = extra_string != NULL ? xstrdup (extra_string) : NULL;
{
extra_string = xstrdup (extra_string);
make_cleanup (xfree, extra_string);
}
b->extra_string = extra_string;
b->ignore_count = ignore_count; b->ignore_count = ignore_count;
b->disposition = tempflag ? disp_del : disp_donttouch; b->disposition = tempflag ? disp_del : disp_donttouch;
b->condition_not_parsed = 1; b->condition_not_parsed = 1;
@ -12839,8 +12832,8 @@ base_breakpoint_create_sals_from_location
static void static void
base_breakpoint_create_breakpoints_sal (struct gdbarch *gdbarch, base_breakpoint_create_breakpoints_sal (struct gdbarch *gdbarch,
struct linespec_result *c, struct linespec_result *c,
char *cond_string, gdb::unique_xmalloc_ptr<char> cond_string,
char *extra_string, gdb::unique_xmalloc_ptr<char> extra_string,
enum bptype type_wanted, enum bptype type_wanted,
enum bpdisp disposition, enum bpdisp disposition,
int thread, int thread,
@ -13088,8 +13081,8 @@ bkpt_create_sals_from_location (const struct event_location *location,
static void static void
bkpt_create_breakpoints_sal (struct gdbarch *gdbarch, bkpt_create_breakpoints_sal (struct gdbarch *gdbarch,
struct linespec_result *canonical, struct linespec_result *canonical,
char *cond_string, gdb::unique_xmalloc_ptr<char> cond_string,
char *extra_string, gdb::unique_xmalloc_ptr<char> extra_string,
enum bptype type_wanted, enum bptype type_wanted,
enum bpdisp disposition, enum bpdisp disposition,
int thread, int thread,
@ -13099,7 +13092,8 @@ bkpt_create_breakpoints_sal (struct gdbarch *gdbarch,
int internal, unsigned flags) int internal, unsigned flags)
{ {
create_breakpoints_sal_default (gdbarch, canonical, create_breakpoints_sal_default (gdbarch, canonical,
cond_string, extra_string, std::move (cond_string),
std::move (extra_string),
type_wanted, type_wanted,
disposition, thread, task, disposition, thread, task,
ignore_count, ops, from_tty, ignore_count, ops, from_tty,
@ -13407,8 +13401,8 @@ tracepoint_create_sals_from_location (const struct event_location *location,
static void static void
tracepoint_create_breakpoints_sal (struct gdbarch *gdbarch, tracepoint_create_breakpoints_sal (struct gdbarch *gdbarch,
struct linespec_result *canonical, struct linespec_result *canonical,
char *cond_string, gdb::unique_xmalloc_ptr<char> cond_string,
char *extra_string, gdb::unique_xmalloc_ptr<char> extra_string,
enum bptype type_wanted, enum bptype type_wanted,
enum bpdisp disposition, enum bpdisp disposition,
int thread, int thread,
@ -13418,7 +13412,8 @@ tracepoint_create_breakpoints_sal (struct gdbarch *gdbarch,
int internal, unsigned flags) int internal, unsigned flags)
{ {
create_breakpoints_sal_default (gdbarch, canonical, create_breakpoints_sal_default (gdbarch, canonical,
cond_string, extra_string, std::move (cond_string),
std::move (extra_string),
type_wanted, type_wanted,
disposition, thread, task, disposition, thread, task,
ignore_count, ops, from_tty, ignore_count, ops, from_tty,
@ -13563,8 +13558,8 @@ strace_marker_create_sals_from_location (const struct event_location *location,
static void static void
strace_marker_create_breakpoints_sal (struct gdbarch *gdbarch, strace_marker_create_breakpoints_sal (struct gdbarch *gdbarch,
struct linespec_result *canonical, struct linespec_result *canonical,
char *cond_string, gdb::unique_xmalloc_ptr<char> cond_string,
char *extra_string, gdb::unique_xmalloc_ptr<char> extra_string,
enum bptype type_wanted, enum bptype type_wanted,
enum bpdisp disposition, enum bpdisp disposition,
int thread, int thread,
@ -13598,7 +13593,8 @@ strace_marker_create_breakpoints_sal (struct gdbarch *gdbarch,
tp = new tracepoint (); tp = new tracepoint ();
init_breakpoint_sal (tp, gdbarch, expanded, init_breakpoint_sal (tp, gdbarch, expanded,
std::move (location), NULL, std::move (location), NULL,
cond_string, extra_string, std::move (cond_string),
std::move (extra_string),
type_wanted, disposition, type_wanted, disposition,
thread, task, ignore_count, ops, thread, task, ignore_count, ops,
from_tty, enabled, internal, flags, from_tty, enabled, internal, flags,
@ -14351,8 +14347,8 @@ create_sals_from_location_default (const struct event_location *location,
static void static void
create_breakpoints_sal_default (struct gdbarch *gdbarch, create_breakpoints_sal_default (struct gdbarch *gdbarch,
struct linespec_result *canonical, struct linespec_result *canonical,
char *cond_string, gdb::unique_xmalloc_ptr<char> cond_string,
char *extra_string, gdb::unique_xmalloc_ptr<char> extra_string,
enum bptype type_wanted, enum bptype type_wanted,
enum bpdisp disposition, enum bpdisp disposition,
int thread, int thread,
@ -14361,8 +14357,9 @@ create_breakpoints_sal_default (struct gdbarch *gdbarch,
int from_tty, int enabled, int from_tty, int enabled,
int internal, unsigned flags) int internal, unsigned flags)
{ {
create_breakpoints_sal (gdbarch, canonical, cond_string, create_breakpoints_sal (gdbarch, canonical,
extra_string, std::move (cond_string),
std::move (extra_string),
type_wanted, disposition, type_wanted, disposition,
thread, task, ignore_count, ops, from_tty, thread, task, ignore_count, ops, from_tty,
enabled, internal, flags); enabled, internal, flags);

View file

@ -604,7 +604,8 @@ struct breakpoint_ops
This function is called inside `create_breakpoint'. */ This function is called inside `create_breakpoint'. */
void (*create_breakpoints_sal) (struct gdbarch *, void (*create_breakpoints_sal) (struct gdbarch *,
struct linespec_result *, struct linespec_result *,
char *, char *, gdb::unique_xmalloc_ptr<char>,
gdb::unique_xmalloc_ptr<char>,
enum bptype, enum bpdisp, int, int, enum bptype, enum bpdisp, int, int,
int, const struct breakpoint_ops *, int, const struct breakpoint_ops *,
int, int, int, unsigned); int, int, int, unsigned);
@ -1329,8 +1330,8 @@ enum breakpoint_create_flags
extern int create_breakpoint (struct gdbarch *gdbarch, extern int create_breakpoint (struct gdbarch *gdbarch,
const struct event_location *location, const struct event_location *location,
char *cond_string, int thread, const char *cond_string, int thread,
char *extra_string, const char *extra_string,
int parse_extra, int parse_extra,
int tempflag, enum bptype wanted_type, int tempflag, enum bptype wanted_type,
int ignore_count, int ignore_count,