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 code to prevent segfault during Eval of thunk Expr #35429

Merged
merged 7 commits into from
Jan 28, 2023

Conversation

ssikdar1
Copy link
Contributor

@ssikdar1 ssikdar1 commented Apr 10, 2020

Description

Trying to address issue #32315.

I have a feeling this isn't correct, but trying to code against the situation where a :thunk Expr is passed to Eval with incorrect values.

For example the following now would have an error returned where they previously segfaulted.

julia> Main.eval(Expr(:thunk, x->x+9))
ERROR: syntax: malformed "thunk" statement
Stacktrace:
 [1] top-level scope at none:1
 [2] eval at ./boot.jl:331 [inlined]
 [3] eval(::Expr) at ./client.jl:452
 [4] top-level scope at REPL[1]:1

julia> Main.eval(Expr(:thunk, Meta.parse("x=17")))
ERROR: null stmt in body_attributes
Stacktrace:
 [1] eval at ./boot.jl:331 [inlined]
 [2] eval(::Expr) at ./client.jl:452
 [3] top-level scope at REPL[2]:1

Although the following does still segfault:

julia> Main.eval(Expr(:thunk, Meta.parse("17")))

signal (11): Segmentation fault: 11
in expression starting at none:1
body_attributes at /Users/ssikdar1/julia/src/toplevel.c:369 [inlined]
jl_toplevel_eval_flex at /Users/ssikdar1/julia/src/toplevel.c:801
jl_toplevel_eval at /Users/ssikdar1/julia/src/toplevel.c:834 [inlined]
jl_toplevel_eval_in at /Users/ssikdar1/julia/src/toplevel.c:857
eval at ./boot.jl:331 [inlined]
eval at ./client.jl:452
jl_apply at /Users/ssikdar1/julia/src/./julia.h:1691 [inlined]
do_call at /Users/ssikdar1/julia/src/interpreter.c:369
eval_stmt_value at /Users/ssikdar1/julia/src/interpreter.c:409 [inlined]

Closes #32315

@ssikdar1 ssikdar1 changed the title add code to prevent segfault add code to prevent segfault during Eval of thunk Expr Apr 10, 2020
@JeffBezanson
Copy link
Sponsor Member

Yes, I don't think this is correct because it doesn't actually check the structure. It checks much later, when having the wrong structure might have resulted in some null pointers, but only if you're lucky.

src/toplevel.c Outdated
@@ -791,7 +794,11 @@ jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_value_t *e, int
thk = (jl_code_info_t*)jl_exprarg(ex, 0);
assert(jl_is_code_info(thk));
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

I think it would be enough to turn this assert into an error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed the assert to error in a7c56d7

@ssikdar1
Copy link
Contributor Author

ssikdar1 commented Apr 18, 2020

@JeffBezanson Changed the assert to error in a7c56d7.

Now all 3 of the examples no longer segfault.

julia> Main.eval(Expr(:thunk, Meta.parse("17")))
ERROR: malformed "thunk" statement
Stacktrace:
 [1] top-level scope at none:1
 [2] eval at ./boot.jl:331 [inlined]
 [3] eval(::Expr) at ./client.jl:452
 [4] top-level scope at REPL[1]:1

julia> Main.eval(Expr(:thunk, Meta.parse("x=17")))
ERROR: malformed "thunk" statement
Stacktrace:
 [1] top-level scope at none:1
 [2] eval at ./boot.jl:331 [inlined]
 [3] eval(::Expr) at ./client.jl:452
 [4] top-level scope at REPL[1]:1

julia> Main.eval(Expr(:thunk, x->x+9))
ERROR: malformed "thunk" statement
Stacktrace:
 [1] top-level scope at none:1
 [2] eval at ./boot.jl:331 [inlined]
 [3] eval(::Expr) at ./client.jl:452
 [4] top-level scope at REPL[2]:1

@vtjnash vtjnash added the merge me PR is reviewed. Merge when all tests are passing label Jan 24, 2023
@IanButterworth IanButterworth merged commit 7d4309c into JuliaLang:master Jan 28, 2023
@IanButterworth IanButterworth removed the merge me PR is reviewed. Merge when all tests are passing label Jan 28, 2023
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

Successfully merging this pull request may close these issues.

Evaluation of "Thunk"-Expression results in EXCEPTION_ACCESS_VIOLATION
5 participants