Updated to bitflags 2.2.1. #138

qwandor · 2023-04-26T14:36:27Z

Description

This updates the bitflags dependency to the latest version. As it is a major version change, some small code changes are required.

API Stability

This PR does not require a breaking API change

This is a breaking API change, because it changes the API of the public types HostIoOpenFlags and HostIoOpenMode which are generated by the bitflags! macro.

Checklist

Documentation
- Ensured any public-facing rustdoc formatting looks good (via cargo doc)
- (if appropriate) Added feature to "Debugging Features" in README.md
Validation
- Included output of running examples/armv4t with RUST_LOG=trace + any relevant GDB output under the "Validation" section below
- Included output of running ./example_no_std/check_size.sh before/after changes under the "Validation" section below
If implementing a new protocol extension IDET
- Included a basic sample implementation in examples/armv4t
- IDET can be optimized out (confirmed via ./example_no_std/check_size.sh)
- OR implementation requires introducing non-optional binary bloat (please elaborate under "Description")
If upstreaming an Arch implementation
- I have tested this code in my project, and to the best of my knowledge, it is working as intended.

Validation

GDB output

(gdb) info mem
Using memory regions provided by the target.
Num Enb Low Addr   High Addr  Attrs 
0   y   0x00000000 0x100000000 rw nocache

armv4t output

$ RUST_LOG=trace cargo run --example armv4t --features=std
    Finished dev [unoptimized + debuginfo] target(s) in 0.02s
     Running `/usr/local/google/home/qwandor/rust/gdbstub/target/debug/examples/armv4t`
loading section ".text" into memory from [0x55550000..0x55550078]
Setting PC to 0x55550000
Waiting for a GDB connection on "127.0.0.1:9001"...
Debugger connected from 127.0.0.1:52612
 TRACE gdbstub::protocol::recv_packet > <-- +
 TRACE gdbstub::protocol::recv_packet > <-- $qSupported:multiprocess+;swbreak+;hwbreak+;qRelocInsn+;fork-events+;vfork-events+;exec-events+;vContSupported+;QThreadEvents+;no-resumed+;memory-tagging+;xmlRegisters=i386#77
 TRACE gdbstub::protocol::response_writer > --> $PacketSize=1000;vContSupported+;multiprocess+;QStartNoAckMode+;ReverseContinue+;ReverseStep+;QDisableRandomization+;QEnvironmentHexEncoded+;QEnvironmentUnset+;QEnvironmentReset+;QStartupWithShell+;QSetWorkingDir+;swbreak+;hwbreak+;QCatchSyscalls+;qXfer:features:read+;qXfer:memory-map:read+;qXfer:exec-file:read+;qXfer:auxv:read+#fa
 TRACE gdbstub::protocol::recv_packet     > <-- +
 TRACE gdbstub::protocol::recv_packet     > <-- $vMustReplyEmpty#3a
 INFO  gdbstub::stub::core_impl           > Unknown command: Ok("vMustReplyEmpty")
 TRACE gdbstub::protocol::response_writer > --> $#00
 TRACE gdbstub::protocol::recv_packet     > <-- +
 TRACE gdbstub::protocol::recv_packet     > <-- $QStartNoAckMode#b0
 TRACE gdbstub::protocol::response_writer > --> $OK#9a
 TRACE gdbstub::protocol::recv_packet     > <-- +
 TRACE gdbstub::protocol::recv_packet     > <-- $Hgp0.0#ad
 TRACE gdbstub::protocol::response_writer > --> $OK#9a
 TRACE gdbstub::protocol::recv_packet     > <-- $qXfer:features:read:target.xml:0,ffb#79
 TRACE gdbstub::protocol::response_writer > --> $m<?xml version="1.0"?>
<!DOCTYPE target SYSTEM "gdb-target.dtd">
<target version="1.0">
    <architecture>armv4t</architecture>
    <feature name="org.gnu.gdb.arm.core">
        <vector id="padding" type="uint32" count="25"/>

        <reg name="r0" bitsize="32" type="uint32"/>
        <reg name="r1" bitsize="32" type="uint32"/>
        <reg name="r2" bitsize="32" type="uint32"/>
        <reg name="r3" bitsize="32" type="uint32"/>
        <reg name="r4" bitsize="32" type="uint32"/>
        <reg name="r5" bitsize="32" type="uint32"/>
        <reg name="r6" bitsize="32" type="uint32"/>
        <reg name="r7" bitsize="32" type="uint32"/>
        <reg name="r8" bitsize="32" type="uint32"/>
        <reg name="r9" bitsize="32" type="uint32"/>
        <reg name="r10" bitsize="32" type="uint32"/>
        <reg name="r11" bitsize="32" type="uint32"/>
        <reg name="r12" bitsize="32" type="uint32"/>
        <reg name="sp" bitsize="32" type="data_ptr"/>
        <reg name="lr" bitsize="32"/>
        <reg name="pc" bitsize="32" type="code_ptr"/>

        <!--
            For some reason, my version of `gdb-multiarch` doesn't seem to
            respect "regnum", and will not parse this custom target.xml unless I
            manually include the padding bytes in the target description.

            On the bright side, AFAIK, there aren't all that many architectures
            that use padding bytes. Heck, the only reason armv4t uses padding is
            for historical reasons (see comment below).

            Odds are if you're defining your own custom arch, you won't run into
            this issue, since you can just lay out all the registers in the
            correct order.
        -->
        <reg name="padding" type="padding" bitsize="32"/>

        <!-- The CPSR is register 25, rather than register 16, because
        the FPA registers historically were placed between the PC
        and the CPSR in the "g" packet. -->
        <reg name="cpsr" bitsize="32" regnum="25"/>
    </feature>
    <xi:include href="extra.xml"/>
