Re: BUG: erroneous read access to volatile mem ref

[Graham Stott <grahams at cygnus dot co dot uk>]

Ralf Gütlein wrote:
> 
> As mentioned earlier this week in gcc@gcc.gnu.org and gnuh8
> mailing lists, there is a bug in code generation for a
> very special case involving volatile mem refs and inline.
> 
> Consider the following real-life example:
> 
> ////// start source
> 
> /* TCW is a fixed-address register that must be set
>    to "0" from time to time; otherwise a watchog reset
>    would be generated. A read access to this register
>    leads to unpredictable results ("write-only")       */
> 
> extern volatile unsigned char TCW;
> 
> inline void Watchdog(void)
> {
>   TCW = 0;
> }
> 
> void test1(void)
> {
>   TCW = 0;     /* this leads to correct code */
> }
> 
> void test2(void)
> {
>   Watchdog();  /* ... but not this one!      */
> }
> ////// end source
> 
> The code generated for h8300 is:
> 
> (...)
> 
> _test1__Fv:
>   ; TCW = 0;
>  sub.b r2l,r2l
>  mov.b r2l,@_TCW  ; << correct!
>  rts
> 
> (...)
> 
> _test2__Fv:
>   ; Watchdog();
>  sub.b r2l,r2l
>  mov.b r2l,@_TCW  ; << correct!
>  mov.b @_TCW,r3l  ; << erroreous read!
>  rts
> 
> You would assume the outcome of both test cases to be identical.
> Although in most cases this unneccessary read is not critical
> (and therefore apparently has escaped everybody's attention), in
> rare cases (as the one above) this is fatal.
> 
> This bug is only present in cc1plus.exe, but not in cc1.exe.
> 
No the bug is in both because it's an interaction between GCC's
statement expression extension ({...}) and volatile variables.

Here's a C testcase
-----------------------------------------------------
extern volatile unsigned char TCW;

#define  WD()   \ 
({              \ 
  TCW = 0;      \
  1;            \
})

int test(void)
{
   return WD();
}
------------------------------------------------------

> After all I could ascertain, this bug is not target dependent.
> At least for i586-cygwin I could reproduce it.
> 
Yes it's target independent.

> A quick investigation on earlier versions turned out that
> this issue has been present in egcs-20000103 snapshot already,
> but not in versions before december 99. As I read somewhere,
> a new inliner has been introduced around december; maybe this
> new module is to blame?
> 
No the new inliner isn't to blame it just exposes the problem in
the backend.

> Regards,
> Ralf
> 
>            .....
>            ô ô )
> -----oOOo--(_)---oOOo------
> 
> Ralf Guetlein
> Biotest Medizintechnik GmbH
> Industriestrasse 19
> D-63755 Alzenau
> Germany
> ---------------------------
> Tel. +49 6023 9487-42
> Fax. +49 6023 9487-33
> ralf.guetlein@biotest-mt.de
> ---------------------------

Graham