Hello:
David Kastrup wrote on 05/09/2018 08:23 PM:
Mamoru TASAKA <mtas...@fedoraproject.org> 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 <mtas...@fedoraproject.org>
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.
So would you mean below?
diff --git a/lily/beam.cc b/lily/beam.cc
index dff62168af..b318f0834d 100644
--- a/lily/beam.cc
+++ b/lily/beam.cc
@@ -457,7 +457,6 @@ 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];
// 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
@@ -471,9 +470,9 @@ Beam::calc_beam_segments (SCM smob)
: seg.stem_index_ + 1 < stems.size ();
bool event = on_beam_bound
- || abs (seg.rank_ - neighbor_seg.rank_) > 1
+ || abs (seg.rank_ - segs[j + event_dir].rank_) > 1
|| (abs (vertical_count) >= seg.max_connect_
- || abs (vertical_count) >=
neighbor_seg.max_connect_);
+ || abs (vertical_count) >= segs[j +
event_dir].max_connect_);
if (!event)
// Then this edge of the current segment is irrelevant because
it will
I am okay with either of these.
Regards,
Mamoru
_______________________________________________
bug-lilypond mailing list
bug-lilypond@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-lilypond
