Validate that function with results return something on all code paths.

[duffy-ocraven]

I have an earlier, not-trimmed down situation, resulting reproducibly with a concluding print of: Illegal instruction: 4. I'm trying to reproduce that.

3065 /usr/local/etc/tmp/spicy$cat ~/Documents/work/Corelight/projects/SS7/cap-voice-chantilly.pcap   | build/bin/spicy-driver ../Issue_210_illegal_instr_from_2696.spicy
Issue_210__illegal_instruction.cc:4425:1: warning: control reaches end of non-void function
}
^
Issue_210__illegal_instruction.cc:4442:1: warning: control reaches end of non-void function
}
^
2 warnings generated.
{
  "Log Location Information": [
    "Field Name": {
      "Packet Timestamp Raw",
      "ageOfLocationInformation",
      "timeAndTimezone Raw",
      "Timestamp",
      "Time since epoch seconds",
      "IMSI",
      "MSISDN",
      "cellGlobalIdOrServiceAreaIdOrLAI",
      "IMEI",
      "iMSI: "0486794635243",
Illegal instruction: 4

What I do have so far are these which are also of potential interest, though the segmentation fault is almost certainly from not initializing parameter named label, into the public type.

module Issue_210_illegal_instruction;
public type wasntEnoughToShowIllegalInstr = unit( label: string) {
 contentExceptLast: bytes &size = 1;
 lastbyte: uint8;
  on %init {
    label_then_BCD(label, self.contentExceptLast.sub(1, |self.contentExceptLast|), self.lastbyte);
  }
};

function label_then_BCD(inout label: string, contentExceptLast: bytes, lastbyte: uint8) : string {
  if ( 0xF0 != lastbyte & 0xF0) {
    label = label + "%0X\x22," % (uint32(0) + lastbyte >> 4,);
  }
#, contentExceptLast: bytes, lastbyte: uint8) : string {  local unSignExtended: uint16;
  print "%s" % label;
#  return label;
}

reports

3084 /usr/local/etc/tmp/spicy$printf "12\n" | build/bin/spicy-driver ../Issue_210_control_path.spicy
Issue_210_illegal_instruction.cc:88:1: warning: control reaches end of non-void function
}
^
1 warning generated.
Segmentation fault: 11

Slight variant: adding a string return type : string, does too, though with a different but similar warning.

3079 /usr/local/etc/tmp/spicy$printf "12\n" | build/bin/spicy-driver ../Issue_210_illegal_instr_from_2696.spicy
Issue_210_illegal_instruction.cc:151:169: warning: control reaches end of non-void function
static auto __hlt::Issue_210_illegal_instruction::label_then_BCD(std::string& label) -> std::string { hilti::rt::print(hilti::rt::fmt(std::string("%s"), label), true); }

[duffy-ocraven]

Note in the difference between the two syntax error texts, the larger has:

...std::string& label) -> std::string { hilti::rt::print(hilti::rt::fmt(std::string("%s"), label), true); }

which has surfaced the hilti::rt:: qualified print and the hilti::rt:: qualified fmt, in a manner that could be considered a layer-violation. That it there recapitulates the final statement as though that is the return signature, is probably a consequence of the lack of a return statement.

[duffy-ocraven]

A short version which reproducibly outputs

3099 /usr/local/etc/tmp/spicy$printf "12\n" | build/bin/spicy-driver ../Issue_210_end_another_way.spicy
Issue_210_illegal_instruction.cc:165:137: warning: control reaches end of non-void function
static auto __hlt::Issue_210_illegal_instruction::label_then_BCD(std::string& label) -> std::string { label = label + std::string("!"); }
                                                                                                                                        ^
1 warning generated.
Illegal instruction: 4

recurred in my experimentation, so I've provided that here.

module Issue_210_illegal_instruction;

#2696 /usr/local/etc/tmp/spicy$cat ~/Documents/work/Corelight/projects/SS7/cap-voice-chantilly.pcap | build/bin/spicy-driver ../JSON_LogLocationBCD.spicy
#SS7LayersPcapOrNg.cc:4425:1: warning: control reaches end of non-void function
#      "IMEI",
#      "iMSI: "0486794635243",
#Illegal instruction: 4

public type enoughToShowSegFault = unit{
    var label: string;
    on %init {
      self.label = "";
      print label_then_BCD(self.label) ; # , contentExceptLast.sub(1, |contentExceptLast|), lastbyte);
    }
};

function label_then_BCD(inout label: string) : string {
  label = label + "!";
#  return label;
}

[rsmmr]

I can reproduce with the short example. Current master gives:

printf "12\n" | ./bin/spicy-driver a.spicy
Issue_210_illegal_instruction.cc:179:1: warning: control reaches end of non-void function
}
^
1 warning generated.
free(): invalid pointer
Aborted

However, it really just seems to be that missing return, if I put that in, it's good. So we'll need validation that all exits from a function return something. That's probably not quite trivial ...

[bbannier]

Note that not returning a value from a non-void function triggers UB in C++ so this is something we definitely should catch during validation, or add semantics to Spicy to always ensure that a possibly default-initialized return value is generated (the first looks clearer to me, at least for now).

If we do not handle this case before hitting C++ we loose the ability to reason about the generated parsers.

[rsmmr]

Ack, it's pretty important. On the downside, this will need full control flow tracking, so it's a bit of work.