draperlaboratory / VIBES

Verified, Incremental, Binary Editing with Synthesis
MIT License
51 stars 1 forks source link

Simple C-Like Patches #87

Closed philzook58 closed 3 years ago

philzook58 commented 3 years ago

Adds FrontC parsing of patches Simple somewhat ad hoc translation of a small subset of C constructs.

I suspect my version of frontc is old. There are also strange warnings about cmx files when I build

Currently I directly inject the frontc parser ahead of the sexp parser. A failure of the frontc parser is assumed to imply that the input was actually in the sexp format, which is still supported. The error handling is not ideal.

[c_patch_to_sexp_string] ingests C-like code and outputs a new string representing a
vibes patch s-expression.
- `int x, y, z;` at the top of your patch to convert to `(var-decls x y z)`
-  Only if then elses of the form `if(cond_expr){goto l1;}else{goto l2;};` ->
    `(branch sexp_of_cond_expr (goto l1) (goto l2))`
- `goto foo;` -> `(goto foo)`
- `goto fallthrough;` -> `fallthrough`
- Supports operators +, -, *,, /, = <<, ++, !=, <, >, <=, >=
- Dereferencing `*a` -> `(load a)`
- Array indexing `a[j]` -> `(load (+ a j))`
- Constants like 0x7 are maintained as unadultered strings

Example supported C:

```
int x, y, z;
x = 0x7;
x = *y;
if(x > 0){
    goto fred;
} else{
    goto larry;
}
```

If FrontC fails to parse, it forwards the parse error. If FrontC parses but a construct
is not supported, failwith is called and crashes the program.
codyroux commented 3 years ago

Cool! Can you tell us what version of FrontC you used?

Also: what's the expectation here: to merge, or to discuss?

philzook58 commented 3 years ago

Discuss whether to merge. Are the non ideal choices acceptable. Will we replace this so immediately it isn't worth merging. Sanity check that this compiles on someone elses system. I suppose I don't particularly see a reason not to merge after reaching satisfactory conclusions. We have all sorts of "temporary" stuff in the code base and that's fine imo.

codyroux commented 3 years ago

I'll re-iterate my feelings in writing then: I don't think we should parse C, then create a string which is then processed and finally invoke Core constructs.

We should be going straight to Core.

ccasin commented 3 years ago

We should be going straight to Core.

This may be true, but it's fine not to do everything at once.

codyroux commented 3 years ago

This is adding work, rather than removing it. It should take about 1 hour to fix this to avoid the intermediate step. I'm happy to do it myself.

ccasin commented 3 years ago

This is adding work, rather than removing it. It should take about 1 hour to fix this to avoid the intermediate step. I'm happy to do it myself.

You're forgetting the work JT is currently doing which works on the sexps

codyroux commented 3 years ago

I actually did not forget: I was hoping we would resolve this "var defs as macros" story before we merged it with the C code.

The thing is that really the register allocator should be in charge of picking locations for variables, so we want the back end to handle all this, though we probably want to start by substituting in BIR first.

I understand we went the S-expression macro route in order to test the features and usability of the design. Let's do that, and re-design the thing before we merge with this FrontC stuff.

philzook58 commented 3 years ago

Subsumed by the new approach in #90