Electronics & Programming

develissimo

Open Source electronics development and programming

  • You are not logged in.

#1 Dec. 18, 2006 02:28:32

Jan W.
Registered: 2009-11-02
Reputation: +  0  -
Profile   Send e-mail  

[Sdcc-user] Is this a bug?


SDCC : mcs51/gbz80/z80/avr/ds390/pic16/pic14/TININative/xa51/ds400/hc08
2.6.1 #4478 (Nov 23 2006) (MINGW32)
When compiling the following snippet for mcs51, both *px translates to

mov dptr,#_x
movx a,@dptr

i.e. *x, which may not be true.

Is this an error or just me being stupid?

Jan Waclawek

PS. In this short test program, I am unable to insert some code modifying
px before "if", to demonstrate that the "if" is not redundant, without
"spoiling the error", but I have a long-long program where this really
happens.

--- code snippet ---
xdata unsigned char *px;
xdata unsigned char x;

unsigned char t;


void incrementpx(void) {
px++;
}

void main(void) {
px = x;

if (px == x) {
incrementpx();
t = *px;
} else {
t = *px;
}
}




-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cashhttp://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV_______________________________________________
Sdcc-user mailing list
Sdcc-user@lists.sourceforge.nethttps://lists.sourceforge.net/lists/listinfo/sdcc-user

Offline

#2 Dec. 18, 2006 08:28:49

Raphael N.
Registered: 2009-11-02
Reputation: +  0  -
Profile   Send e-mail  

[Sdcc-user] Is this a bug?


Hi Jan,

> When compiling the following snippet for mcs51, both *px translates to
>
> mov dptr,#_x
> movx a,@dptr
>
> i.e. *x, which may not be true.
>
> Is this an error or just me being stupid?

Hmmm, the latter ;-): As px is obviously modified in an interrupt
routine, you need to declare px as volatile:

> --- code snippet ---
> xdata unsigned char *px;

volatile __xdata unsigned char *px;

should do the job, otherwise

> void main(void) {
> px = x;
>
> if (px == x) {

*is* always true.

Regards,
Raphael



-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cashhttp://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV_______________________________________________
Sdcc-user mailing list
Sdcc-user@lists.sourceforge.nethttps://lists.sourceforge.net/lists/listinfo/sdcc-user

Offline

#3 Dec. 18, 2006 08:37:50

Frieder F.
Registered: 2009-11-02
Reputation: +  0  -
Profile   Send e-mail  

[Sdcc-user] Is this a bug?


Hi Jan,

Jan Waclawek schrieb:
> Is this an error or just me being stupid?

It's a bug - please file a bug report about it.

The bug can be reproduced within the regression
test suite with the code below:


---------8<-----------------------------------------

/** bugxxxxxxx.c
*/
#include <testfwk.h>

#if defined(SDCC) && !defined(SDCC_z80)
# define XDATA __xdata
#else
# define XDATA
#endif

XDATA unsigned char *px;
XDATA unsigned char x = {0x80,0x81,0x82};

unsigned char t;

void
incrementpx(void)
{
px++;
}

void
testMe(void)
{
px = x;

if (px == x) {
incrementpx();
t = *px;
} else {
t = *px;
}
ASSERT(t == 0x81);
}


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cashhttp://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV_______________________________________________
Sdcc-user mailing list
Sdcc-user@lists.sourceforge.nethttps://lists.sourceforge.net/lists/listinfo/sdcc-user

Offline

#4 Dec. 18, 2006 10:08:37

Jan W.
Registered: 2009-11-02
Reputation: +  0  -
Profile   Send e-mail  

[Sdcc-user] Is this a bug?


It seems that we have a tie... Anybody else having a vote? :-)


Raphael,

>As px is obviously modified in an interrupt
>routine,
>

No, px is modified in incrementpx which is a standard function.
Also, I have a program where there is some code between "px=x" and the
subsequent "if" so that the "if" condition is false, but cannot reproduce this
behaviour in a simple program presentable here.


Jan Waclawek


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cashhttp://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV_______________________________________________
Sdcc-user mailing list
Sdcc-user@lists.sourceforge.nethttps://lists.sourceforge.net/lists/listinfo/sdcc-user

Offline

#5 Dec. 18, 2006 11:20:01

Raphael N.
Registered: 2009-11-02
Reputation: +  0  -
Profile   Send e-mail  

[Sdcc-user] Is this a bug?


Hi again,

> It seems that we have a tie... Anybody else having a vote? :-)

Well, me again.
I am wrong, Frieder is probably correct. I did not realize that the call
to incrementpx() occured before dereferencing... Furthermore, volatile
in the place I indicated does not help too much.
So this is a bug---which vanishes if the storage class __xdata is
removed.

Sorry for my premature comment,
Raphael



-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cashhttp://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV_______________________________________________
Sdcc-user mailing list
Sdcc-user@lists.sourceforge.nethttps://lists.sourceforge.net/lists/listinfo/sdcc-user

Offline

#6 Dec. 18, 2006 20:04:33