</target>#38
 TRACE gdbstub::protocol::recv_packet     > <-- $qXfer:features:read:target.xml:80d,ffb#15
 TRACE gdbstub::protocol::response_writer > --> $l#6c
 TRACE gdbstub::protocol::recv_packet     > <-- $qXfer:features:read:extra.xml:0,ffb#16
 TRACE gdbstub::protocol::response_writer > --> $m<?xml version="1.0"?>
<!DOCTYPE target SYSTEM "gdb-target.dtd">
<feature name="custom-armv4t-extension">
    <!--
        maps to a simple scratch register within the emulator. the GDB
        client can read the register using `p }custom` and set it using
        `set }custom=1337`
    -->
    <reg name="custom" bitsize="32" type="uint32"/>

    <!--
        pseudo-register that return the current time when read.

        notably, i've set up the target to NOT send this register as part of
        the regular register list, which means that GDB will fetch/update
        this register via the 'p' and 'P' packets respectively
    -->
    <reg name="time" bitsize="32" type="uint32"/>

    <!--
        pseudo-register that is always unavailable.

        it is supposed to be reported as 'x'-ed bytes in replies to 'p' packets
        and shown by the GDB client as "<unavailable>".
    -->
    <reg name="unavailable" bitsize="32" type="uint32"/>
</feature>#7d
 TRACE gdbstub::protocol::recv_packet     > <-- $qXfer:features:read:extra.xml:3c5,ffb#b1
 TRACE gdbstub::protocol::response_writer > --> $l#6c
 TRACE gdbstub::protocol::recv_packet     > <-- $qXfer:auxv:read::0,ffb#d8
 TRACE gdbstub::protocol::response_writer > --> $m#bb
 TRACE gdbstub::protocol::recv_packet     > <-- $qXfer:auxv:read::8,ffb#e0
 TRACE gdbstub::protocol::response_writer > --> $l#6c
 TRACE gdbstub::protocol::recv_packet     > <-- $qTStatus#49
 INFO  gdbstub::stub::core_impl           > Unknown command: Ok("qTStatus")
 TRACE gdbstub::protocol::response_writer > --> $#00
 TRACE gdbstub::protocol::recv_packet     > <-- $?#3f
 TRACE gdbstub::protocol::response_writer > --> $T05thread:p01.01;#06
 TRACE gdbstub::protocol::recv_packet     > <-- $qfThreadInfo#bb
 TRACE gdbstub::protocol::response_writer > --> $mp01.01#cd
 TRACE gdbstub::protocol::recv_packet     > <-- $qsThreadInfo#c8
 TRACE gdbstub::protocol::response_writer > --> $l#6c
 TRACE gdbstub::protocol::recv_packet     > <-- $qAttached:1#fa
GDB queried if it was attached to a process with PID 1
 TRACE gdbstub::protocol::response_writer > --> $1#31
 TRACE gdbstub::protocol::recv_packet     > <-- $qXfer:exec-file:read:1:0,ffb#b7
 TRACE gdbstub::protocol::response_writer > --> $m/test.elf#c1
 TRACE gdbstub::protocol::recv_packet     > <-- $qXfer:exec-file:read:1:9,ffb#c0
 TRACE gdbstub::protocol::response_writer > --> $l#6c
 TRACE gdbstub::protocol::recv_packet     > <-- $vFile:setfs:0#bf
 TRACE gdbstub::protocol::response_writer > --> $F0#76
 TRACE gdbstub::protocol::recv_packet     > <-- $vFile:open:6a7573742070726f62696e67,0,1c0#ed
 TRACE gdbstub::protocol::response_writer > --> $F-1,02#32
 TRACE gdbstub::protocol::recv_packet     > <-- $vFile:setfs:1#c0
 TRACE gdbstub::protocol::response_writer > --> $F0#76
 TRACE gdbstub::protocol::recv_packet     > <-- $vFile:open:2f746573742e656c66,0,0#1e
 TRACE gdbstub::protocol::response_writer > --> $F00#a6
 TRACE gdbstub::protocol::recv_packet     > <-- $vFile:pread:0,1000,0#ef
UUUUxx#eabstub::protocol::response_writer > --> $F1000;ELF(UU4@4 (
 TRACE gdbstub::protocol::recv_packet     > <-- $vFile:fstat:0#bc
 TRACE gdbstub::protocol::response_writer > --> $F40;p#26
 TRACE gdbstub::protocol::recv_packet     > <-- $vFile:pread:0,1000,10540#b9
 TRACE gdbstub::protocol::response_writer > --> $F0230;UxxUUx&xw2�U@D Od_[0��f0HYop�}
