[sdcc-devel] [ sdcc-Bugs-838241 ] No checks for matching of prototype and definition

[SourceForge.net <no...@so...>]

Bugs item #838241, was opened at 2003-11-07 17:47
Message generated for change (Comment added) made by epetrich
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=100599&aid=838241&group_id=599

Category: C-Front End
Group: fixed
>Status: Closed
Resolution: Fixed
Priority: 5
Submitted By: Stas Sergeev (stsp)
Assigned to: Erik Petrich (epetrich)
Summary: No checks for matching of prototype and definition

Initial Comment:
Hi.

SDCC doesn't seem to check whether the variable is
declared similarly to how it is defined. This leads
to a bad code and sometimes it is difficult to find
the problem.
For example the following code produces no errors
during compile:
---
extern data char a;
idata char a;
---
Now "a" is declared to be in data but defined to be
in idata.
If the declaration is in a header file, every module
that includes that header will be compilled as if the
"a" is in a data, but it will actually be in idata
and the compilled code will end up overwriting some
other memory locations with what is intended to go to
"a".

The following code produced no error either, and
probably even shouldn't, but the generated code is
wrong again:
---
extern idata char a;  //in some header
char a;   //in some C module
---
Now in all modules, except where "char a;" is defined,
"a" is treated as being in idata, but in that module
it defaults to data. I think such a definitions are
correct: I think specifying "idata" in a declaration
should be sufficient and repeating it in a definition
should not be necessary (is this true?)

The following code treats "a" as volatile everywhere
except where it is defined:
---
extern volatile char a;
char a;
["a" is not volatile here - declaration is overridden
by a definition, which is not good]
---
I think this is a bug either. At least it took quite
a some time of mine to figure out why my program
crashes:)

Here is an example that demonstrates the both problems:
---
extern volatile char a;
char a;
extern idata char b;
volatile char b;

char main() {
a = 5;
b = 1;
return a+b;
}
---

The generated asm follows:
---;var.c:7: a = 5;
;     genAssign
	mov	_a,#0x05
;var.c:8: b = 1;
;     genAssign
	mov	_b,#0x01
[_b should be in idata, at least other modules will
expect it to be there based on an extern decl]
;var.c:9: return a+b;
;     genPlus
	mov	a,#0x05
[must be "mov a,_a" since "a" is expected to be
volatile - apparently it is not]
	add	a,_b
	mov	dpl,a
;     genRet
00101$:
	ret
---

SDCC is from CVS, testing the mcs51 port.
$ sdcc -v
SDCC :
mcs51/gbz80/z80/avr/ds390/pic14/pic16/TININative/xa51/ds400/hc08
2.3.6 (Nov  7 2003) (UNIX)

----------------------------------------------------------------------

>Comment By: Erik Petrich (epetrich)
Date: 2003-12-03 02:20

Message:
Logged In: YES 
user_id=635249

Here's another try at fixing this: src/SDCCsymt.c 1.170


----------------------------------------------------------------------

Comment By: Stas Sergeev (stsp)
Date: 2003-11-27 14:11

Message:
Logged In: YES 
user_id=501371

> In any case, I think that requiring the type qualifiers to
> match is best for SDCC.
Yes, and to require people to make their code better, is
always a good idea:)
However, something goes wrong here. I've just tried the
obvious things like
---
extern idata char a;
data char a;
---
and the like variations, and even
---
extern code char a;
const xdata char a;
---
and they all are compiling! I don't know why, I probably
done something bad to your fix since I wasn't able to
compile some valid code with it, but now I am trying the
same with the clean sdcc, and this all still compiles, just
like there was no fix at all.
Am I missing something again, or your fix doesn't catch
those for some reasons?

----------------------------------------------------------------------

Comment By: Erik Petrich (epetrich)
Date: 2003-11-27 03:11

Message:
Logged In: YES 
user_id=635249

> extern volatile char a;
> char a;
> 
> char main()
> {
>   return a;
> }
>
> gcc eats such a code perfectly, the volatileness gets
i> nherited from an extern decl. I don't know what the
> standard says here, but I personally have nothing against
> your approach here.

This seems to have changed; on my newer computer I have gcc
3.2.2 and it complains that the declarations have
conflicting type. However, my older computer has 2.91.66 and
doesn't complain with this example, but if volatile is
replaced with const, it complains too.

I haven't found anything in the C90 or C99 standards
regarding type compatibility between a tentative and final
definition. K&R 2nd ed. says that they should "contain the
same set of type specifiers". This only makes sense if "type
specifiers" is used in the general sense of all the elements
that together specify the type since each type has only a
single "type specifier" in the narrower sense. 

In any case, I think that requiring the type qualifiers to
match is best for SDCC. Otherwise, the following code would
be valid but very difficult to support with SDCC:

char a;   /* tentative definition */
void test(void) {
  a;
}
extern volatile char a; /* final definition */

Since SDCC is a single pass compiler, by the time it would
find out that "a" is volatile, test() would have already
been generated, treating "a" as non-volatile. Oops.


----------------------------------------------------------------------

Comment By: Stas Sergeev (stsp)
Date: 2003-11-25 12:26

Message:
Logged In: YES 
user_id=501371

