Opened on 02/04/09 at 21:11:36
Closed on 02/17/09 at 17:33:27
#1370 closed defect (fixed)
mvme167 console driver 'BSP_output_char' fails to convert '\n' -> '\n\r'
Reported by: | strauman | Owned by: | Joel Sherrill |
---|---|---|---|
Priority: | normal | Milestone: | 4.10 |
Component: | bsps | Version: | 4.9 |
Severity: | normal | Keywords: | |
Cc: | Blocked By: | ||
Blocking: |
Description
gcc-4.3.2 optimizes storing the '\r' character to memory
away since it is only read from inline assembly code.
The attached patch uses static, global storage to
fix this problem.
A more detailed explanation was posted to the mailing list:
============================================================
Here's a new morsel:
The mvme167 BSP does something like that
void _BSP_output_char(char c)
{
char cr = '\r';
_167Bug_pollWrite(&c, 1);
/* convert '\n' -> '\n' '\r' sequence */
if ( '\n' == c )
_167Bug_pollWrite(&cr,1);
}
In 4.9 (gcc 4.3.2) this doesn't work anymore.
Here's why:
_167Bug_pollWrite contains inline assembly;
something like
void _167Bug_pollWrite(const char *buf, int len)
{
- asm volatile( <actual assembly code>
- "a"(buf), "a"(buf+len) );
}
Newer gccs are 'smart' enough to realize that
the value the 'cr' variable was initialized with
is never used and hence optimizes storing the '\r'
character into memory away (because it is NOT
smart enough to actually understand the inline
assembly code and realize that the value is needed
there).
The naive approach here would be to proceed as
suggested by the gcc manual and add 'len' bytes
of memory starting at 'buf' to the list of
input operands. HOWEVER: this does NOT necessarily
do what you want; the actual semantics are
different for each architecture and may have
side-effects which are undocumented.
For an discussion and explanation consult this thread:
http://gcc.gnu.org/ml/gcc/2008-03/threads.html#00976
In the above case I could gcc 4.3.2 get to do
what is needed (i.e., storing the '\r' character
in memory) by either
1) declaring the 'cr' variable 'volatile'
volatile const char cr = '\r';
2) adding a general 'memory barrier' but this may be
more costly than 1) or having a proper way to
tell gcc that the inline asm reads or writes
a specific memory region (again: adding a memory
input/output operand may have OTHER SIDEEFFECTS
which are not documented in the gcc manual and
differ for each architecture).
const char cr = '\r';
asm volatile("":::"memory");
3) In this particular example, our variable is actually
constant so we could make it a static variable and
hope it is actually initialized:
static const char cr = '\r';
4) Same rationale: use a static string
_167Bug_pollWrite("\r",1)
Hope this helps alerting people that inline assembly
has to be used with extreme care and is best avoided.
-- Till
Attachments (1)
Change History (2)
Changed on 02/17/09 at 17:16:09 by strauman
Attachment: | rtems-PR#1370-mvme167-bsp-console.diff added |
---|
comment:1 Changed on 02/17/09 at 17:33:27 by Joel Sherrill
Resolution: | → fixed |
---|---|
Status: | new → closed |
Patch applied to 4.9 branch and cvs head.
Proposed fix