�0�P
        Lf��#cc
 TRACE gdbstub::protocol::recv_packet     > <-- $vFile:pread:0,1000,34#26
 TRACE gdbstub::protocol::response_writer > --> $F1000;UUUUxx#83
 TRACE gdbstub::protocol::recv_packet     > <-- $vFile:pread:0,1000,104b2#e8
 TRACE gdbstub::protocol::response_writer > --> $F02be;.symtab.strtab.shstrtab.text.bss.debug_info.debug_abbrev.debug_aranges.debug_line.debug_str.comment.ARM.attributes.debug_frameUxxUUx&xw2�U@D Od_[0��f0HYop�}
�0�P
        Lf��#d8
 TRACE gdbstub::protocol::recv_packet     > <-- $vFile:pread:0,1000,10078#bf
 TRACE gdbstub::protocol::response_writer > --> $F06f8;s
                                                        .UUx�oUUx�ox    o�ty    o�l4UU0i        o�pint%
                                                                                                      :
                                                                                                       ;
                                                                                                        9
                                                                                                         I@▒�B:
                                                                                                               ;
                                                                                                                9
                                                                                                                 I▒
                                                                                                                   }

                                                                                                                    >
test.c                                                                                                              UUx[�
      UU        gKLgiJ
                      /%ef
                          jtest.cGNU C11 9.2.1 20191025 (release) [ARM/arm-9-branch revision 277599] -mfloat-abi=soft -marm -march=armv4t -g -O0 -std=c11mainGCC: (15:9-2019-q4-0ubuntu1) 9.2.1 20191025 (release) [ARM/arm-9-branch revision 277599]A)aeabi4T  ▒▒
                                                                                                                                                                                                                                                                  ����|
UUxB�B
B�UUxUU
�UU
   xUU▒xUUxUU+xUU7UUx<xUUIxUUWUUtest.c}a__DATA_START__end__DATA_END____BSS_END__main__TEXT_END____BSS_START____TEXT_START__.symtab.strtab.shstrtab.text.bss.debug_info.debug_abbrev.debug_aranges.debug_line.debug_str.comment.ARM.attributes.debug_frameUxxUUx&xw2�U@D Od_[0��f0HYop�}
�0�P
        Lf��#90
 TRACE gdbstub::protocol::recv_packet     > <-- $vFile:pread:0,1000,0#ef
UUUUxx#eabstub::protocol::response_writer > --> $F1000;ELF(UU4@4 (
 TRACE gdbstub::protocol::recv_packet     > <-- $vFile:fstat:0#bc
 TRACE gdbstub::protocol::response_writer > --> $F40;p#26
 TRACE gdbstub::protocol::recv_packet     > <-- $vFile:open:2f746573742e656c66,0,0#1e
 TRACE gdbstub::protocol::response_writer > --> $F00#a6
 TRACE gdbstub::protocol::recv_packet     > <-- $vFile:fstat:0#bc
 TRACE gdbstub::protocol::response_writer > --> $F40;p#26
 TRACE gdbstub::protocol::recv_packet     > <-- $vFile:pread:0,1000,10540#b9
 TRACE gdbstub::protocol::response_writer > --> $F0230;UxxUUx&xw2�U@D Od_[0��f0HYop�}
�0�P
        Lf��#cc
 TRACE gdbstub::protocol::recv_packet     > <-- $vFile:pread:0,1000,34#26
 TRACE gdbstub::protocol::response_writer > --> $F1000;UUUUxx#83
 TRACE gdbstub::protocol::recv_packet     > <-- $vFile:pread:0,1000,104b2#e8
 TRACE gdbstub::protocol::response_writer > --> $F02be;.symtab.strtab.shstrtab.text.bss.debug_info.debug_abbrev.debug_aranges.debug_line.debug_str.comment.ARM.attributes.debug_frameUxxUUx&xw2�U@D Od_[0��f0HYop�}
�0�P
        Lf��#d8
 TRACE gdbstub::protocol::recv_packet     > <-- $vFile:pread:0,1000,10078#bf
 TRACE gdbstub::protocol::response_writer > --> $F06f8;s
                                                        .UUx�oUUx�ox    o�ty    o�l4UU0i        o�pint%
                                                                                                      :
                                                                                                       ;
                                                                                                        9
                                                                                                         I@▒�B:
                                                                                                               ;
                                                                                                                9
                                                                                                                 I▒
                                                                                                                   }

                                                                                                                    >
test.c                                                                                                              UUx[�
      UU        gKLgiJ
                      /%ef
                          jtest.cGNU C11 9.2.1 20191025 (release) [ARM/arm-9-branch revision 277599] -mfloat-abi=soft -marm -march=armv4t -g -O0 -std=c11mainGCC: (15:9-2019-q4-0ubuntu1) 9.2.1 20191025 (release) [ARM/arm-9-branch revision 277599]A)aeabi4T  ▒▒
                                                                                                                                                                                                                                                                  ����|
