Menu ▾ ▴

#232 semihosting_fileio read return wrong number of read bytes

0.9.0
new
nobody
None
2022-01-09
2019-04-21
hogthrob
No

The current implementation of read for semihosting_fileio read works only for reads returning exactly the requested amount OR in case of EOF/ERROR.

In the case of "not full" read with respect to requested bytes, the number of requested bytes - returned bytes was returned (i.e. how many bytes have not been read), which should have been the number of read bytes.

Please integrate the following fix:

https://github.com/db4ple/openocd/commit/0a87bac689a271c8e6fb97a48ee23e4d019a4438

Discussion

  • Liviu Ionescu (ilg)

    Liviu Ionescu (ilg) - 2019-04-21

    I confirm that for partial reads the SEMIHOSTING_SYS_READ call should return the number of remaining bytes, and the regular (not the fileio) case does this.

    However I do not have experience with the fileio use case. Your patch seems ok, but I'd like someone more experienced to confirm this.

    Perhaps you can also explain in a few words how this works. Is the call forwarded to the gdb client? What is this call returning for partial reads?

    Anyway, if the maintainers aggree on this patch, I suggest you submmit it via Gerrit.

     

  • hogthrob

    hogthrob - 2019-04-21

    Sure. In order to understand this patch, you have to read the comments given on first half of the SYS_READ in the same file somewhere above explaining the GDB protocol return value for SYS_READ

    The GDB remote semihosting protocol implements SYS_READ but returns the (requested bytes - actually read bytes) in its result value. The patched function translates this return value into values which would be returned from the "normal" read call (the number of bytes actually read).

    I have no idea how to use Gerrit, is there some kind of instructions available. BTW, I tried to fork the OpenOCD repository on SF and many hours later I got an email from SF telling me that this operation failed. If a fork is necessary to submit something to Gerrit, I am out of business...

     

    • Paul Fertser

      Paul Fertser - 2019-04-21

      Hello,

      Thank you for the report and the patch.

      On Sun, Apr 21, 2019 at 08:31:05PM -0000, hogthrob wrote:

      I have no idea how to use Gerrit, is there some kind of instructions available.

      There certainly is, the file is called HACKING in the root of the
      OpenOCD sources.

      Fortunately, it doesn't depend on any SF functionality.

      --
      Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software!
      mailto:fercerpav@gmail.com

       

  • Liviu Ionescu (ilg)

    Liviu Ionescu (ilg) - 2019-04-21

    If you mean the comments in semihosting_common.c, the SEMIHOSTING_SYS_READ case, they do not refer to the gdb client protocol, but to the arm semihosting protol (I contributed that file, and the large comment blocks were copied from the arm semihosting pdf file).

    As for the SF behaviour, no comments, I moved everything to GitHub several years ago.

    As for using Gerrit to contribute patches to OpenOCD,, there are some explanations somewhere on the OpenOCD site, it is basically a git thing, you push commits to some custom branches and they are validated and later merged into the master branch. If you never did Gerrit before, it might be an useful experience, possibly frustrating initially, but generally ok.

     

  • hogthrob

    hogthrob - 2019-04-21

    Well, we both are right, sort of (you more, I less, the fix: absolutely):

    https://github.com/db4ple/openocd/blob/0a87bac689a271c8e6fb97a48ee23e4d019a4438/src/target/semihosting_common.c#L789

    You are right, it is not directly refering to the GDB protocol.

    In fact my assumptions were wrong, the GDB protocol DOES return the number of written bytes (https://sourceware.org/gdb/onlinedocs/gdb/Host-I_002fO-Packets.html#Host-I_002fO-Packets).

    And the semihosting protocol wants the unread bytes.

    The good thing is, my patch converts one in the other, so it still works as intended, although I misunderstood why.

     

  • hogthrob

    hogthrob - 2019-04-21

    I will not use Gerrit, as it exposes more personal information than I want it to. It not even allows me to delete my account, should have checked this before.

    Sorry, but take my patch or leave it. It is just two lines.

     

  • Liviu Ionescu (ilg)

    Liviu Ionescu (ilg) - 2019-04-21

    That's for the project maintainers to decide, I'm not one of them.

    From my point of view, the patch seems ok.

    However I never used OpenOCD semihosting via the GDB client and I don't know how to test it.

     

    Last edit: Liviu Ionescu (ilg) 2019-04-21

    • hogthrob

      hogthrob - 2019-04-22

      BTW, how to test semihosting via GDB is simple:

      Assuming you have a STM32 semihosted application (in .elf) and openocd is running:

      arm-none-eabi-gdb <elf>
      ...
      target remote :3333
      monitor arm semihosting enable
      monitor arm semihosting_fileio enable
      load
      continue
      ...</elf>

      Now your file IO is done through gdb. It really only makes sense if you forward the local OpenOCD tcp port via SSH (or some other means) to another machine on which the GDB runs (and where you want/must have the files), otherwise the GDB fileio and the OpenOCD fileio make not much of a difference.

       

      • Liviu Ionescu (ilg)

        Liviu Ionescu (ilg) - 2019-04-22

        Thank you for the explanations. Indeed, this makes sense only when the gdb client and server are running on different machines.

         

  • hogthrob

    hogthrob - 2019-04-21

    Hi Liviu,
    I know. Thanks for the discussion in any case.

     

  • Pavel Kirienko

    Pavel Kirienko - 2022-01-04

    @hogthrob would you object if I submitted your patch via Gerrit? I also stumbled upon this issue.

     

    • hogthrob

      hogthrob - 2022-01-05

      Not at all, please go ahead!

       

Log in to post a comment.