Jan W.
Registered: 2009-11-02
Reputation: +  0  -
Profile   Send e-mail  

[Sdcc-user] Is this a bug?


Oh yes, I almost forgot.
My quick fix (with zero overhead) for the problem was to include the
following line to any branch of the "if".
*px += 0;
I don't know why it works so I don't know if this is an ultimate quickfix
just tried and have seen in asm the result is OK.

JW



-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cashhttp://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV_______________________________________________
Sdcc-user mailing list
Sdcc-user@lists.sourceforge.nethttps://lists.sourceforge.net/lists/listinfo/sdcc-user

Offline

#7 Jan. 1, 2011 01:53:06

Peter V.
Registered: 2009-11-02
Reputation: +  0  -
Profile   Send e-mail  

[Sdcc-user] Is this a bug?


I'm not sure if this is a bug or just questionable coding, so I'll
ask here before reporting it as a bug. I'm trying to figure out why someone
elses code tha works on snaps less than 5559 but not after 5560 is broken (so
far without success :-)). While poking at that I came across this change where
the same code sometimes gets a warning and sometimes doesn't:

$ cat test4.c
#include <w7100.h>

typedef unsigned char uint8;
typedef volatile unsigned char vuint8;

#define COMMON_BASE 0x0000
#define UIPR0 ((volatile __xdata uint8 *)(COMMON_BASE + 0x002A))


// required for IINCHIP_WRITE and IINCHIP_READ port for SDCC to
// avoid compiler use of non mapped memory access while banking

static __data uint8 w_d;

uint8 IINCHIP_READ(__xdata vuint8 *addr) __reentrant __critical
{
w_d = *((__xdata vuint8 *)(addr));
return w_d;
}

main()
{
static __xdata uint8 invalid_ip;

invalid_ip = IINCHIP_READ(UIPR0+1);
return (0);
}

This compiler is snap 5559 compiled from source

pe...@pc700 ~/wiznet/nos_test
$ sdcc -v
SDCC : mcs51/gbz80/z80/avr/ds390/pic16/pic14/TININative/xa51/ds400/hc08 2.9.4 #
(Dec 31 2010) (CYGWIN)

pe...@pc700 ~/wiznet/nos_test
$ /usr/local//bin/sdcc -c test4.c

pe...@pc700 ~/wiznet/nos_test
$

Note it doesn't complain about the conversion without a cast in the line

invalid_ip = IINCHIP_READ(UIPR0+1);

where UIPR0 is in fact a pointer to an unsigned char and thus is doing pointer
arithmatic (nor am I a good enough C coder to know if it should be complaining
or not :-)).

nor does a recent 3_0_1
snapshot

pe...@pc700 ~/wiznet/nos_test
$ "/cygdrive/c/program files/SDCC3_0_1"/bin/sdcc -v
SDCC : mcs51/gbz80/z80/ds390/pic16/pic14/TININative/ds400/hc08 3.0.1 #6066 (Nov
26 2010) (MINGW32)

pe...@pc700 ~/wiznet/nos_test
$ "/cygdrive/c/program files/SDCC3_0_1"/bin/sdcc test4.c

pe...@pc700 ~/wiznet/nos_test

but snap 5560 (and I think some later ones, I've been binary searching for the
failure point through previous snaps :-)) issues a warning about the lack of
a cast.

This compiler is snap 5560 compiled from source (which also breaks the full code
for reasons as yet unknown) which does complain about the lack of a cast.

pe...@pc700 ~/wiznet/nos_test
$ /usr/local//bin/sdcc -c test4.c
test4.c:25: warning 154: converting integral to pointer without a cast
from type 'const-unsigned-char literal'
to type 'volatile-unsigned-char xdata* auto'

pe...@pc700 ~/wiznet/nos_test
$ sdcc -v
SDCC : mcs51/gbz80/z80/avr/ds390/pic16/pic14/TININative/xa51/ds400/hc08 2.9.4 #
(Dec 30 2010) (CYGWIN)

Comments on a reportable bug or operator error in the form of poor
coding practice (the original code from Wiznet that this is based on is full
of poor coding practices :-))?

Peter Van Epp


Sdcc-user mailing list
Sdcc-user@lists.sourceforge.nethttps://lists.sourceforge.net/lists/listinfo/sdcc-user

Offline

#8 Jan. 1, 2011 10:51:01

Maarten B.
Registered: 2009-11-02
Reputation: +  0  -
Profile   Send e-mail  

[Sdcc-user] Is this a bug?


Peter,

I only see a bug that was there after 5560 and has been solved again
in 3.0.1. There is no conversion from integer to pointer without
cast in this line:

invalid_ip = IINCHIP_READ(UIPR0+1);

So there should be no warning.

Maarten

