Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rebis-dev: build_meta_predicate_clause triggers UB #2763

Open
adri326 opened this issue Jan 9, 2025 · 1 comment
Open

rebis-dev: build_meta_predicate_clause triggers UB #2763

adri326 opened this issue Jan 9, 2025 · 1 comment

Comments

@adri326
Copy link
Contributor

adri326 commented Jan 9, 2025

I noticed a segfault in the CIs, and after running miri on that test case for a whopping 12 hours, it managed to narrow down the issue onto build_meta_predicate_clause calling term_predicate_key(loader.machine_heap(), subterm_loc), where subterm_loc >= loader.machine_heap().cell_len().

The issue can be easily reproduced by adding a bounds check in impl std::ops::Index for Heap on rebis-dev: loads of tests suddenly break as a result, all emanating from build_meta_predicate_clause.

I'm not sure what build_meta_predicate_clause is supposed to do when it's called in this environment. For instance, it gets called with term.focus == 8373 and arity == 1 on the following heap segment:

    8355 -> HeapCellValue { tag: Var, value: 8355, m: false, f: false }
    8356 -> HeapCellValue { tag: Atom, name: "once", arity: 1, m: false, f: false }
    8357 -> HeapCellValue { tag: Var, value: 8355, m: false, f: false }
    8358 -> HeapCellValue { tag: Str, value: 8356, m: false, f: false }
    8359 -> HeapCellValue { tag: Atom, name: ":-", arity: 0, m: false, f: false }
    8360 -> HeapCellValue { tag: Atom, name: "call", arity: 0, m: false, f: false }
    8361 -> HeapCellValue { tag: Var, value: 8355, m: false, f: false }
    8362 -> HeapCellValue { tag: Atom, name: "call", arity: 1, m: false, f: false }
    8363 -> HeapCellValue { tag: Var, value: 8355, m: false, f: false }
    8364 -> HeapCellValue { tag: Str, value: 8362, m: false, f: false }
    8365 -> HeapCellValue { tag: Atom, name: "!", arity: 0, m: false, f: false }
    8366 -> HeapCellValue { tag: Atom, name: ",", arity: 2, m: false, f: false }
    8367 -> HeapCellValue { tag: Var, value: 8364, m: false, f: false }
    8368 -> HeapCellValue { tag: Var, value: 8365, m: false, f: false }
    8369 -> HeapCellValue { tag: Str, value: 8366, m: false, f: false }
    8370 -> HeapCellValue { tag: Atom, name: ":-", arity: 2, m: false, f: false }
    8371 -> HeapCellValue { tag: Var, value: 8358, m: false, f: false }
    8372 -> HeapCellValue { tag: Var, value: 8369, m: false, f: false }
    8373 -> HeapCellValue { tag: Str, value: 8370, m: false, f: false }
    8374 -> HeapCellValue { tag: Atom, name: "repeat", arity: 0, m: false, f: false }

From manually parsing it, we get that 8373 corresponds to once(G) :- call(G), !. or, in polish form, ':-'(once(G), ','(call(G), !)), but from inspecting the code calling build_meta_predicate_clause, I get that it is currently trying to expand the meta-predicate call/1.

As of now, build_meta_predicate_clause doesn't get fed with the proper address of the call (here 8362, the term argument to clause_to_query_term), and does replacements in incoherent locations (namely the arguments of ':-').

This might be the cause of #2745.

I'm working on a fix, but I wouldn't mind help :)

@adri326
Copy link
Contributor Author

adri326 commented Jan 9, 2025

Okay, it looks like #2745 originates from somewhere else, since the clause given to build_meta_predicate_clause is already populated with module: prefixes, which puzzles me as to what build_meta_predicate_clause does. Making it return nothing passes all tests.

adri326 added a commit to adri326/scryer-prolog that referenced this issue Jan 9, 2025
I don't know if that function is still needed, as making it return Default::default()
still passes all tests.

Nonetheless, this is now one less source of segfaults.

Fixes mthom#2763
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant