CodePeer : Find the Bug Challenge
« Back to Index

Challenge 5 – Dead, unused, predetermined and suspicious code

The package Conductor defines an automated conductor for an orchestra, which is updated both periodically every few micro-seconds (entry Advance of task Concert) and each time a sensor detects that a musician plays a new note (entry Start_Of_Note of task Concert).

Have a look at the following code and see if you can spot the errors. Once you think you’ve got them all, click on the “Go CodePeer” button to see how well you matched up to CodePeer’s comprehensive and rapid analysis of the same code.

Error line 97: “high: conditional check raises exception here: checking that Part >= 1”

Here’s how CodePeer helps you coming to this conclusion: Local variable Part is initialized to zero, whereas array Orchestra starts at index one, hence the mismatch. In fact, the code here is dead, as the test for the loop will fail at the very first iteration. CodePeer is reporting that an error would occur if the execution was able to reach this program point.

Error line 114: “high: conditional check raises exception here: checking that Part >= 1 and Part <= num_parts.all”

Here’s how CodePeer helps you coming to this conclusion: Local variable Part is initialized to zero, whereas array Orchestra starts at index one, hence the mismatch. Contrary to the previous error, the code is not dead here, due to the different form of the loop test. CodePeer reports the same error on the following line 115 when accessing field Dur.

Error line 125: “warning: suspicious constant operation (min(…))/96 always evaluates to 0”

Here’s how CodePeer helps you coming to this conclusion: CodePeer computes a range of Score.Duration’First .. Score.Duration’Last – 1 for local variable Min from its initialization on line 122. Then, dividing Min by Score.Duration’Last can only result in value zero, which is clearly suspicious for an operation on non-constant operands. The problem stems from the inversion of the conversion to fixed-point type Position and the division on line 125, so that the division occurs in integers here instead of fixed-points.

