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 fine-grained control over Target Features #12

Open
daniel5151 opened this issue Aug 18, 2020 · 3 comments
Open

Support fine-grained control over Target Features #12

daniel5151 opened this issue Aug 18, 2020 · 3 comments
Labels
API-ergonomics Nothing's "broken", but the API could be improved design-required Getting this right will require some thought

Comments

@daniel5151
Copy link
Owner

daniel5151 commented Aug 18, 2020

Aside from specifying the core architecture of the target, the Target Description XML is also used to specify various features of the architecture. As the docs say: "Features are currently used to describe available CPU registers and the types of their contents"

e.g: for x86, the core registers are defined under the org.gnu.gdb.i386.core feature, while the optional SSE registers are defined under the org.gnu.gdb.i386.sse feature.

Currently, the Arch trait isn't very well suited to describing features, requiring a separate implementation for each set of features. i.e: if an architecture has 3 different features, A, B, and C, each with their own registers, there would have to be 3! (i.e: 6) different register structs to describe each set of features!

I'd like to preserve the current "described at compile time" approach to the Arch trait, and I think with a little bit of experimentation, it shouldn't be too hard to find some sort of ergonomic trait-based solution to the problem.


Some general ideas:

  • It might make sense to have the target_features_xml method accept a FnMut(&'static str) callback (instead of returning Option<'static str>, as that would allow building up an XML file in "chunks" based on which features are available.
@daniel5151 daniel5151 added the new-protocol-extension New feature or request label Aug 23, 2020
daniel5151 added a commit that referenced this issue Sep 13, 2020
After merging all the arch PRs, I realized that everyone did their own
thing when it comes to #[derive]s and wording on the various structs,
which made the docs feel kind-of disjointed. This PR makes everything
much more uniform.

For example, all implementors of the `Register` trait are now guaranteed
to implement `Debug`, `Default`, and `Clone`, the latter of which being
particularly useful when writing tests.

There is an argument to be made that `Eq` and `PartialEq` should also be
auto-derived, but aside from writing assert tests, I'm not too sure when
those would be useful?

Additionally, I've changed all instances of `struct FooArch;` to
`enum FooArch{}`, which ensures that `Arch` implementations are purely a
type level construct.

I may have to revert this at some point, depending on how #12 gets
tackled, but for now, having Arch implementations as enums makes more
sense.
daniel5151 added a commit that referenced this issue Sep 13, 2020
After merging all the arch PRs, I realized that everyone did their own
thing when it comes to #[derive]s and wording on the various structs,
which made the docs feel kind-of disjointed. This PR makes everything
much more uniform.

For example, all implementors of the `Register` trait are now guaranteed
to implement `Debug`, `Default`, `Clone`, `Eq`, and `PartialEq`.

Additionally, I've changed all instances of `struct FooArch;` to
`enum FooArch{}`, which ensures that `Arch` implementations are purely a
type level construct.

I may have to revert this at some point, depending on how #12 gets
tackled, but for now, having Arch implementations as enums makes more
sense.
daniel5151 added a commit that referenced this issue Sep 13, 2020
After merging all the arch PRs, I realized that everyone did their own
thing when it comes to #[derive]s and wording on the various structs,
which made the docs feel kind-of disjointed. This PR makes everything
much more uniform.

For example, all implementors of the `Register` trait are now guaranteed
to implement `Debug`, `Default`, `Clone`, and `PartialEq`. Note that
this does _not_ include `Eq`, as some architectures have explicit IEEE
floating point registers (i.e: PowerPC), which do not implement `Eq`.

Additionally, I've changed all instances of `struct FooArch;` to
`enum FooArch{}`, which ensures that `Arch` implementations are purely a
type level construct.

I may have to revert this at some point, depending on how #12 gets
tackled, but for now, having Arch implementations as enums makes more
sense.
@daniel5151 daniel5151 added the API-ergonomics Nothing's "broken", but the API could be improved label Sep 16, 2020
@daniel5151 daniel5151 removed the new-protocol-extension New feature or request label Oct 10, 2020
@daniel5151
Copy link
Owner Author

Hmm, I wonder if a quick-and-dirty solution might be to create some sort of "arch builder" macro?

Instead of exposing types that implement Arch directly, each arch exposes a bunch of different "feature" structures, which encode a feature's associated registers + xml string. Consumers of the crate will then invoke the arch builder macro with the set of features they're interested in, creating a brand-new Arch impl suited specifically for their project.

A rough sketch of what the end-user API might look like:

arch_builder! {
    name: CustomX86,
    arch: x86_64,
    features: [
        core, sse, segments, avx
    ]
}

Not sure what the implementation might look like, but this would fix the immediate problem of how to easily compose different Arch traits.

Of course, this approach isn't that great, since macro-based codegen is kinda hacky in my book. That said, I wanted to write this idea down before I forgot it, since it might be a good jumping off point in the future.

@daniel5151 daniel5151 added the design-required Getting this right will require some thought label Jul 22, 2021
@daniel5151
Copy link
Owner Author

So here's something interesting: https://sourceware.org/gdb/onlinedocs/gdb/Target-Description-Format.html#Inclusion

Turns out target description XMLs support using <xi:include href="foo.xml"/> to split the description into multiple files...

For example:

target.xml

<?xml version="1.0"?>
<!DOCTYPE target SYSTEM "gdb-target.dtd">
<target version="1.0">
  <architecture>i386:x86-64</architecture>
  <xi:include href="core64-regs.xml"/>
</target>

core64-regs.xml

<?xml version="1.0"?>
<!DOCTYPE target SYSTEM "gdb-target.dtd">
<feature name="org.gnu.gdb.i386.core">
    ...
</feature>

I'm definitely going to update the TargetDescriptionXmlOverride extension to support custom annexes, but I think it might be very useful to somehow leverage this functionality to support fine grained target features...

@ptosi
Copy link
Contributor

ptosi commented Aug 17, 2022

I plan on working on this by implementing #109 (comment) i.e. making the Arch provide a standard XML <xi:include>-ing all its (Arch-defined + GDB-standard) features and make the gdbstub backend forward <xi:include> callbacks from the GDB client to the Target, giving it the opportunity, for each feature, to return the associated Arch-defined XML, its own (don't really have a use-case for this... yet?), or None.

This will help with unifying the MIPS architecture and trimming the ridiculously large list of AArch64 system registers that's reported to the GDB client (e.g. because the HW doesn't support an optional feature or because the GDB server can assume that the code won't ever run at certain ELs).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API-ergonomics Nothing's "broken", but the API could be improved design-required Getting this right will require some thought
Projects
None yet
Development

No branches or pull requests

2 participants