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

Lexer raises `Fatal Error` on parsing `#9342101923012312312` #7165

Closed
vicuna opened this issue Mar 5, 2016 · 7 comments

Comments

Projects
None yet
2 participants
@vicuna
Copy link

commented Mar 5, 2016

Original bug ID: 7165
Reporter: @kayceesrk
Assigned to: @gasche
Status: closed (set by @xavierleroy on 2017-09-24T15:33:08Z)
Resolution: fixed
Priority: normal
Severity: minor
Version: 4.02.3
Target version: 4.03.1+dev
Fixed in version: 4.04.0 +dev / +beta1 / +beta2
Category: ~DO NOT USE (was: OCaml general)
Tags: afl
Monitored by: @kayceesrk @gasche

Bug description

ocamlc and ocamlopt crash with Fatal error: exception Failure("int_of_string") if the input file just contains #9342101923012312312. The int_of_string call at [0] fails due to overflow.
The failing test case was generated with afl instrumented OCaml [1].

[0]

{ update_loc lexbuf name (int_of_string num);

[1] ocamllabs/opam-repo-dev#23

Steps to reproduce

$ cat crash.ml
#9342101923012312312
$ ocamlc crash.ml
Fatal error: exception Failure("int_of_string")

File attachments

@vicuna

This comment has been minimized.

Copy link
Author

commented Mar 5, 2016

Comment author: @gasche

I proposed a fix for this issue in
#494

I have done some afl-fuzzing with OCaml myself, but I did not take the long and brave route of instrumenting the compiler output. I just compiled the C runtime with instrumentation, and tried to fuzz "input_value". I found plenty of crashes (something like 65 distinct path), but then found out that I completely lack the expertise to analyse them and say whether any of them should be fixed. (I think everyone agree that running input_value on untrusted input is bad form, but there are a range of possible bad behaviors that a buggy C runtime could allow that we may want to prevent nonetheless). Would there be a volunteer to look at those crash reports?

Also, why is the afl-instrumented ocamlopt variant not (yet) submitted as a pull request upstream? I would love to have an -afl-instrument flag in ocamlopt.

@vicuna

This comment has been minimized.

Copy link
Author

commented Mar 5, 2016

Comment author: @kayceesrk

Thanks for the fix. I'll prod stedolan to submit a PR upstream.

At the moment, most bugs found are in the lexer since fuzzing tends to explore in a breadth-first fashion. I would like to selectively compile different phases of the compiler with instrumentation to perform better testing.

@vicuna

This comment has been minimized.

Copy link
Author

commented Mar 5, 2016

Comment author: @gasche

I'm a bit skeptical about using afl-fuzz to test the compiler itself in general -- because it finds crashes, which are not the kind of defects I would be primarily concerned about. I suspect you may get more mileage out of a Csmith-style tool generating simple valid OCaml programs, and (for example) checking that the bytecode and native behavior coincide -- especially with flambda, that could be juicy. afl-fuzzing the runtime could be interesting, but I'm not sure how to do it -- we would need a tool reading input files that describe well-defined sequences of runtime calls.

(The afl-instrumentation of the compiler is of course very interesting, as it would allow to easily fuzz any OCaml programs, in particular those where finding crashes is important and interesting.)

@vicuna

This comment has been minimized.

Copy link
Author

commented Mar 5, 2016

Comment author: @stedolan

I definitely want the afl instrumentation widely available, but I'm not sure that upstreaming into the main OCaml distribution is the right choice (although I'm more than happy to do that if it's desired).

The issue is that you generally want to compile a large piece of software and its dependencies with instrumentation turned on. Telling all of the various OCaml build systems that they should pass a new -afl-instrument flag to ocamlopt is a bit painful. It seems easier to provide a compiler that can be installed with opam switch, which then builds everything with instrumentation turned on without having to muck around with build flags.

So, I'm not sure how best to make this available - possibly a switch in the main opam repo might be better than a flag in the stock compiler.

@vicuna

This comment has been minimized.

Copy link
Author

commented Mar 6, 2016

Comment author: @gasche

There are two orthogonal issues at play here: whether the instrumentation logic is included upstream or only in a fork, and the best trick to enable it globally.

If you can propose a clean pull request (I trust you can), it is useful/interesting to other people (count me in), and you can convince the maintainers to have it (I don't know about that part), it should definitely be submitted upstream. The best way to use it might still be to deploy a separate switch, but that switch would just toggle an upstream option by default.

Re. global deployment: I remember that Pierre Chambart and Mark Shinwell proposed, as part of the flambda work, a way to enable compiler options globally in a switch by setting a file somewhere (in ocamlc -where iirc.). I don't know what the status of this feature is. You should ask Mark if you have the opportunity to.

@vicuna

This comment has been minimized.

Copy link
Author

commented Mar 7, 2016

Comment author: @mshinwell

You should ask Pierre about the global config file. :)
I believe that code has been included in the 4.03 branch already.

@vicuna

This comment has been minimized.

Copy link
Author

commented Mar 11, 2016

Comment author: @stedolan

There is now a pull request for afl instrumentation at #504

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.