Bug 15774 - Conflicting function decls not diagnosed
Summary: Conflicting function decls not diagnosed
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 3.4.0
: P2 normal
Target Milestone: ---
Assignee: Kai Tietz
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-06-02 10:54 UTC by Danny Smith
Modified: 2011-02-19 21:26 UTC (History)
4 users (show)

See Also:
Host: i386-pc-mingw32
Target: i386-pc-mingw32
Build: i386-pc-mingw32
Known to work:
Known to fail: 2.95.3, 3.3.3, 3.4.0, 4.0.0, 4.1.0, 4.2.1, 4.3.0
Last reconfirmed: 2006-06-30 02:48:45


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Danny Smith 2004-06-02 10:54:09 UTC
The C++ frontend doesn't always call targetm.compare_type_attributes
when comparing function decls in decl.c: decls_match.  As a result
conflicting decls are not diagnosed.  Here is a testcase.

/* stdcall.c */
extern void foo(int);

void bar() { foo(1);}

void __attribute__((stdcall)) foo(int i) {};

This should casue fatal error because of type conflict between
declaration and definition of foo.

In C it does.

D:\develop\bugs>gcc -S -Wall stdcall.c 
> stdcall.c:5: error: conflicting types for 'foo'
> stdcall.c:1: error: previous declaration of 'foo' was here

In C++ it doesn't

D:\develop\bugs>gcc -xc++ -S -Wall stdcall.c
compiles without error.

But it produces bad code.  This is easier to spot on windows targets because
of the target specific stdcall decoration.

        .file        "stdcall.c"
        .text
        .align 4
        .align 2
.globl __Z3barv
        .def        __Z3barv;        .scl        2;        .type        
32;        .endef
__Z3barv:
        pushl        %ebp
        movl        %esp, %ebp
        subl        $8, %esp
        subl        $12, %esp
        pushl        $1
        call        __Z3fooi   <<< undefined
        addl        $16, %esp  <<< Should be $12.
        leave
        ret
        .align 2
.globl __Z3fooi@4
        .def        __Z3fooi@4;        .scl        2;        .type        
32;        .endef
__Z3fooi@4:
        pushl        %ebp
        movl        %esp, %ebp
        leave
        ret        $4
        .def        __Z3fooi;        .scl        3;        .type        
32;        .endef


The testcase fails in 3.4.0 and 3.5.0 on i386-i386-pc-mingw32


The following patch to cp/declc fixes the testcase.  I don't think it is right, 
but I'll run it throw bootstrap and regtest and  submit proper patch if it 
doesn't cause any other problems.

Danny

Index: decl.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/decl.c,v
retrieving revision 1.1212
diff -c -3 -p -r1.1212 decl.c
*** decl.c	28 May 2004 21:58:15 -0000	1.1212
--- decl.c	2 Jun 2004 10:35:33 -0000
*************** decls_match (tree newdecl, tree olddecl)
*** 1021,1027 ****
  	    }
  #endif
  	  else
! 	    types_match = compparms (p1, p2);
  	}
        else
  	types_match = 0;
--- 1021,1030 ----
  	    }
  #endif
  	  else
! 	    types_match = compparms (p1, p2)
! 			  && comptypes (TREE_TYPE (newdecl),
! 					 TREE_TYPE (olddecl),
! 					 COMPARE_REDECLARATION);
  	}
        else
  	types_match = 0;
Comment 1 Andrew Pinski 2004-06-02 11:37:54 UTC
Confirmed.
Comment 2 Giovanni Bajo 2004-06-28 01:04:37 UTC
Danny,

did you test the patch? What was the outcome?
Comment 3 Danny Smith 2004-06-28 01:42:12 UTC
Yes I did test it and it was wrong, because it appears  C++ front end handles 
attributes of class member functions differently than global functions. 
Attributes of member functions are only honoured when declared within class 
definition,  which makes sense to me.  So for something like this (g++-old-
deja/g++.ext/attrib2.C) my patch would cause an error to be emitted.  Should it?
Should C++ require (as does C) that the subsequent definition also have an 
explicit attribute?

// { dg-do run { target i?86-*-* } }
// Test that stdcall doesn't prevent us from using op delete.
// Contributed by Jason Merrill <jason@cygnus.com>

struct A {
  void operator delete (void *) __attribute__ ((stdcall));
};

void foo()
{
  A* ap = new A;
  delete ap;
}

void  A:: operator delete (void *)  { }

int main()
{
  A* ap = new A;
  delete ap;
}
Comment 4 Giovanni Bajo 2004-06-28 07:40:38 UTC
I think the idea is that if you *ADD* an attribute in a later redeclaration, 
you should get an error (like in your testcase), but if you omit it in the 
redeclaration it is ok (the attribute is implicit in that case).

// OK
void foo(void) stdcall;
void foo(void) {}

// OK
void bar(void) stdcall;
void bar(void) stdcall {}

// ERROR
void foobar(void);
void foobar(void) stdcall {}


Jason, would you kindly comment on this? We need to know which way to proceed 
with this patch.
Comment 5 Jason Merrill 2004-06-29 03:03:15 UTC
Subject: Re:  Conflicting function decls not diagnosed

On 28 Jun 2004 07:40:43 -0000, "giovannibajo at libero dot it" <gcc-bugzilla@gcc.gnu.org> wrote:

> I think the idea is that if you *ADD* an attribute in a later
> redeclaration, you should get an error (like in your testcase), but if
> you omit it in the redeclaration it is ok (the attribute is implicit in
> that case).

> Jason, would you kindly comment on this? We need to know which way to proceed 
> with this patch.

That makes sense to me as a general rule.

Jason
Comment 6 Danny Smith 2007-09-19 21:52:46 UTC
With 4.1, 4.2 and trunk, there is still no diagnosis of change in calling convention in decl,  but the bug in generated code is exposed only with -fno-unit-at-time.
Danny
Comment 7 Kai Tietz 2010-12-25 10:41:09 UTC
Author: ktietz
Date: Sat Dec 25 10:41:05 2010
New Revision: 168241

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=168241
Log:
2010-12-25  Kai Tietz  <kai.tietz@onevision.com>

	PR c++/15774
	* decl.c (decls_match): Check for FUNCTION_DECL
	also for identity of compatible attributes.


ChangeLog gcc/testsuite

2010-12-25  Kai Tietz  <kai.tietz@onevision.com>

	PR c++/15774
	* g++.dg/warn/pr15774-1.C: New test.
	* g++.dg/warn/pr15774-2.C: New test.


Added:
    trunk/gcc/testsuite/g++.dg/warn/pr15774-1.C
    trunk/gcc/testsuite/g++.dg/warn/pr15774-2.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/decl.c
    trunk/gcc/testsuite/ChangeLog
Comment 8 Kai Tietz 2011-01-13 21:40:17 UTC
This bug is fixed on trunk (4.6) version. But the final solution needs some more work, so that those kind of issues are handled in type-comparision directly and double code (as present in the moment) can be avoided.
Comment 9 Kai Tietz 2011-02-19 21:26:39 UTC
After discussing in more detail with Jason, I'll close this bug as fixed.