UUxB�B
B�UUxUU
�UU
   xUU▒xUUxUU+xUU7UUx<xUUIxUUWUUtest.c}a__DATA_START__end__DATA_END____BSS_END__main__TEXT_END____BSS_START____TEXT_START__.symtab.strtab.shstrtab.text.bss.debug_info.debug_abbrev.debug_aranges.debug_line.debug_str.comment.ARM.attributes.debug_frameUxxUUx&xw2�U@D Od_[0��f0HYop�}
�0�P
        Lf��#90
 TRACE gdbstub::protocol::recv_packet     > <-- $vFile:pread:0,1000,0#ef
UUUUxx#eabstub::protocol::response_writer > --> $F1000;ELF(UU4@4 (
 TRACE gdbstub::protocol::recv_packet     > <-- $vFile:fstat:0#bc
 TRACE gdbstub::protocol::response_writer > --> $F40;p#26
 TRACE gdbstub::protocol::recv_packet     > <-- $vFile:pread:0,1000,102fc#1b
 TRACE gdbstub::protocol::response_writer > --> $F0474;UUxUU
�UU
   xUU▒xUUxUU+xUU7UUx<xUUIxUUWUUtest.c}a__DATA_START__end__DATA_END____BSS_END__main__TEXT_END____BSS_START____TEXT_START__.symtab.strtab.shstrtab.text.bss.debug_info.debug_abbrev.debug_aranges.debug_line.debug_str.comment.ARM.attributes.debug_frameUxxUUx&xw2�U@D Od_[0��f0HYop�}
�0�P
        Lf��#31
 TRACE gdbstub::protocol::recv_packet     > <-- $vFile:pread:0,1000,10078#bf
 TRACE gdbstub::protocol::response_writer > --> $F06f8;s
                                                        .UUx�oUUx�ox    o�ty    o�l4UU0i        o�pint%
                                                                                                      :
                                                                                                       ;
                                                                                                        9
                                                                                                         I@▒�B:
                                                                                                               ;
                                                                                                                9
                                                                                                                 I▒
                                                                                                                   }

                                                                                                                    >
test.c                                                                                                              UUx[�
      UU        gKLgiJ
                      /%ef
                          jtest.cGNU C11 9.2.1 20191025 (release) [ARM/arm-9-branch revision 277599] -mfloat-abi=soft -marm -march=armv4t -g -O0 -std=c11mainGCC: (15:9-2019-q4-0ubuntu1) 9.2.1 20191025 (release) [ARM/arm-9-branch revision 277599]A)aeabi4T  ▒▒
                                                                                                                                                                                                                                                                  ����|
UUxB�B
B�UUxUU
�UU
   xUU▒xUUxUU+xUU7UUx<xUUIxUUWUUtest.c}a__DATA_START__end__DATA_END____BSS_END__main__TEXT_END____BSS_START____TEXT_START__.symtab.strtab.shstrtab.text.bss.debug_info.debug_abbrev.debug_aranges.debug_line.debug_str.comment.ARM.attributes.debug_frameUxxUUx&xw2�U@D Od_[0��f0HYop�}
�0�P
        Lf��#90
 TRACE gdbstub::protocol::recv_packet     > <-- $qSymbol::#5b
 INFO  gdbstub::stub::core_impl           > Unknown command: Ok("qSymbol::")
 TRACE gdbstub::protocol::response_writer > --> $#00
 TRACE gdbstub::protocol::recv_packet     > <-- $qXfer:exec-file:read:1:0,ffb#b7
 TRACE gdbstub::protocol::response_writer > --> $m/test.elf#c1
 TRACE gdbstub::protocol::recv_packet     > <-- $qXfer:exec-file:read:1:9,ffb#c0
 TRACE gdbstub::protocol::response_writer > --> $l#6c
 TRACE gdbstub::protocol::recv_packet     > <-- $Hc-1#09
 TRACE gdbstub::protocol::response_writer > --> $OK#9a
 TRACE gdbstub::protocol::recv_packet     > <-- $qOffsets#4b
 TRACE gdbstub::protocol::response_writer > --> $Text=00;Data=00;Bss=00#94
 TRACE gdbstub::protocol::recv_packet     > <-- $g#67
 TRACE gdbstub::protocol::response_writer > --> $00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000107856341200005555xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx1000000078563412#0a
 TRACE gdbstub::protocol::recv_packet     > <-- $qXfer:auxv:read::0,ffb#d8
 TRACE gdbstub::protocol::response_writer > --> $m#bb
 TRACE gdbstub::protocol::recv_packet     > <-- $qXfer:auxv:read::8,ffb#e0
 TRACE gdbstub::protocol::response_writer > --> $l#6c
 TRACE gdbstub::protocol::recv_packet     > <-- $qfThreadInfo#bb
 TRACE gdbstub::protocol::response_writer > --> $mp01.01#cd
 TRACE gdbstub::protocol::recv_packet     > <-- $qsThreadInfo#c8
 TRACE gdbstub::protocol::response_writer > --> $l#6c
 TRACE gdbstub::protocol::recv_packet     > <-- $qXfer:memory-map:read::0,ffb#18
 TRACE gdbstub::protocol::response_writer > --> $m<?xml version="1.0"?>