Error line 156: “medium: condition predetermined because (Dur <= Score.Duration'last) is always true”

Here’s how CodePeer helps you coming to this conclusion: Local variable Dur is of type Score.Duration, hence it cannot exceed Score.Duration’Last. The problem here is that the programmer used Dur where Cumul_Dur was expected.

Error line 167: “warning: unused assignment into Dur”

Here’s how CodePeer helps you coming to this conclusion: Dur is a local variable, hence assigning to it is only useful if its value is read afterwards, which is not the case here. Instead of defining Dur as a local variable on line 143, it should be defined as a renaming like Pos on line 140.

-- file --

package Score is

   type Pitch is new Natural range 0 .. 127;
   --  MIDI note in the MIDI Tuning Standard, only representing semitones here

   type Long_Duration is new Natural;
   subtype Duration is Long_Duration range 0 .. 96;
   --  Fraction of a measure, 96 representing the complete measure, which
   --  allows representing from thirty-second note to whole note for measures
   --  with one to four beats

   type Position is delta 10.0**(-2) range 0.0 .. 10_000.0;
   --  Distance from the start of the score in number of measures

   type Instrument is
     (Flute, Recorder, Violin, Trumpet, Clarinet, Oboe, Cornet, Flugelhorn,
      Piccolo_Trumpet, Piccolo, Alto_Saxophone, Alto_Flute, Viola, Horn,
      Alto_Horn, Trombone, Tenor_Saxophone, Bass_Trumpet, Bassoon,
      English_Horn, Baritone_Saxophone, Baritone_Horn, Bass_Clarinet, Cello,
      Euphonium, Bass_Trombone, Contrabassoon, Bass_Saxophone, Double_Bass,
      Tuba, Contrabass_Saxophone, Contrabass_Bugle);

   type Interpreter is (First, Second, Third, Fourth, Fifth);

   procedure Get_Note
     (Inter :     Interpreter;
      Instr :     Instrument;
      Pos   :     Position;
      Fwd   :     Long_Duration;
      Pit   : out Pitch;
      Dur   : out Duration);
   --  Return the Pitch and Duration of the note starting forward by Fwd from
   --  position Pos for the Interpreter of the Instrument

end Score;

-- file --

with Score; use Score;

package Conductor is

   type Positions is array (Positive range <>) of Position;
   type Durations is array (Positive range <>) of Score.Duration;

   type Part (Max_Players : Positive) is record
      Inter       : Interpreter;  --  Interpreter for this part
      Instr       : Instrument;   --  Instrument for this part
      Num_Players : Positive;     --  Number of players in 1 .. Max_Players
      Pos         : Positions (1 .. Max_Players);
      --  Current position of each player in the score
      Dur         : Durations (1 .. Max_Players);
      --  Remaining duration of the current note for each player
   end record;

   type Parts is array (Positive range <>) of Part (5);

   task type Concert (Num_Parts : Positive) is
      --  Initialize the parts to be played
      entry Initialize (Initial_Parts : Parts);
      --  Signal that a player starts a note
      entry Start_Of_Note
        (Index : Positive;  --  Index of the Part being played in the array
         Num   : Positive;  --  Number of the player in the Part
         Pit   : Pitch);    --  Pitch of the note
      --  Advance the position of all players by a Step, which is expected to
      --  be much smaller than the interval between two consecutive notes
      entry Advance (Step : Score.Duration);
      --  Start of the concert
      entry Play;
   end Concert;

end Conductor;

-- file conductor.adb --

package body Conductor is

   task body Concert is
      Orchestra : Parts (1 .. Num_Parts);

      accept Initialize (Initial_Parts : Parts)
            Part : Integer := 0;
            while Part in Orchestra'Range loop
               Orchestra (Part) := Initial_Parts (Part);
               Part := Part + 1;
            end loop;
      end Initialize;

      accept Play;  --  Start the concert

            accept Advance (Step : Score.Duration)
                  Part : Integer := 0;
                  while Part < Num_Parts loop
                        Pos : Positions renames Orchestra (Part).Pos;
                        Dur : Durations renames Orchestra (Part).Dur;
                        for Index in Pos'Range loop
                              --  Do not allow to advance beyond a new note as
                              --  it is the role of Start_Of_Note to do so
                              Min : constant Score.Duration :=
                                    Score.Duration'Min (Step, Dur (Index) - 1);
                              Pos (Index) := Pos (Index) +
                                Position (Min / Score.Duration'Last);
                        end loop;
                     Part := Part + 1;
                  end loop;
            accept Start_Of_Note
              (Index : Positive;
               Num   : Positive;
               Pit   : Pitch)
                  Pos        : Position renames Orchestra (Index).Pos (Num);
                  Inter      : Interpreter    := Orchestra (Index).Inter;
                  Instr      : Instrument     := Orchestra (Index).Instr;
                  Dur        : Score.Duration := Orchestra (Index).Dur (Num);
                  Expect_Pit : Pitch;
                  Expect_Dur : Score.Duration;
                  Cumul_Dur  : Long_Duration  := Dur;
                  --  Look for the next note to be played
                  Get_Note (Inter, Instr, Pos, Dur, Expect_Pit, Expect_Dur);

                  --  If this note's pitch does not match, suppose that either
                  --  the player or the sensor missed a note and continue

                  while Pit /= Expect_Pit and then
                    --  Look for a note at the appropriate pitch
                    Dur <= Score.Duration'Last
                    --  If it cannot be found one measure in advance, give up
                     Cumul_Dur := Cumul_Dur + Expect_Dur;
                       (Inter, Instr, Pos, Cumul_Dur, Expect_Pit, Expect_Dur);
                  end loop;

                  if Pit = Expect_Pit then
                     --  A matching note was found, so update the state
                     Pos := Pos + Position (Cumul_Dur / Score.Duration'Last);
                     Dur := Expect_Dur;
                  end if;
            end Start_Of_Note;
         end select;
      end loop;

   end Concert;

end Conductor;

See CodePeer in Action!

In this video, AdaCore engineer Yannick Moy walks you through this month’s challenge using CodePeer and GNAT Programming Studio, GNAT Pro IDE.