diff mbox

ARM: semi-hosting support for stderr

Message ID 4C24CECA.1040308@st.com
State New
Headers show

Commit Message

Christophe Lyon June 25, 2010, 3:44 p.m. UTC
Hello,

I propose this small patch so that ARM semi-hosting handles stderr as 
expected when linking with Newlib/libgloss.



Christophe

Comments

Christophe Lyon Aug. 19, 2010, 3:11 p.m. UTC | #1
Ping?

On 25.06.2010 17:44, Christophe LYON wrote:
> Hello,
>
> I propose this small patch so that ARM semi-hosting handles stderr as
> expected when linking with Newlib/libgloss.
>
> diff --git a/arm-semi.c b/arm-semi.c
> index 9549e6c..6874036 100644
> --- a/arm-semi.c
> +++ b/arm-semi.c
> @@ -211,8 +211,11 @@ uint32_t do_arm_semihosting(CPUState *env)
>            if (strcmp(s, ":tt") == 0) {
>                if (ARG(1)<  4)
>                    return STDIN_FILENO;
> -            else
> +            else if (ARG(1) == 4)
>                    return STDOUT_FILENO;
> +           else
> +             return STDERR_FILENO; /* See newlib/libgloss
> +                                      implementation.  */
>            }
>            if (use_gdb_syscalls()) {
>                gdb_do_syscall(arm_semi_cb, "open,%s,%x,1a4", ARG(0),
>
>
> Christophe
>
>
Peter Maydell Nov. 14, 2010, 5:50 p.m. UTC | #2
On 19 August 2010 16:11, Christophe Lyon <christophe.lyon@st.com> wrote:
> On 25.06.2010 17:44, Christophe LYON wrote:
>> I propose this small patch so that ARM semi-hosting handles stderr as
>> expected when linking with Newlib/libgloss.

Apologies for this rather belated reply, but I only recently managed
to confirm the relevant information...

Unfortunately there is no means in the semihosting ABI to distinguish
app stdout from stderr. Effectively semihosting defines a single input
channel and a single output channel, not three stdin/stdout/stderr
channels. All the implementations of semihosting that I am aware
of work this way, so it's not just an omission from the semihosting
documentation.

Although newlib/libgloss use different modes for opening stdout
and stderr, the ARM C library implementation does not, for
instance; so your patch is relying on a detail of implementation of
a particular semihosting user. So I don't think we can apply this
patch, because it could break other (nonlibgloss) users of
semihosting.

-- PMM
Christophe Lyon Nov. 15, 2010, 12:27 p.m. UTC | #3
On 14.11.2010 18:50, Peter Maydell wrote:
> Although newlib/libgloss use different modes for opening stdout
> and stderr, the ARM C library implementation does not, for
> instance; so your patch is relying on a detail of implementation of
> a particular semihosting user. So I don't think we can apply this
> patch, because it could break other (nonlibgloss) users of
> semihosting.
> 

Thanks for your answer.

I think we could argue it's a bug in the ARM semihosting spec, so maybe
we could request them to change this. However, I am unsure how to do
this efficiently, as I recently tried with little success to have them
change the "angel_SWIreason_ReportException" such that at least
ADP_Stopped_AplicationExit can have the program return code as an extra
parameter... (anyway I have another small Qemu patch to handle this
situation by returning the contents of r0 when the program entered
exit(). I can submit it if someone is willing to review it).

Christophe.
diff mbox

Patch

diff --git a/arm-semi.c b/arm-semi.c
index 9549e6c..6874036 100644
--- a/arm-semi.c
+++ b/arm-semi.c
@@ -211,8 +211,11 @@  uint32_t do_arm_semihosting(CPUState *env)
          if (strcmp(s, ":tt") == 0) {
              if (ARG(1) < 4)
                  return STDIN_FILENO;
-            else
+            else if (ARG(1) == 4)
                  return STDOUT_FILENO;
+           else
+             return STDERR_FILENO; /* See newlib/libgloss
+                                      implementation.  */
          }
          if (use_gdb_syscalls()) {
              gdb_do_syscall(arm_semi_cb, "open,%s,%x,1a4", ARG(0),