> I'm not sure if this is a bug or just questionable coding, so I'll
> ask here before reporting it as a bug. I'm trying to figure out why someone
> elses code tha works on snaps less than 5559 but not after 5560 is broken (so
> far without success :-)). While poking at that I came across this change where
> the same code sometimes gets a warning and sometimes doesn't:
>
> $ cat test4.c
> #include <w7100.h>
>
> typedef unsigned char uint8;
> typedef volatile unsigned char vuint8;
>
> #define COMMON_BASE 0x0000
> #define UIPR0 ((volatile __xdata uint8 *)(COMMON_BASE + 0x002A))
>
>
> // required for IINCHIP_WRITE and IINCHIP_READ port for SDCC to
> // avoid compiler use of non mapped memory access while banking
>
> static __data uint8 w_d;
>
> uint8 IINCHIP_READ(__xdata vuint8 *addr) __reentrant __critical
> {
> w_d = *((__xdata vuint8 *)(addr));
> return w_d;
> }
>
> main()
> {
> static __xdata uint8 invalid_ip;
>
> invalid_ip = IINCHIP_READ(UIPR0+1);
> return (0);
> }
>
> This compiler is snap 5559 compiled from source
>
> pe...@pc700 ~/wiznet/nos_test
> $ sdcc -v
> SDCC : mcs51/gbz80/z80/avr/ds390/pic16/pic14/TININative/xa51/ds400/hc08 2.9.4
> #
> (Dec 31 2010) (CYGWIN)
>
> pe...@pc700 ~/wiznet/nos_test
> $ /usr/local//bin/sdcc -c test4.c
>
> pe...@pc700 ~/wiznet/nos_test
> $
>
> Note it doesn't complain about the conversion without a cast in the line
>
> invalid_ip = IINCHIP_READ(UIPR0+1);
>
> where UIPR0 is in fact a pointer to an unsigned char and thus is doing pointer
> arithmatic (nor am I a good enough C coder to know if it should be complaining
> or not :-)).
>
> nor does a recent 3_0_1
> snapshot
>
> pe...@pc700 ~/wiznet/nos_test
> $ "/cygdrive/c/program files/SDCC3_0_1"/bin/sdcc -v
> SDCC : mcs51/gbz80/z80/ds390/pic16/pic14/TININative/ds400/hc08 3.0.1 #6066
> (Nov
> 26 2010) (MINGW32)
>
> pe...@pc700 ~/wiznet/nos_test
> $ "/cygdrive/c/program files/SDCC3_0_1"/bin/sdcc test4.c
>
> pe...@pc700 ~/wiznet/nos_test
>
> but snap 5560 (and I think some later ones, I've been binary searching for the
> failure point through previous snaps :-)) issues a warning about the lack of
> a cast.
>
> This compiler is snap 5560 compiled from source (which also breaks the full
> code
> for reasons as yet unknown) which does complain about the lack of a cast.
>
> pe...@pc700 ~/wiznet/nos_test
> $ /usr/local//bin/sdcc -c test4.c
> test4.c:25: warning 154: converting integral to pointer without a cast
> from type 'const-unsigned-char literal'
> to type 'volatile-unsigned-char xdata* auto'
>
> pe...@pc700 ~/wiznet/nos_test
> $ sdcc -v
> SDCC : mcs51/gbz80/z80/avr/ds390/pic16/pic14/TININative/xa51/ds400/hc08 2.9.4
> #
> (Dec 30 2010) (CYGWIN)
>
> Comments on a reportable bug or operator error in the form of poor
> coding practice (the original code from Wiznet that this is based on is full
> of poor coding practices :-))?
>
> Peter Van Epp
>
>
> Sdcc-user mailing list
> Sdcc-u***@*ists.sourceforge.net
>https://lists.sourceforge.net/lists/listinfo/sdcc-user>




Sdcc-user mailing list
Sdcc-user@lists.sourceforge.nethttps://lists.sourceforge.net/lists/listinfo/sdcc-user

Offline

#9 Jan. 1, 2011 20:44:38

Peter V.
Registered: 2009-11-02
Reputation: +  0  -
Profile   Send e-mail  

[Sdcc-user] Is this a bug?


On Sat, Jan 01, 2011 at 11:48:33AM +0100, Maarten Brock wrote:
> Peter,
>
> I only see a bug that was there after 5560 and has been solved again
> in 3.0.1. There is no conversion from integer to pointer without
> cast in this line:
>
> invalid_ip = IINCHIP_READ(UIPR0+1);
>
> So there should be no warning.
>
> Maarten
>

Thanks I wasn't sure whether pointer arithmatic was still legal in C.
Indeed on a current 3.0.1 snap there is no warning.
As well I found (and reported as a bug) that post 5559 8051 near
pointers look to be broken which looks to be the cause of why the code that
worked on 2.9.0 release doesn't on 3.0.1.

Peter Van Epp


Sdcc-user mailing list
Sdcc-user@lists.sourceforge.nethttps://lists.sourceforge.net/lists/listinfo/sdcc-user

Offline

Board footer

Moderator control

Enjoy the 23rd of October
PoweredBy

The Forums are managed by develissimo stuff members, if you find any issues or misplaced content please help us to fix it. Thank you! Tell us via Contact Options
Leave a Message
Welcome to Develissimo Live Support