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

libutee: provide an implementation of putchar() #1754

Merged
merged 1 commit into from
Aug 21, 2017

Conversation

jforissier
Copy link
Contributor

@jforissier jforissier commented Aug 17, 2017

Calling printf() from a TA to print a single character results in a
linker error:

  39  TEE_Result TA_CreateEntryPoint(void)
  40  {
  41          printf(".");
  42          /* ... */
  43  }

  hello_world_ta.o: In function `TA_CreateEntryPoint':
  hello_world_ta.c:41: undefined reference to `putchar'

In this case, the compiler has optimized the printf() call into a call
to putchar(), assuming that we have a C library and that it complies to
the relevant standards (so that printf() and putchar() may be used
interchangeably).

One way to fix the issue is to prevent such optimizations by using
-fno-builtin or -ffreestanding, at the cost of slightly larger code
size and possibly reduced performance.

Another option is to simply provide the missing putchar() function. It
is the purpose of this commit.

Reported-by: Zeng Tao <[email protected]>
Signed-off-by: Jerome Forissier <[email protected]>

@jforissier
Copy link
Contributor Author

Replaces #1723.

If we go this route (allowing builtin functions), we'll need to fix our printf() because it is inconsistent with our puts(). I can do that in a separate PR.

@jenswi-linaro
Copy link
Contributor

Reviewed-by: Jens Wiklander <[email protected]>

Copy link
Contributor

@etienne-lms etienne-lms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some minor stuff about sign propagation in putc() return value.


if (trace_get_level() >= TRACE_PRINTF_LEVEL)
trace_ext_puts(str);
return c;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: return (int)(unsigned char)c; ?

@@ -108,4 +117,9 @@ int puts(const char *str __unused)
return 0;
}

int putchar(int c __unused)
{
return c;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: remove __unused above.
minor: return (int)(unsigned char)c; (char is expected to be cast as an unsigned char)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right.

@jforissier
Copy link
Contributor Author

Updated.

@etienne-lms
Copy link
Contributor

Reviewed-by: Etienne Carriere <[email protected]>

Calling printf() from a TA to print a single character results in a
linker error:

  39  TEE_Result TA_CreateEntryPoint(void)
  40  {
  41          printf(".");
  42          /* ... */
  43  }

  hello_world_ta.o: In function `TA_CreateEntryPoint':
  hello_world_ta.c:41: undefined reference to `putchar'

In this case, the compiler has optimized the printf() call into a call
to putchar(), assuming that we have a C library and that it complies to
the relevant standards (so that printf() and putchar() may be used
interchangeably).

One way to fix the issue is to prevent such optimizations by using
-fno-builtin or -ffreestanding, at the cost of slightly larger code
size and possibly reduced performance.

Another option is to simply provide the missing putchar() function. It
is the purpose of this commit.

Reported-by: Zeng Tao <[email protected]>
Signed-off-by: Jerome Forissier <[email protected]>
Reviewed-by: Jens Wiklander <[email protected]>
Reviewed-by: Etienne Carriere <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants