bug-lilypond
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: 2.19.81 aborts on many .ly files when compiled with gcc8 -Wp, -D_GLI


From: David Kastrup
Subject: Re: 2.19.81 aborts on many .ly files when compiled with gcc8 -Wp, -D_GLIBCXX_ASSERTIONS
Date: Wed, 09 May 2018 13:23:27 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/26.0.50 (gnu/linux)

Mamoru TASAKA <address@hidden> writes:

> And the attached proposal patch should fix this issue. Please review this.

>>From 463aabe95b2dd3856c928c5e6917eda2138f3aa4 Mon Sep 17 00:00:00 2001
> From: Mamoru TASAKA <address@hidden>
> Date: Tue, 8 May 2018 22:34:41 +0900
> Subject: [PATCH] Fix out-of-bounds access detected by -D_GLIBCXX_ASSERTIONS
>
> file: lily/beam.cc
> In Beam::calc_beam_segments(), when on_beam_bound is true, access for segs[] 
> is currenctly out-of-bounds.
> So calculate on_beam_bound first to avoid that invalid access. In this case, 
> neighbor_seg is not used
> anyway, so this is okay.
> ---
>  lily/beam.cc | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/lily/beam.cc b/lily/beam.cc
> index dff62168af..d32624a4ff 100644
> --- a/lily/beam.cc
> +++ b/lily/beam.cc
> @@ -457,15 +457,15 @@ Beam::calc_beam_segments (SCM smob)
>            Beam_stem_segment const &seg = segs[j];
>            for (LEFT_and_RIGHT (event_dir))
>              {
> -              Beam_stem_segment const &neighbor_seg = segs[j + event_dir];
> +              bool on_beam_bound = (event_dir == LEFT) ? j == 0
> +                                   : j == segs.size () - 1;
> +              Beam_stem_segment const &neighbor_seg = segs[on_beam_bound ? 0 
> : j + event_dir];
>                // TODO: make names clearer? --jneem
>                // on_line_bound: whether the current segment is on the 
> boundary of the WHOLE beam
>                // on_beam_bound: whether the current segment is on the 
> boundary of just that part
>                //   of the beam with the current beam_rank
>                bool on_line_bound = (seg.dir_ == LEFT) ? seg.stem_index_ == 0
>                                     : seg.stem_index_ == stems.size () - 1;
> -              bool on_beam_bound = (event_dir == LEFT) ? j == 0
> -                                   : j == segs.size () - 1;
>                bool inside_stem = (event_dir == LEFT)
>                                   ? seg.stem_index_ > 0
>                                   : seg.stem_index_ + 1 < stems.size ();

Well spotted but this patch is butt-ugly by calculating a nonsense
reference in the troublesome case that is then being ignored.

Since neighbor_seg is not even getting used more than twice and that in
the same expression, I'd just write out segs[j + event_dir] instead and
let the optimizer do the rest.  The problematic expression is then not
even generated because of short-circuit evaluation.

-- 
David Kastrup



reply via email to

[Prev in Thread] Current Thread [Next in Thread]