[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