Resolve two problems that occurred when working with semihosting service
through multiple connection cycles (connect-disconnect-reconnect):
1) Double free:
When the same service handles multiple connections sequentially,
the same memory gets freed repeatedly, because function
'semihosting_service_connection_closed_handler()' incorrectly frees
service->priv->name on every connection closure.
2) Memory leak:
Function 'free_services()' misses service->priv->name cleanup for
semihosting redirection. Memory remains allocated after service
destruction.
The solution introduces a new 'dtor()' field in the service structure
that is called exactly once during free_service() execution.
To reproduce the issue, you can do the following:
1. openocd -f target.cfg -c init -c 'arm semihosting enable' -c
'arm semihosting_redirect tcp 4445'
# in another terminal
2. nc localhost 4445
3. Ctr+C
4. nc localhost 4445
5. Ctr+C
Change-Id: I0dc8021cc3e21c5af619c71a1821a1afe9bffe78
Signed-off-by: Kulyatskaya Alexandra <a.kulyatskaya@syntacore.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/9196
Tested-by: jenkins
Reviewed-by: Evgeniy Naydanov <eugnay@gmail.com>
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
Some link if not anymore accessible.
Replace them with current one and add a backup in case one gets
not accessible anymore.
Change-Id: Iffca714555e94e5322a5daac1ea756e36bbd3a8f
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/9188
Tested-by: jenkins
The coding style requires the 'case' to be at the same indentation
level of its 'switch' statement.
Align the code accordingly.
While there:
- add space around the operators;
- drop useless empty line.
Skip all riscv code, as it is going to be updated soon from the
external fork.
No changes are reported by
git log -p -w --ignore-blank-lines --patience
Change-Id: I2691dfdd2b6734143e14160b46183623e9773539
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/9051
Tested-by: jenkins
To prepare for aligning switch and case statements, fix in advance
some checkpatch error due to existing code:
- remove useless parenthesis;
- remove useless 'break';
- join spit lines;
- add space around operators;
- remove 'else' after exit() and return.
Change-Id: I8a87a0ea104205d087dcb8cbf4c67ff13a47742f
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/9050
Tested-by: jenkins
Reviewed-by: zapb <dev@zapb.de>
The functions:
- semihosting_opcode_to_str();
- semihosting_write_fields();
- semihosting_set_field();
are not referenced outside the file.
Make them static.
Change-Id: Ia8d35554673145fdfe0e501543eb18919863039f
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/8551
Tested-by: jenkins
Do not free the service in 'connection_closed_handler' because it is
free'd by the server infrastructure.
Checkpatch-ignore: COMMIT_LOG_LONG_LINE
This error was detected with valgrind:
==272468== Invalid free() / delete / delete[] / realloc()
==272468== at 0x484B27F: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==272468== by 0x1F34C7: remove_service (server.c:374)
==272468== by 0x2ED3D5: semihosting_tcp_close_cnx (semihosting_common.c:1819)
==272468== by 0x2ED3D5: handle_common_semihosting_redirect_command (semihosting_common.c:1926)
==272468== by 0x1FC703: exec_command (command.c:520)
==272468== by 0x1FC703: jim_command_dispatch (command.c:931)
==272468== by 0x36980F: JimInvokeCommand (in /home/marc/openocd/build/src/openocd)
==272468== by 0x1FFFFFFFFF: ???
==272468== by 0x53ED09F: ???
==272468== by 0x300000001: ???
==272468== by 0x1FFEFFF7FF: ???
==272468== by 0x3D3984: ??? (in /home/marc/openocd/build/src/openocd)
==272468== by 0x2: ???
==272468== Address 0x5fff650 is 0 bytes inside a block of size 24 free'd
==272468== at 0x484B27F: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==272468== by 0x2ECA42: semihosting_service_connection_closed_handler (semihosting_common.c:1807)
==272468== by 0x1F2E39: remove_connection.isra.0 (server.c:164)
==272468== by 0x1F349E: remove_connections (server.c:350)
==272468== by 0x1F349E: remove_service (server.c:364)
==272468== by 0x2ED3D5: semihosting_tcp_close_cnx (semihosting_common.c:1819)
==272468== by 0x2ED3D5: handle_common_semihosting_redirect_command (semihosting_common.c:1926)
==272468== by 0x1FC703: exec_command (command.c:520)
==272468== by 0x1FC703: jim_command_dispatch (command.c:931)
==272468== by 0x36980F: JimInvokeCommand (in /home/marc/openocd/build/src/openocd)
==272468== by 0x1FFFFFFFFF: ???
==272468== by 0x53ED09F: ???
==272468== by 0x300000001: ???
==272468== by 0x1FFEFFF7FF: ???
==272468== by 0x3D3984: ??? (in /home/marc/openocd/build/src/openocd)
==272468== Block was alloc'd at
==272468== at 0x484DA83: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==272468== by 0x2ED326: handle_common_semihosting_redirect_command (semihosting_common.c:1931)
==272468== by 0x1FC703: exec_command (command.c:520)
==272468== by 0x1FC703: jim_command_dispatch (command.c:931)
==272468== by 0x36980F: JimInvokeCommand (in /home/marc/openocd/build/src/openocd)
==272468== by 0x1FFFFFFFFF: ???
==272468== by 0x53ED09F: ???
==272468== by 0x400000002: ???
==272468== by 0x1FFEFFF7FF: ???
==272468== by 0x3D3984: ??? (in /home/marc/openocd/build/src/openocd)
==272468== by 0x2: ???
==272468==
Change-Id: I3e5323f145a98d1ff9ea7d03f87ed96140f49a18
Signed-off-by: Marc Schink <dev@zapb.de>
Reviewed-on: https://review.openocd.org/c/openocd/+/8257
Tested-by: jenkins
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
This patch fixes the handling of errno by setting the sys_errn
only if error has actually occurred during the semihosting call.
It also fixes few issues where error was not set in the first place.
Change-Id: I2fbe562f3ec5e6220b800de04cd33aa1f409c7a0
Signed-off-by: Marek Vrbka <marek.vrbka@codasip.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/7730
Tested-by: jenkins
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-by: Jan Matyas <jan.matyas@codasip.com>
This patch introduces function semihosting_opcode_to_str() which
converts semihosting opcodes to strings. This function is then
used in debug messages to improve log analysis and troubleshooting.
Change-Id: Iffea49dae13d6a626ae0db40d379cba3c9ea5bd3
Signed-off-by: Marek Vrbka <marek.vrbka@codasip.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/7726
Tested-by: jenkins
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-by: Jan Matyas <jan.matyas@codasip.com>
On Windows, isatty() can return any non-zero value if it's an interactive
device. Which diverges from the ARM semihosting specification. This patch
introduces a fix to make the SYS_ISTTY operation conform to spec.
Change-Id: I9bc4f3cb82370812825d52419851910b3e3f35cc
Signed-off-by: Marek Vrbka <marek.vrbka@codasip.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/7725
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
Tested-by: jenkins
Reviewed-by: Jan Matyas <jan.matyas@codasip.com>
The internal variable 'gdb_actual_connections' is used by log and
by semihosting to determine if there are active GDB connections.
Keep the variable local in server's code and only export its value
through a dedicated function.
This solves the issue detected by 'parse' of the variable defined
as global but not declared in any include file.
Change-Id: I6e14f4cb1097787404094636f8a2a291340222dd
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/7673
Tested-by: jenkins
With the old checkpatch we cannot use the correct format for the
SPDX tags in the file .c, in fact the C99 comments are not allowed
and we had to use the block comment.
With the new checkpatch, let's switch to the correct SPDX format.
Change created automatically through the command:
sed -i \
's,^/\* *\(SPDX-License-Identifier: .*[^ ]\) *\*/$,// \1,' \
$(find src/ contrib/ -name \*.c)
Change-Id: I6da16506baa7af718947562505dd49606d124171
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/7153
Tested-by: jenkins
Without the help of checkpatch, some CamelCase symbol passed
through the filter of maintainer's review.
Drop them.
Change-Id: If5fb07b2ffb89e853dd2a61f20d4134aa6e20d24
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Fixes: 48f267d4ad ("flash/stm32l4x: avoid using magic numbers for device ids")
Fixes: 5ab74bde06 ("semihosting: User defined operation, Tcl command exec on host")
Reviewed-on: https://review.openocd.org/c/openocd/+/7154
Tested-by: jenkins
Replace the FSF boilerplate with the SPDX tag.
The SPDX tag on files *.c is incorrect, as it should use the C99
single line comment using '//'. But current checkpatch doesn't
allow C99 comments, so keep using standard C comments, by now.
Change-Id: I255ad17235ff1e01bf0aa4deed4d944e1d693ddb
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/7071
Tested-by: jenkins
ARM/RISC-V semihosting calls SYS_READ/SYS_WRITE require
inversion of the result value as described in
"Semihosting for AArch32 and AArch64". Prior to
this patch, this was done correctly only if
(semihosting->is_fileio==false).
This patch has been tested with STM32F446.
Change-Id: I1b34c8d8393f7dfa66ee6539904a2eaf8f9154b0
Signed-off-by: Pavel Kirienko <pavel.kirienko@gmail.com>
Fixes: https://sourceforge.net/p/openocd/tickets/232/
Reviewed-on: https://review.openocd.org/c/openocd/+/6803
Tested-by: jenkins
Reviewed-by: Tim Newsome <tim@sifive.com>
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
Commit b9526f1401 ("semihosting: permit redirection of
semihosting I/O to TCP") introduces a new comparison with NULL.
Remove it.
Change-Id: Ice4333c50d16f7592f0ff86b1640217fa42e34f6
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Fixes: b9526f1401 ("semihosting: permit redirection of semihosting I/O to TCP")
Reviewed-on: https://review.openocd.org/c/openocd/+/7031
Tested-by: jenkins
Custom user syscalls can be handled with target events in the TCL scripts.
This patch gives another opportunity to handle custom syscalls in the c files.
Besides that some utility functions are also exported for the custom handlers.
Signed-off-by: Erhan Kurubas <erhan.kurubas@espressif.com>
Change-Id: Ice13d527540a0de0b2a8abda912ae3dcff3834b7
Reviewed-on: https://review.openocd.org/c/openocd/+/6889
Tested-by: jenkins
Reviewed-by: Ian Thompson <ianst@cadence.com>
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
Commit dbbac5f11d ("semihosting: use open mode flags from GDB,
not from sys/stat.h") fixes the conversion of the mode flags from
ARM semihosting encoding for SEMIHOSTING_SYS_OPEN to GDB mapping
for open().
Doing this, it breaks the conversion to local host's OS mapping
for open().
Split the conversion array to one for GDB and one for local host.
The local host conversion array is taken directly from the old
code.
Change-Id: I385321ddd32c3ac5cf6da3f1ce9eff76b05dd527
Fixes: dbbac5f11d ("semihosting: use open mode flags from GDB, not from sys/stat.h")
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Reported-by: Erhan Kurubas <erhan.kurubas@espressif.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/6870
Tested-by: jenkins
This command permits the usage of a TCP port to perform debug and stdio
operations:
- debug : READC, WRITEC and WRITE0
- stdio : READ, WRITE
This will permit the separation of semihosting message from OpenOCD log,
and separate semihosting messages per core.
syntax: arm semihosting_redirect (disable | tcp <port> [debug|stdio|all])
this allows to select which operations to be performed via TCP (debug,
stdio or all (default)).
Note: for stdio operations, only I/O from/to ':tt' file descriptors are
redirected.
tested using netcat on ubuntu
Change-Id: I37053463667ba109d52429d4f98bc98d0ede298d
Signed-off-by: Tarek BOCHKATI <tarek.bouchkati@gmail.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/5562
Tested-by: jenkins
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
Enabling a portion (0x100 - 0x107) of the user defined semihosting
operation number range (0x100 - 0x1FF) to be processed with the help of
the existing target event mechanism, to implement a general-purpose Tcl
interface for the target available on the host, via semihosting
interface.
Example usage:
- The user configures a Tcl command as a callback for one of the newly
defined events (semihosting-user-cmd-0x10X) in the configuration
file.
- The target can make a semihosting call with <opnum>, passing optional
parameters for the call.
If there is no callback registered to the user defined operation number,
nothing happens.
Example usage: Configure RTT automatically with the exact, linked
control block location from target.
Signed-off-by: Zoltán Dudás <zedudi@gmail.com>
Change-Id: I10e1784b1fecd4e630d78df81cb44bf1aa2fc247
Reviewed-on: https://review.openocd.org/c/openocd/+/6748
Tested-by: jenkins
Reviewed-by: Oleksij Rempel <linux@rempel-privat.de>
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
Values defined in sys/stat.h are not guaranteed to match
the constants defined by the GDB remote protocol, which are defined in
https://sourceware.org/gdb/onlinedocs/gdb/Open-Flags.html#Open-Flags.
On my local system (Manjaro 21.2.1 x86_64), for example, O_TRUNC is
defined as 0x40, whereas GDB requires it to be 0x400,
causing all "w" file open modes to misbehave.
This patch has been tested with STM32F446.
Change-Id: Ifb2c740fd689e71d6f1a4bde1edaecd76fdca910
Signed-off-by: Pavel Kirienko <pavel.kirienko@gmail.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/6804
Tested-by: jenkins
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
We have the macro
#define COMMAND_HANDLER(name) \
static __COMMAND_HANDLER(name)
Use it!
Change-Id: I0e5385cb54197c743348f0d2ce215c93b8e396a4
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/6786
Tested-by: jenkins
Patch generated automatically through the new checkpatch with
flags "--types COMPARISON_TO_NULL --fix-inplace".
This only fixes the comparisons
if (symbol == NULL)
if (symbol != NULL)
The case of NULL on the left side of the comparison is not tested.
Some automatic fix is incorrect and has been massaged by hands:
- if (*psig == NULL)
+ if (*!psig)
changed as
+ if (!*psig)
Change-Id: If4a1e2b4e547e223532e8e3d9da89bf9cb382ce6
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-on: http://openocd.zylin.com/6351
Tested-by: jenkins
There are more than 1000 NULL comparisons to be aligned to the
coding style.
For recurrent NULL comparison it's preferable using trivial
scripts in order to minimize the review effort.
Patch generated automatically with the command:
sed -i PATTERN $(find src/ -type f)
where PATTERN is in the list:
's/(\([a-z][a-z0-9_]*\) == NULL)/(!\1)/g'
's/(\([a-z][a-z0-9_]*->[a-z][a-z0-9_]*\) == NULL)/(!\1)/g'
's/(\([a-z][a-z0-9_]*\.[a-z][a-z0-9_]*\) == NULL)/(!\1)/g'
's/(\([a-z][a-z0-9_]*\) != NULL)/(\1)/g'
's/(\([a-z][a-z0-9_]*->[a-z][a-z0-9_]*\) != NULL)/(\1)/g'
's/(\([a-z][a-z0-9_]*\.[a-z][a-z0-9_]*\) != NULL)/(\1)/g'
's/(NULL == \([a-z][a-z0-9_]*\))/(!\1)/g'
's/(NULL == \([a-z][a-z0-9_]*->[a-z][a-z0-9_]*\))/(!\1)/g'
's/(NULL == \([a-z][a-z0-9_]*\.[a-z][a-z0-9_]*\))/(!\1)/g'
's/(NULL != \([a-z][a-z0-9_]*\))/(\1)/g'
's/(NULL != \([a-z][a-z0-9_]*->[a-z][a-z0-9_]*\))/(\1)/g'
's/(NULL != \([a-z][a-z0-9_]*\.[a-z][a-z0-9_]*\))/(\1)/g'
Change-Id: Ida103e325d6d0600fb69c0b7a1557ee969db4417
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-on: http://openocd.zylin.com/6350
Tested-by: jenkins
This change fixes behavior of the SEMIHOSTING_SYS_CLOSE operation.
It ensures that OpenOCD's own stdin/stdout/stderr streams are never
closed, not even if the target requests it via semihosting.
Change-Id: Ia85af5963d1a3516284fd834f7197369a8fb268c
Signed-off-by: Jan Matyas <matyas@codasip.com>
Reviewed-on: http://openocd.zylin.com/6291
Tested-by: jenkins
Reviewed-by: Tim Newsome <tim@sifive.com>
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
Capture the value of "errno" in semihosting isatty() call,
as is done in other syscalls.
Change-Id: I41b72175635f06c000536f583e3efa30fb57379e
Signed-off-by: Jan Matyas <matyas@codasip.com>
Reviewed-on: http://openocd.zylin.com/6292
Tested-by: jenkins
Reviewed-by: Tim Newsome <tim@sifive.com>
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
Add to doxygen comment the missing parameters.
Remove from doxygen comment any non-existing parameter.
Fix the parameter names in doxygen comment to match the one in the
function prototype.
Where the parameter name in the doxygen description seems better
than the one in the code, change the code.
Escape the character '<' to prevent doxygen to interpret it as an
xml tag.
Change-Id: I22da723339ac7d7a7a64ac4c1cc4336e2416c2cc
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-on: http://openocd.zylin.com/6002
Tested-by: jenkins
the same semihosting handlers chain is declared twice:
1. in src/target/armv4_5.c
2. in src/target/riscv/riscv.c
to make it simpler we moved the declaration into
'src/target/semihosting_common.c' under semihosting_common_handlers[].
then we used this into both of armv4_5.c and riscv.c
Change-Id: If813b3fd5eb2476658f1308f741c4e805141f617
Signed-off-by: Tarek BOCHKATI <tarek.bouchkati@gmail.com>
Reviewed-on: http://openocd.zylin.com/5473
Tested-by: jenkins
Reviewed-by: Muhammad Omair Javaid <omair.javaid@linaro.org>
Reviewed-by: Tim Newsome <tim@sifive.com>
Reviewed-by: Liviu Ionescu <ilg@livius.net>
To prepare for handling TCL return values consistently, all calls
to command_print/command_print_sameline should switch to CMD as
first parameter.
Change prototype of command_print() and command_print_sameline()
to pass CMD instead of CMD_CTX.
Since the first parameter is currently not used, the change can be
done though scripts without manual coding.
This patch is created using the command:
sed -i PATTERN $(find src/ doc/ -type f)
with all the following patters:
's/\(command_print(cmd\)->ctx,/\1,/'
's/\(command_print(CMD\)_CTX,/\1,/'
's/\(command_print(struct command_\)context \*context,/\1invocation *cmd,/'
's/\(command_print_sameline(cmd\)->ctx,/\1,/'
's/\(command_print_sameline(CMD\)_CTX,/\1,/'
's/\(command_print_sameline(struct command_\)context \*context,/\1invocation *cmd,/'
This change is inspired by http://openocd.zylin.com/1815 from Paul
Fertser but is now done through scripting.
Change-Id: I3386d8f96cdc477e7a2308dd18269de3bed04385
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Signed-off-by: Paul Fertser <fercerpav@gmail.com>
Reviewed-on: http://openocd.zylin.com/5081
Tested-by: jenkins
Reviewed-by: Tomas Vanek <vanekt@fbl.cz>
This patch fixes gdb fileio support to allow gdb console to be used as stdout.
Now we can do something like
gdb <inferior file>
(gdb) tar ext :3333
(gdb) load
(gdb) monitor arm semihosting enable
(gdb) monitor arm semihosting_fileio enable
(gdb) continue
Here: Output from inferior using puts, printf etc will be routed to gdb console.
Change-Id: I9cb0dddda1de58038c84f5b035c38229828cd744
Signed-off-by: Omair Javaid <omair.javaid@linaro.org>
Reviewed-on: http://openocd.zylin.com/4538
Tested-by: jenkins
Reviewed-by: Matthias Welwarsky <matthias@welwarsky.de>
This patch implements semihosting support for AArch64. This picks
code from previously submitted AArch64 semihosting support patch
and rebases on top of reworked semihosting code. Tested in AArch64
mode on a Lemaker Hikey Board with NewLib and GDB.
Change-Id: I228a38f1de24f79e49ba99d8514d822a28c2950b
Signed-off-by: Omair Javaid <omair.javaid@linaro.org>
Reviewed-on: http://openocd.zylin.com/4537
Tested-by: jenkins
Reviewed-by: Matthias Welwarsky <matthias@welwarsky.de>
In 2016, ARM released the second edition of the semihosting specs
("Semihosting for AArch32 and AArch64"), adding support for 64-bits.
To ease the reuse of the semihosting logic for other platforms
(like RISC-V), the semihosting code was isolated from the ARM
target and updated to the latest specs.
The new code is already in use since January (in GNU MCU Eclipse
OpenOCD) and no problems were reported, neither for ARM nor for
RISC-V targets, after more than 7K downloads.
The 2 new files were formatted with uncrustify.
Change-Id: Ie84dbd86a547323bb8a5d24eab68fc7dad013d96
Signed-off-by: Liviu Ionescu <ilg@livius.net>
Reviewed-on: http://openocd.zylin.com/4518
Tested-by: jenkins
Reviewed-by: Matthias Welwarsky <matthias@welwarsky.de>