> is being accepted since the first parameter is always
> passed the same way (in registers or on the stack)
OK, I see, thanks.

> I will look at the rest of the problems further.
Would be nice:)

> Btw, could you please suggest a better way to point sdcc
> to put the constant xdata pointer into code?
Sorry, that appeared to be an example from sdcc manual.
The problem is that it requires LyX which is more recent
than I have here, so sometimes I have problems finding
things is the sdcc manual sources. :-0

----------------------------------------------------------------------

Comment By: Erik Petrich (epetrich)
Date: 2003-11-21 13:10

Message:
Logged In: YES 
user_id=635249

The case of

  extern char test_func(char arg);
  char test_func(char arg) reentrant

is being accepted since the first parameter is always passed 
the same way (in registers or on the stack) for a given port 
regardless of the reentrant attribute. Thus, the generated 
code is still correct dispite the slight declaration mismatch; 
this hack avoided having to change some of the library and 
header files. However,

  extern char test_func(char arg1, char arg2);
  char test_func(char arg1, char arg2) reentrant

will trigger an error, since arg2 is passed differently 
depending on the presence or absence of reentrant.

I will look at the rest of the problems further.


----------------------------------------------------------------------

Comment By: Stas Sergeev (stsp)
Date: 2003-11-21 12:14

Message:
Logged In: YES 
user_id=501371

Hi again.

> This is because the compiler modifies the types for some
> cases. For example:
> const int x;
> becomes
> const code int x;
Exactly! Thinking a bit more about it, I came to a
conclusion that I have to reopen that bug.
Here's why.
The patch is definitely good but perhaps (IMHO) requires
some more work.
Compiling the following (perfectly valid) code, your patch
breaks:
---
xdata at 0 char ptr0;
xdata at 1 char ptr1;
typedef xdata char * xdata_ptr;
extern code const xdata_ptr s[2];
code const xdata_ptr s[2] = { &ptr0, &ptr1 };

char main()
{
  return s[0] == s[1];
}
---

$ sdcc xdata.c 
xdata.c:5: error: extern definition for 's' mismatches with
declaration.
from type 'const-char const-xdata*  code-[2] '
to type 'const-char xdata*  code-[2] '
xdata.c:9: error: code not generated for 'main' due to
previous errors

Btw, could you please suggest a better way to point sdcc
to put the constant xdata pointer into code? Without a
typedef I haven't found any possibility.

Additionally, I have the following problems with your fix:
1. The following code does not compile:
---
extern volatile char a;
char a;

char main()
{
  return a;
}
---

$ sdcc volat.c 
volat.c:2: error: extern definition for 'a' mismatches with
declaration.
from type 'volatile-char'
to type 'char'
volat.c:6: error: code not generated for 'main' due to
previous errors

gcc eats such a code perfectly, the volatileness gets
inherited from an extern decl. I don't know what the
standard says here, but I personally have nothing against
your approach here.

2. Things like
---
extern char test_func(char arg);
char test_func(char arg) reentrant
---
are still not catched as an errors.

3. The mismatch of volatile is still not reliable, neither
are some other things.
The following code compiles:
---
extern xdata at 0 char a;
volatile char a;

char main()
{
  return a;
}
---

It would be a good idea to resolve at least some of that
problems (I know it is difficult:)
All the test cases are attached.

----------------------------------------------------------------------

Comment By: Erik Petrich (epetrich)
Date: 2003-11-20 23:27

Message:
Logged In: YES 
user_id=635249

Yes, it was forgotten debugging output; maybe I shouldn't
stay up so late.

It was displaying storage class (data, xdata, code,  etc)
for the previous and current declaration. The trickiest part
of making sure the declarations match is that the storage
class may not match at this point, but might match later.
This is because the compiler modifies the types for some
cases. For example:

   const int x;
becomes
   const code int x;
for global (level=0) variables

   int x;
becomes
  xdata int x;
for -mmcs51 --model-large

So it was also displaying the current storage class as well
as the anticipated final storage class.

Anyway, src/SDCCsymt.c 1.168 no longer displays this
extraneous information.


----------------------------------------------------------------------

Comment By: Stas Sergeev (stsp)
Date: 2003-11-20 12:06

Message:
Logged In: YES 
user_id=501371

Thank you for fixing the bug!

There are some strange error messages however, compiling
the following code:
---
extern data char a[10];
idata char a[10];

int main() {
  return a[0];
}
---

$ sdcc decls.c 
level = 0
SPEC_SCLS (src) = 8, SPEC_SCLS (dest) = 7
srcScls = 8, destScls = 7
decls.c:2: error: extern definition for 'a' mismatches with
declaration.
from type 'char data-[10] '
to type 'char idata-[10] '
-:0: error: code not generated for 'main' due to previous
errors


What do that messages stand for? Looks like a forgotten
debug output.
The test-case is attached. This is not a problem at all,
just a strange messages:)

----------------------------------------------------------------------

Comment By: Erik Petrich (epetrich)
Date: 2003-11-12 02:33

Message:
Logged In: YES 
user_id=635249

Fixed with src/SDCCsymt.h 1.68 and src/SDCCsymt.c 1.167


----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=100599&aid=838241&group_id=599