<!DOCTYPE memory-map
    PUBLIC "+//IDN gnu.org//DTD GDB Memory Map V1.0//EN"
            "http://sourceware.org/gdb/gdb-memory-map.dtd">
<memory-map>
    <memory type="ram" start="0x0" length="0x100000000"/>
</memory-map>#76
 TRACE gdbstub::protocol::recv_packet     > <-- $qXfer:memory-map:read::f4,ffb#82
 TRACE gdbstub::protocol::response_writer > --> $l#6c
 TRACE gdbstub::protocol::recv_packet     > <-- $m0,1#fa
 TRACE gdbstub::protocol::response_writer > --> $00#60
 TRACE gdbstub::protocol::recv_packet     > <-- $m0,1#fa
 TRACE gdbstub::protocol::response_writer > --> $00#60
 TRACE gdbstub::protocol::recv_packet     > <-- $m0,40#2d
 TRACE gdbstub::protocol::response_writer > --> $00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000#6c
 TRACE gdbstub::protocol::recv_packet     > <-- $m0,8#01
 TRACE gdbstub::protocol::response_writer > --> $0000000000000000#86
 TRACE gdbstub::protocol::recv_packet     > <-- $m0,7#00
 TRACE gdbstub::protocol::response_writer > --> $00000000000000#84

Before/After `./example_no_std/check_size.sh` output

Before

target/release/gdbstub-nostd  :
section               size    addr
.interp                 28     792
.note.gnu.property      32     824
.note.gnu.build-id      36     856
.note.ABI-tag           32     892
.gnu.hash               36     928
.dynsym                360     968
.dynstr                204    1328
.gnu.version            30    1532
.gnu.version_r          64    1568
.rela.dyn              408    1632
.init                   23    4096
.plt                    16    4128
.plt.got                 8    4144
.text                14955    4160
.fini                    9   19116
.rodata                922   20480
.eh_frame_hdr          260   21404
.eh_frame             1340   21664
.init_array              8   28072
.fini_array              8   28080
.dynamic               448   28088
.got                   136   28536
.data                    8   28672
.bss                     8   28680
.comment                31       0
Total                19410

After

target/release/gdbstub-nostd  :
section               size    addr
.interp                 28     792
.note.gnu.property      32     824
.note.gnu.build-id      36     856
.note.ABI-tag           32     892
.gnu.hash               36     928
.dynsym                360     968
.dynstr                204    1328
.gnu.version            30    1532
.gnu.version_r          64    1568
.rela.dyn              408    1632
.init                   23    4096
.plt                    16    4128
.plt.got                 8    4144
.text                15355    4160
.fini                    9   19516
.rodata               1178   20480
.eh_frame_hdr          268   21660
.eh_frame             1396   21928
.init_array              8   28072
.fini_array              8   28080
.dynamic               448   28088
.got                   136   28536
.data                    8   28672
.bss                     8   28680
.comment                31       0
Total                20130

daniel5151

Thanks for the PR!

As you've identified, this is indeed a breaking change, which means we'll need to land it in the dev/0.7 branch for now.

Though, before we land it... I would like to get to the bottom of why check_size.sh went up by ~700 bytes. That's quite a lot for something as simple as bitflags!

Any theories?

src/stub/core_impl.rs

qwandor · 2023-04-26T16:44:53Z

One of the major changes between bitflags 1 and 2 is that it introduces the BitFlags trait to work with flags generically, rather than just implementing the same methods on each bitflags type. I wonder if that's causing some extra code or vtable to be included somehow, even if it's not really needed here?

daniel5151 · 2023-04-26T23:39:13Z

Do you have a pressing need to merge this PR in?

If so, I don't mind landing it in dev/0.7 for now, but if not, I'd like to leave it open until I have a chance to dig into this binary size regression myself.

qwandor · 2023-04-27T09:08:55Z

Thanks for the reply.

