#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)

rtems-PR#1370-mvme167-bsp-console.diff (2.2 KB) - added by strauman on Feb 17, 2009 at 5:16:09 PM.
Proposed fix

Download all attachments as: .zip

Change History (2)

Changed on Feb 17, 2009 at 5:16:09 PM by strauman

Proposed fix

comment:1 Changed on Feb 17, 2009 at 5:33:27 PM by Joel Sherrill

Resolution: fixed
Status: newclosed

Patch applied to 4.9 branch and cvs head.

Note: See TracTickets for help on using tickets.