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

Support for compiling to binary object #3232

Merged
merged 7 commits into from
Jan 11, 2023
Merged

Support for compiling to binary object #3232

merged 7 commits into from
Jan 11, 2023

Conversation

dkm
Copy link
Member

@dkm dkm commented Jan 4, 2022

Support for compiling until assembler, before invoking the linker (eg. gcc -c).
Basic support for displaying relocation information.

Currently only used in c/c++/d/ada/rust and may/will propagate to other lang/compilers later.

fixes #3222

Signed-off-by: Marc Poulhiès dkm@kataplop.net

@dkm dkm marked this pull request as draft January 4, 2022 21:03
@github-actions github-actions bot added the ui label Jan 4, 2022
@dkm
Copy link
Member Author

dkm commented Jan 4, 2022

This is still WIP and mostly of PoC.
Currently able to compile to binary object and match relocations and display (crudely) them.

reloc

Still have to figure out the best way to select this mode (vs current binary, execution, ...) and how to nicely display reloc info.

@dkm
Copy link
Member Author

dkm commented Jan 5, 2022

Relocation parsing fixed, no particular handling and reusing the text field of the asm lines.

Also, the checkboxes are greyed for incompatible choices (binaryobj vs binary/exec):
binary_dis

binaryobj_dis

@dkm
Copy link
Member Author

dkm commented Jan 5, 2022

I should probably find how to display relocation after the opcode hex value instead of before.

@dkm
Copy link
Member Author

dkm commented Jan 23, 2022

reloc

Updated few config files, should do some more tests. Screenshot showing what to expect.

@dkm
Copy link
Member Author

dkm commented Jan 23, 2022

I'll probably add some filters to hide/show relocations.

@partouf
Copy link
Contributor

partouf commented Apr 9, 2022

So far I like where this code is going. What do you want it to feature next?

@dkm
Copy link
Member Author

dkm commented May 3, 2022

So far I like where this code is going. What do you want it to feature next?

Sorry for being (very) silent lately, have been busy with other things. I think the prototype is mostly OK, but I would like to:

  • check for better parsing of relocation info from the objdump dump. Can't say I really understand how the asm layout is being done currently. I hacked something that looks ok-ish, but would like it to be "correct"
  • check it interacts correctly with existing stuff. Don't want to discover after the facts that it counter intuitive when X or Y is used
  • better test with different compilers/targets
  • (opt) only checked with C/C++/Ada, but could be enabled with other languages (at least Rust, but there needs to be an objdump available and defined).

I'm planning on reviving it, it's next in my todo list. It's not a dead PR :)

@RubenRBS
Copy link
Member

RubenRBS commented Nov 9, 2022

Do let us know if we can help with anything!

@partouf
Copy link
Contributor

partouf commented Jan 8, 2023

The objdump/parser changes will require changes to https://github.com/compiler-explorer/asm-parser
If you have some examples of in -> out for unittesting that will probably go a long way

@mattgodbolt mattgodbolt self-requested a review January 8, 2023 23:14
Copy link
Member

@mattgodbolt mattgodbolt left a comment

Choose a reason for hiding this comment

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

Some early comments: will take a look properly tomorrow! Thanks so much for this!!

lib/base-compiler.ts Outdated Show resolved Hide resolved
lib/compilers/ada.ts Show resolved Hide resolved
lib/compilers/rust.ts Show resolved Hide resolved
lib/parsers/asm-parser.ts Outdated Show resolved Hide resolved
static/panes/compiler.ts Outdated Show resolved Hide resolved
static/panes/compiler.ts Outdated Show resolved Hide resolved
static/panes/compiler.ts Outdated Show resolved Hide resolved
@dkm
Copy link
Member Author

dkm commented Jan 9, 2023

THanks for the review, will respin this later tonight!

lib/base-compiler.ts Outdated Show resolved Hide resolved
lib/base-compiler.ts Outdated Show resolved Hide resolved
const relocdata = match.groups.relocdata;
// value/addend matched but not used yet.
const match_value = relocdata.match(this.relocDataSymNameRe);
asm.push({
Copy link
Member Author

Choose a reason for hiding this comment

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

@partouf does that look the correct way to have the relocation displayed? I remember that when starting this I thought of having a dedicated structure for relocations... But then kept things simple like this.

Copy link
Contributor

@partouf partouf Jan 9, 2023

Choose a reason for hiding this comment

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

I think so. You could add the label/symbolname that's used (with https://github.com/compiler-explorer/compiler-explorer/blob/main/types/asmresult/asmresult.interfaces.ts#L31), but since it probably doesn't actually exist, it wouldn't be used anyway. So I think it's good enough.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok! So will replicate this in asm-parser

@RubenRBS RubenRBS self-requested a review January 10, 2023 21:11
@dkm
Copy link
Member Author

dkm commented Jan 10, 2023

The objdump/parser changes will require changes to https://github.com/compiler-explorer/asm-parser If you have some examples of in -> out for unittesting that will probably go a long way

Hopefully done in compiler-explorer/asm-parser#31

Copy link
Member

@mattgodbolt mattgodbolt left a comment

Choose a reason for hiding this comment

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

LGTM! Pending the merge and build of the asm-parser too (which I'm not sure how to do)

@@ -65,6 +65,7 @@ To specify a compilation request as a JSON document, post it as the appropriate
},
"filters": {
"binary": false,
"binaryObject": false,
Copy link
Member

Choose a reason for hiding this comment

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

Thanks 👍🏻

@partouf
Copy link
Contributor

partouf commented Jan 11, 2023

LGTM! Pending the merge and build of the asm-parser too (which I'm not sure how to do)

https://github.com/compiler-explorer/infra/blob/main/bin/yaml/tools.yaml#L66

I believe it's merge the PR in asm-parser repo - wait for the workflow to build the new version, then on admin do ce_install install --force asm-parser
and then restart the instances to take effect (new version + environment refresh)

@partouf
Copy link
Contributor

partouf commented Jan 11, 2023

Oh, I'm wrong, it might also require making a release here https://github.com/compiler-explorer/asm-parser/releases

@partouf partouf merged commit 6905568 into main Jan 11, 2023
@partouf partouf deleted the dkm/compile_binary_obj branch January 11, 2023 18:39
dkm added a commit that referenced this pull request Jan 11, 2023
With the merge of #3232, we need to distinguish "compile to binary
object" (compiler + assembler) and "compile to binary" (compiler + assembler +
linker).

Renaming "compile to binary" as "link to binary" is a more correct in this
context and less confusing.

We don't rename the internal field (kept as 'binary') because it's part of the
API and we can't break it.

Signed-off-by: Marc Poulhiès <dkm@kataplop.net>
@partouf
Copy link
Contributor

partouf commented Jan 11, 2023

this is now live

dkm added a commit that referenced this pull request Jan 12, 2023
Condition was badly changed in #3232 (compiling to binary objects)
dkm added a commit that referenced this pull request Jan 12, 2023
Condition was badly changed in #3232 (compiling to binary objects)
mattgodbolt pushed a commit that referenced this pull request Jan 24, 2023
Condition was badly changed in #3232 (compiling to binary objects)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[REQUEST]: be able to compile to object file and stop before linking
5 participants