[PRISM] Use Prism checksum for hash comparison

Merged: https://github.com/ruby/ruby/pull/19429

ast.c

@@ -284,17 +284,6 @@ iseq_compiled_by_prism_p(rb_execution_context_t *ec, VALUE self)
     return RBOOL(ISEQ_BODY(rb_iseqw_to_iseq(self))->prism);
 }
 
-static VALUE
-source_hash_of(rb_execution_context_t *ec, VALUE self, VALUE str)
-{
-    StringValue(str);
-
-    rb_source_hash_state_t state;
-    rb_source_hash_init(&state);
-    rb_source_hash_update(&state, (const uint8_t *)RSTRING_PTR(str), (size_t)RSTRING_LEN(str));
-    return ULL2NUM(rb_source_hash_finalize(&state));
-}
-
 static VALUE
 node_id_for_backtrace_location(rb_execution_context_t *ec, VALUE module, VALUE location)
 {

ast.rb

@@ -374,14 +374,11 @@ class RubyVM::InstructionSequence
       result = source ? Ruby::Prism.parse(source, version: "current") : Ruby::Prism.parse_file(path, version: "current")
       return nil unless result.success?
 
-      # Hash exactly the bytes that prism parsed. The data section after an
-      # __END__ marker is not part of the code, so the hash covers the source
-      # only up to the end of the __END__ line.
-      code = result.source.source
-      if (data_loc = result.data_loc) && (eol = code.byteindex("\n", data_loc.start_offset))
-        code = code.byteslice(0, eol + 1)
-      end
-      return nil unless Primitive.source_hash_of(code) == source_hash
+      # The instruction sequence stores a checksum of 0 as 1, because a source
+      # hash of 0 means that there is no source hash (see pm_parse_process).
+      checksum = result.source.checksum
+      checksum = 1 if checksum == 0
+      return nil if checksum != source_hash
 
       root = result.value
     else

common.mk

@@ -123,7 +123,8 @@ LIBPRISM_OBJS = \
 		prism/stringy.$(OBJEXT) \
 		prism/strncasecmp.$(OBJEXT) \
 		prism/strpbrk.$(OBJEXT) \
-		prism/tokens.$(OBJEXT)
+		prism/tokens.$(OBJEXT) \
+		prism/xxh3.$(OBJEXT)
 
 EXTPRISM_OBJS = prism/api_node.$(OBJEXT) \
 		prism/extension.$(OBJEXT) \

depend

@@ -136,6 +136,7 @@ prism/stringy.$(OBJEXT): $(top_srcdir)/prism/stringy.c
 prism/strncasecmp.$(OBJEXT): $(top_srcdir)/prism/strncasecmp.c
 prism/strpbrk.$(OBJEXT): $(top_srcdir)/prism/strpbrk.c
 prism/tokens.$(OBJEXT): $(top_srcdir)/prism/tokens.c
+prism/xxh3.$(OBJEXT): $(top_srcdir)/prism/xxh3.c
 prism_init.$(OBJEXT): {$(VPATH)}prism_init.c
 proc.$(OBJEXT): {$(VPATH)}proc.c
 process.$(OBJEXT): {$(VPATH)}process.c

prism_compile.c

@@ -10837,36 +10837,24 @@ pm_warning_emit_callback(const pm_diagnostic_t *diagnostic, void *data) {
 }
 
 /**
- * Parse the parse result and raise a Ruby error if there are any syntax errors.
- * It returns an error if one should be raised. It is assumed that the parse
- * result object is zeroed out.
- */
-/**
- * Compute the hash of the source code that was parsed. The data section after
- * an __END__ marker is not part of the code, so the hash covers the source
- * only up to the end of the __END__ line, which also matches the range that
- * parse.y hashes.
+ * Returns the source hash of the source that was parsed, as stored on
+ * instruction sequences. A source hash of 0 means that there is no source hash,
+ * so the checksum is remapped to another value in that case.
+ * Prism::Source#checksum is compared against this value in ast.rb, which
+ * applies the same remapping.
  */
 static uint64_t
 pm_source_hash(const pm_parser_t *parser)
 {
-    const uint8_t *start = pm_parser_start(parser);
-    const uint8_t *end = pm_parser_end(parser);
-    const pm_location_t *data_loc = pm_parser_data_loc(parser);
-
-    if (data_loc->length != 0) {
-        const uint8_t *cursor = start + data_loc->start;
-        while (cursor < end && *cursor != '\n') cursor++;
-        if (cursor < end) cursor++;
-        end = cursor;
-    }
-
-    rb_source_hash_state_t state;
-    rb_source_hash_init(&state);
-    rb_source_hash_update(&state, start, (size_t) (end - start));
-    return rb_source_hash_finalize(&state);
+    uint64_t checksum = pm_parser_checksum(parser);
+    return checksum == 0 ? 1 : checksum;
 }
 
+/**
+ * Parse the parse result and raise a Ruby error if there are any syntax errors.
+ * It returns an error if one should be raised. It is assumed that the parse
+ * result object is zeroed out.
+ */
 static VALUE
 pm_parse_process(pm_parse_result_t *result, pm_node_t *node, VALUE *script_lines)
 {
@@ -11199,9 +11187,33 @@ pm_node_find(const pm_node_t *node, void *data)
     return context->node == NULL;
 }
 
+/**
+ * Returns whether parsing the given source produces the given source hash.
+ */
+bool
+pm_source_hash_matches(VALUE source, VALUE filepath, uint64_t source_hash)
+{
+    pm_parse_result_t result;
+    pm_parse_result_init(&result);
+    pm_parse_string(&result, source, filepath, NULL);
+
+    bool matches = result.parser != NULL && pm_source_hash(result.parser) == source_hash;
+    pm_parse_result_free(&result);
+
+    RB_GC_GUARD(source);
+    RB_GC_GUARD(filepath);
+    return matches;
+}
+
+/**
+ * Find the location of the node with the given node ID in the given source.
+ * Raises if the source does not have the given source hash, which is checked
+ * before any syntax error is raised so that source modified into invalid Ruby
+ * is reported as modified.
+ */
 bool
 pm_node_source_location(VALUE source, VALUE filepath, int start_line,
-                        int node_id, rb_code_location_t *location)
+                        int node_id, uint64_t source_hash, rb_code_location_t *location)
 {
     pm_parse_result_t result;
     pm_parse_result_init(&result);
@@ -11209,6 +11221,11 @@ pm_node_source_location(VALUE source, VALUE filepath, int start_line,
     pm_options_line_set(result.options, start_line);
     VALUE error = pm_parse_string(&result, source, filepath, NULL);
 
+    if (result.parser != NULL && pm_source_hash(result.parser) != source_hash) {
+        pm_parse_result_free(&result);
+        rb_raise(rb_eRuntimeError, "source has been modified");
+    }
+
     if (!NIL_P(error)) {
         pm_parse_result_free(&result);
         rb_exc_raise(error);

prism_compile.h

@@ -191,8 +191,10 @@ VALUE pm_parse_string(pm_parse_result_t *result, VALUE source, VALUE filepath, V
 VALUE pm_parse_stdin(pm_parse_result_t *result);
 void pm_options_version_for_current_ruby_set(pm_options_t *options);
 void pm_parse_result_free(pm_parse_result_t *result);
+bool pm_source_hash_matches(VALUE source, VALUE filepath, uint64_t source_hash);
 bool pm_node_source_location(VALUE source, VALUE filepath, int start_line,
-                             int node_id, struct rb_code_location_struct *location);
+                             int node_id, uint64_t source_hash,
+                             struct rb_code_location_struct *location);
 
 rb_iseq_t *pm_iseq_new(pm_scope_node_t *node, VALUE name, VALUE path, VALUE realpath, const rb_iseq_t *parent, enum rb_iseq_type, int *error_state);
 rb_iseq_t *pm_iseq_new_top(pm_scope_node_t *node, VALUE name, VALUE path, VALUE realpath, const rb_iseq_t *parent, int *error_state);

ruby_parser.c

@@ -1096,10 +1096,13 @@ parser_aset_script_lines_for(VALUE path, rb_parser_ary_t *lines)
     rb_hash_aset(hash, path, script_lines);
 }
 
-/* The source hash API currently computes FNV-1a, but the algorithm is an
- * implementation detail. The hash values are only ever compared against
- * hashes computed by the same interpreter, so the algorithm can be changed
- * freely between releases. */
+/* The source hash API hashes the source that parse.y reads. Code parsed by
+ * prism is hashed by prism itself (see pm_parser_checksum), and the two are
+ * never compared with each other, so they need not use the same algorithm.
+ *
+ * This currently computes FNV-1a, but the algorithm is an implementation
+ * detail. The hash values are only ever compared against hashes computed by the
+ * same interpreter, so the algorithm can be changed freely between releases. */
 void
 rb_source_hash_init(rb_source_hash_state_t *state)
 {

vm_backtrace.c

@@ -522,7 +522,10 @@ iseq_from_e_script_p(const rb_iseq_t *iseq, VALUE path, uint64_t source_hash)
         !RTEST(rb_e_script)) {
         return false;
     }
-    if (!location_source_hash_matches(rb_e_script, source_hash)) return false;
+    bool matches = ISEQ_BODY(iseq)->prism ?
+        pm_source_hash_matches(rb_e_script, path, source_hash) :
+        location_source_hash_matches(rb_e_script, source_hash);
+    if (!matches) return false;
 
     const rb_iseq_t *source_iseq = iseq;
     for (; source_iseq; source_iseq = ISEQ_BODY(source_iseq)->parent_iseq) {
@@ -536,7 +539,7 @@ iseq_from_e_script_p(const rb_iseq_t *iseq, VALUE path, uint64_t source_hash)
     rb_code_location_t source_location;
     bool found;
     if (ISEQ_BODY(iseq)->prism) {
-        found = pm_node_source_location(rb_e_script, path, 1, node_id, &source_location);
+        found = pm_node_source_location(rb_e_script, path, 1, node_id, source_hash, &source_location);
     }
     else {
         found = rb_ast_node_source_location(
@@ -632,7 +635,9 @@ location_source_range_m(VALUE self)
     if (NIL_P(parser_path)) {
         parser_path = rb_str_new_cstr("(eval)");
     }
-    if (!location_source_hash_matches(source, source_hash)) {
+    // Prism hashes the source itself, so for prism-compiled code the source
+    // hash is checked by pm_node_source_location.
+    if (!ISEQ_BODY(iseq)->prism && !location_source_hash_matches(source, source_hash)) {
         rb_raise(rb_eRuntimeError, "source has been modified");
     }
 
@@ -645,6 +650,7 @@ location_source_range_m(VALUE self)
             parser_path,
             first_lineno,
             node_id,
+            source_hash,
             &code_location
         );
     }