For context, I work on Android. We vendor in all the Rust crates that we use (you can see them at https://cs.android.com/android/platform/superproject/+/master:external/rust/crates/ if you're curious), and gdbstub is among them. As much as possible, we try to avoid having multiple versions of a crate in the tree, both because of the maintenance overhead and because of the impact on overall binary size.

bitflags has caused a problem, because we now have some crates updated upstream to version 2, while some are still using version 1. We can maintain downstream patches to work around this, but this is more maintenance burden so I'd like if possible to get every crate we care about using bitflags 2 upstream.

So from my point of view the sooner this change is in and released the better, but there are still a number of other crates in the same position or otherwise blocking the update so if we have to maintain local patches then we will.

daniel5151 · 2023-04-27T16:36:49Z

Thanks for the context!

As someone who often takes point manicuring the dependency tree at $work, I completely understand. I've sent out my fair share of PRs and DMs asking projects to bump syn to version 2 myself...

Unfortunately, I don't have a great estimate for when I'll be cutting 0.7 release, as there are quite a few small-to-medium sized issues that'll that need to get resolved as part of the 0.7 milestone, and I'm not sure when I'll find the time to sit down and hammer those out.

Let me know if you think it'd be helpful to land this PR in dev/0.7 (as opposed to just leaving it open for now).

daniel5151 · 2023-04-30T20:04:55Z

Okay, I had a chance to dig in here, and I've found the culprit.

The new version of bitflags! emits a FromStr impl, which includes a call to .trim()
Prior to this change, gdbstub only contained a single call to .trim() (as part of its target.xml handling)
Because there are now two calls to .trim(), LLVM no longer inlines .trim(), even though that new use of .trim() is then dead-code-eliminated!

I've tested this theory by reverting the bitflags changes, and adding a simple call to core::hint::black_box("foo").trim() into the binary, and lo and behold - there's a massive binary size jump!

Not gonna lie, this is pretty funny, and I'm glad I could figure this out haha.

Anyways, in terms of "action items" - I think the best thing to do would be to swap out the current .trim() call in gdbstub to a hand-rolled one that will get inlined. Assuming that does the trick, I'll go ahead and merge that in as an unrelated commit into dev/0.7, and then apply this PR on-top of it.

qwandor · 2023-05-03T16:15:20Z

Thanks for digging into that! It sounds rather like a compiler bug, might be worth filing it somewhere?

daniel5151 · 2023-05-09T23:16:25Z

It sounds rather like a compiler bug, might be worth filing it somewhere?

Maybe... but this might be niche enough that I'm not gonna do it myself. Activation energy and whatnot ¯\_(ツ)_/¯

In any case... I think we should leave this PR open until bitflags/bitflags#348 (comment) resolves, at which point, we can switch over to the more lightweight macro, and sidestep this weird compiler interaction entirely.

qwandor · 2023-05-10T09:18:01Z

Okay, thanks.

qwandor · 2023-05-25T11:16:20Z

I've updated to bitflags 2.3.1 and the external version of the macro.

Before/After `./example_no_std/check_size.sh` output

Before

    Analyzing target/release/gdbstub-nostd

File  .text    Size             Crate Name
2.4%  72.1% 10.4KiB         [Unknown] main
0.2%   4.9%    733B           gdbstub gdbstub::stub::state_machine::GdbStubStateMachineInner<gdbstub::stub::state_machine::state::Running,T,C>::report_stop
0.1%   2.0%    301B           gdbstub gdbstub::protocol::commands::breakpoint::BasicBreakpoint::from_slice
0.1%   1.9%    276B           gdbstub gdbstub::stub::core_impl::resume::<impl gdbstub::stub::core_impl::GdbStubImpl<T,C>>::write_stop_common
0.1%   1.8%    269B           gdbstub gdbstub::protocol::common::hex::decode_hex_buf
0.1%   1.7%    259B           gdbstub <gdbstub::protocol::common::thread_id::ThreadId as core::convert::TryFrom<&[u8]>>::try_from
0.1%   1.7%    254B           gdbstub gdbstub::protocol::response_writer::ResponseWriter<C>::write_specific_thread_id
0.0%   1.4%    212B           gdbstub gdbstub::protocol::response_writer::ResponseWriter<C>::write
0.0%   0.9%    130B           gdbstub gdbstub::protocol::response_writer::ResponseWriter<C>::inner_write
0.0%   0.9%    128B           gdbstub gdbstub::protocol::common::hex::decode_hex
0.0%   0.8%    115B           gdbstub gdbstub::protocol::common::hex::decode_hex
0.0%   0.7%    111B           gdbstub gdbstub::protocol::response_writer::ResponseWriter<C>::flush
0.0%   0.7%    106B    gdbstub_nostd? <gdbstub_nostd::gdb::DummyTarget as gdbstub::target::ext::base::multithread::MultiThreadBase>::read_addrs
0.0%   0.7%     97B           gdbstub gdbstub::protocol::response_writer::ResponseWriter<C>::write_num
0.0%   0.6%     96B           gdbstub <gdbstub::protocol::common::thread_id::IdKind as core::convert::TryFrom<&[u8]>>::try_from
0.0%   0.6%     95B           gdbstub gdbstub::protocol::response_writer::ResponseWriter<C>::write_num
0.0%   0.6%     88B           gdbstub gdbstub::protocol::response_writer::ResponseWriter<C>::write_hex
0.0%   0.5%     79B           gdbstub <usize as gdbstub::internal::be_bytes::BeBytes>::from_be_bytes
0.0%   0.5%     77B           gdbstub <u32 as gdbstub::internal::be_bytes::BeBytes>::from_be_bytes
0.0%   0.4%     65B    gdbstub_nostd? <gdbstub_nostd::gdb::DummyTarget as gdbstub::target::ext::base::multithread::MultiThreadBase>::write_addrs
0.0%   0.4%     65B    gdbstub_nostd? <gdbstub_nostd::gdb::DummyTarget as gdbstub::target::ext::base::multithread::MultiThreadBase>::write_registers
0.0%   0.4%     65B    gdbstub_nostd? <gdbstub_nostd::gdb::DummyTarget as gdbstub::target::ext::base::multithread::MultiThreadBase>::read_registers
0.0%   0.4%     64B      gdbstub_arch <gdbstub_arch::arm::reg::arm_core::ArmCoreRegs as gdbstub::arch::Registers>::gdb_deserialize
0.0%   0.4%     57B compiler_builtins __rust_probestack
0.0%   0.3%     50B    gdbstub_nostd? <gdbstub_nostd::gdb::DummyTarget as gdbstub::target::ext::base::multithread::MultiThreadResume>::set_resume_action_continue
0.0%   0.3%     50B    gdbstub_nostd? <gdbstub_nostd::gdb::DummyTarget as gdbstub::target::ext::base::multithread::MultiThreadResume>::clear_resume_actions
0.0%   0.3%     50B    gdbstub_nostd? <gdbstub_nostd::gdb::DummyTarget as gdbstub::target::ext::base::multithread::MultiThreadResume>::resume
0.0%   0.2%     34B         [Unknown] _start
0.0%   0.0%      6B    gdbstub_nostd? <gdbstub_nostd::gdb::DummyTarget as gdbstub::target::ext::breakpoints::SwBreakpoint>::add_sw_breakpoint
0.0%   0.0%      0B                   And 0 smaller methods. Use -n N to show more.
3.4% 100.0% 14.5KiB                   .text section size, the file size is 428.4KiB
target/release/gdbstub-nostd  :
section               size    addr
.interp                 28     792
.note.gnu.property      32     824
.note.gnu.build-id      36     856
.note.ABI-tag           32     892
.gnu.hash               36     928
.dynsym                360     968
.dynstr                204    1328
.gnu.version            30    1532
.gnu.version_r          64    1568
.rela.dyn              408    1632
.init                   23    4096
.plt                    16    4128
.plt.got                 8    4144
.text                14820    4160
.fini                    9   18980
.rodata                922   20480
.eh_frame_hdr          260   21404
.eh_frame             1340   21664
.init_array              8   28072
.fini_array              8   28080
.dynamic               448   28088
.got                   136   28536
.data                    8   28672
.bss                     8   28680
.comment                31       0
Total                19275

After

    Analyzing target/release/gdbstub-nostd

File  .text    Size             Crate Name
2.4%  72.1% 10.4KiB         [Unknown] main
0.2%   4.9%    733B           gdbstub gdbstub::stub::state_machine::GdbStubStateMachineInner<gdbstub::stub::state_machine::state::Running,T,C>::report_stop
0.1%   2.0%    301B           gdbstub gdbstub::protocol::commands::breakpoint::BasicBreakpoint::from_slice
0.1%   1.9%    276B           gdbstub gdbstub::stub::core_impl::resume::<impl gdbstub::stub::core_impl::GdbStubImpl<T,C>>::write_stop_common
0.1%   1.8%    269B           gdbstub gdbstub::protocol::common::hex::decode_hex_buf
0.1%   1.7%    259B           gdbstub <gdbstub::protocol::common::thread_id::ThreadId as core::convert::TryFrom<&[u8]>>::try_from
0.1%   1.7%    254B           gdbstub gdbstub::protocol::response_writer::ResponseWriter<C>::write_specific_thread_id
0.0%   1.4%    212B           gdbstub gdbstub::protocol::response_writer::ResponseWriter<C>::write
0.0%   0.9%    130B           gdbstub gdbstub::protocol::response_writer::ResponseWriter<C>::inner_write
0.0%   0.9%    128B           gdbstub gdbstub::protocol::common::hex::decode_hex
0.0%   0.8%    115B           gdbstub gdbstub::protocol::common::hex::decode_hex
0.0%   0.7%    111B           gdbstub gdbstub::protocol::response_writer::ResponseWriter<C>::flush
0.0%   0.7%    106B    gdbstub_nostd? <gdbstub_nostd::gdb::DummyTarget as gdbstub::target::ext::base::multithread::MultiThreadBase>::read_addrs
0.0%   0.7%     97B           gdbstub gdbstub::protocol::response_writer::ResponseWriter<C>::write_num
0.0%   0.6%     96B           gdbstub <gdbstub::protocol::common::thread_id::IdKind as core::convert::TryFrom<&[u8]>>::try_from
0.0%   0.6%     95B           gdbstub gdbstub::protocol::response_writer::ResponseWriter<C>::write_num
0.0%   0.6%     88B           gdbstub gdbstub::protocol::response_writer::ResponseWriter<C>::write_hex
0.0%   0.5%     79B           gdbstub <usize as gdbstub::internal::be_bytes::BeBytes>::from_be_bytes
0.0%   0.5%     77B           gdbstub <u32 as gdbstub::internal::be_bytes::BeBytes>::from_be_bytes
0.0%   0.4%     65B    gdbstub_nostd? <gdbstub_nostd::gdb::DummyTarget as gdbstub::target::ext::base::multithread::MultiThreadBase>::write_addrs
0.0%   0.4%     65B    gdbstub_nostd? <gdbstub_nostd::gdb::DummyTarget as gdbstub::target::ext::base::multithread::MultiThreadBase>::write_registers
0.0%   0.4%     65B    gdbstub_nostd? <gdbstub_nostd::gdb::DummyTarget as gdbstub::target::ext::base::multithread::MultiThreadBase>::read_registers
0.0%   0.4%     64B      gdbstub_arch <gdbstub_arch::arm::reg::arm_core::ArmCoreRegs as gdbstub::arch::Registers>::gdb_deserialize
0.0%   0.4%     57B compiler_builtins __rust_probestack
0.0%   0.3%     50B    gdbstub_nostd? <gdbstub_nostd::gdb::DummyTarget as gdbstub::target::ext::base::multithread::MultiThreadResume>::set_resume_action_continue
0.0%   0.3%     50B    gdbstub_nostd? <gdbstub_nostd::gdb::DummyTarget as gdbstub::target::ext::base::multithread::MultiThreadResume>::clear_resume_actions
0.0%   0.3%     50B    gdbstub_nostd? <gdbstub_nostd::gdb::DummyTarget as gdbstub::target::ext::base::multithread::MultiThreadResume>::resume
0.0%   0.2%     34B         [Unknown] _start
0.0%   0.0%      6B    gdbstub_nostd? <gdbstub_nostd::gdb::DummyTarget as gdbstub::target::ext::breakpoints::SwBreakpoint>::add_sw_breakpoint
0.0%   0.0%      0B                   And 0 smaller methods. Use -n N to show more.
3.4% 100.0% 14.5KiB                   .text section size, the file size is 429.9KiB
target/release/gdbstub-nostd  :
section               size    addr
.interp                 28     792
.note.gnu.property      32     824
.note.gnu.build-id      36     856
.note.ABI-tag           32     892
.gnu.hash               36     928
.dynsym                360     968
.dynstr                204    1328
.gnu.version            30    1532
.gnu.version_r          64    1568
.rela.dyn              408    1632
.init                   23    4096
.plt                    16    4128
.plt.got                 8    4144
.text                14820    4160
.fini                    9   18980
.rodata                922   20480
.eh_frame_hdr          260   21404
.eh_frame             1340   21664
.init_array              8   28072
.fini_array              8   28080
.dynamic               448   28088
.got                   136   28536
.data                    8   28672
.bss                     8   28680
.comment                31       0
Total                19275

daniel5151 · 2023-05-25T23:54:04Z

Sweet, that's awesome.
I'll go ahead and merge this into dev/0.7 then.

Thanks again!

daniel5151 · 2023-11-24T20:02:15Z

FYI, gdbstub 0.7 has just been released. Cheers!

qwandor · 2023-11-24T20:09:18Z

Great, thanks!

daniel5151 reviewed Apr 26, 2023

View reviewed changes

src/stub/core_impl.rs Outdated Show resolved Hide resolved

qwandor changed the base branch from master to dev/0.7 April 26, 2023 16:40

daniel5151 force-pushed the dev/0.7 branch from 04909b7 to 2cf3299 Compare April 30, 2023 20:09

daniel5151 mentioned this pull request Apr 30, 2023

Allow opting-out of InternalBitFlags fmt/FromStr impls? bitflags/bitflags#347

Closed

qwandor added 4 commits May 25, 2023 11:40

Updated to bitflags 2.2.1.

15c256f

Don't derive so many traits for ProtocolFeatures.

dfb275c

Update to bitflags 2.3.1.

0dc7ef5

Use external version of bitflags macro.

9173b82

daniel5151 merged commit d622369 into daniel5151:dev/0.7 May 25, 2023
2 checks passed

daniel5151 mentioned this pull request Nov 21, 2023

Update bitflags to 2.4 #141

Closed

2 tasks

qwandor deleted the bitflags branch November 22, 2023 16:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updated to bitflags 2.2.1. #138

Updated to bitflags 2.2.1. #138

qwandor commented Apr 26, 2023 •

edited

daniel5151 left a comment

qwandor commented Apr 26, 2023

daniel5151 commented Apr 26, 2023

qwandor commented Apr 27, 2023

daniel5151 commented Apr 27, 2023

daniel5151 commented Apr 30, 2023

qwandor commented May 3, 2023

daniel5151 commented May 9, 2023

qwandor commented May 10, 2023

qwandor commented May 25, 2023

Before

After

daniel5151 commented May 25, 2023

daniel5151 commented Nov 24, 2023

qwandor commented Nov 24, 2023

Updated to bitflags 2.2.1. #138

Updated to bitflags 2.2.1. #138

Conversation

qwandor commented Apr 26, 2023 • edited

Description

API Stability

Checklist

Validation

Before

After

daniel5151 left a comment

Choose a reason for hiding this comment

qwandor commented Apr 26, 2023

daniel5151 commented Apr 26, 2023

qwandor commented Apr 27, 2023

daniel5151 commented Apr 27, 2023

daniel5151 commented Apr 30, 2023

qwandor commented May 3, 2023

daniel5151 commented May 9, 2023

qwandor commented May 10, 2023

qwandor commented May 25, 2023

Before

After

daniel5151 commented May 25, 2023

daniel5151 commented Nov 24, 2023

qwandor commented Nov 24, 2023

qwandor commented Apr 26, 2023 •

edited