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

Add proper error for attempted use of undef slot #50556

Merged
merged 2 commits into from
Jul 16, 2023
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Add proper error for attempted use of undef slot
Fixes the segfault in #50518 and turns it into a proper error at
both the syntax level (to catch lowering generating bad slot references)
as well as at the codegen level (to catch e.g. bad generated functions
and opaque closures). However, note that the latter case is technically
undefined behavior, because we do not model the possibility that an
otherwise-defined argument could throw at access time. Of course,
throwing an error is allowable as undefined behavior and preferable
to a segfault.
Keno committed Jul 14, 2023
commit eb37e922434211ac99f8259a3e22891e10478f94
6 changes: 6 additions & 0 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
@@ -4921,6 +4921,12 @@ static jl_cgval_t emit_local(jl_codectx_t &ctx, jl_value_t *slotload)
size_t sl = jl_slot_number(slotload) - 1;
jl_varinfo_t &vi = ctx.slots[sl];
jl_sym_t *sym = slot_symbol(ctx, sl);
if (sym == jl_unused_sym) {
// This shouldn't happen in well-formed input, but let's be robust,
// since we otherwise cause undefined behavior here.
emit_error(ctx, "(INTERNAL ERROR): Tried to use `#undef#` argument.");
return jl_cgval_t();
}
return emit_varinfo(ctx, vi, sym);
}

5 changes: 3 additions & 2 deletions src/julia-syntax.scm
Original file line number Diff line number Diff line change
@@ -296,7 +296,7 @@
(if (eq? n '|#self#|) (gensy) n))
arg-names))))
(let ((body (insert-after-meta body ;; don't specialize on generator arguments
`((meta nospecialize ,@arg-names)))))
`((meta nospecialize ,@(map (lambda (idx) `(slot ,(+ idx 1))) (iota (length arg-names))))))))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC, there was previously a bug (and open issue) that vararg arguments didn't get nospecialize'd for macros. Any idea if this fixed it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. This is for generated functions.

`(block
(global ,name)
(function (call ,name ,@arg-names) ,body)))))
@@ -5051,7 +5051,8 @@ f(x) = yt(x)
(define slot-table (symbol-to-idx-map (map car (car (lam:vinfo lam)))))
(define sp-table (symbol-to-idx-map (lam:sp lam)))
(define (renumber-stuff e)
(cond ((symbol? e)
(cond ((eq? e UNUSED) (error "Attempted to use slot marked unused"))
((symbol? e)
(let ((idx (get slot-table e #f)))
(if idx
`(slot ,idx)
5 changes: 5 additions & 0 deletions test/syntax.jl
Original file line number Diff line number Diff line change
@@ -3507,3 +3507,8 @@ let x = 1 => 2
@test_throws ErrorException @eval a => b = 2
@test_throws "function Base.=> must be explicitly imported to be extended" @eval a => b = 2
end

# Test that bad lowering does not segfault (ref #50518)
@test_throws ErrorException("syntax: Attempted to use slot marked unused") @eval function funused50518(::Float64)
$(Symbol("#unused#"))
end