[Bug target/57631] New: spurious warning for avr interrupts with asm labels

[Bug target/57631] New: spurious warning for avr interrupts with asm labels

[pebbles at riseup dot net]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57631

            Bug ID: 57631
           Summary: spurious warning for avr interrupts with asm labels
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: pebbles at riseup dot net

Hi,

Currently AVR interrupts do not mix well with asm labels, as:

// test.c ...
__attribute__((signal,used))
void __vector_1() { asm(""); }

__attribute__((signal,used))
void pretendName() asm("__vector_2");
void pretendName() { asm(""); }
// ... test.c

Gives:

$ avr-gcc test.c -S
test.c: In function ‘pretendName’:
test.c:6:6: warning: ‘pretendName’ appears to be a misspelled signal handler
[enabled by default]

'__vector_1' is accepted by the compiler fine, but 'pretendName' is rejected. 
A quick view of test.s shows 'pretendName' is correctly translated to
'__vector_2'.

If one adds a main() function to test.c, links and objdumps the result, both
functions are properly inserted into the interrupt table.

There is no misspelling here.  This warning should not be issued, and makes
-Werror die.

I'm on Debian Squeeze.  I've tried with this version of gcc:

COLLECT_GCC=avr-gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/avr/4.7.2/lto-wrapper
Target: avr
Configured with: ../src/configure -v --enable-languages=c,c++ --prefix=/usr/lib
--infodir=/usr/share/info --mandir=/usr/share/man --bindir=/usr/bin
--libexecdir=/usr/lib --libdir=/usr/lib --enable-shared --with-system-zlib
--enable-long-long --enable-nls --without-included-gettext --disable-libssp
--build=i486-linux-gnu --host=i486-linux-gnu --target=avr
Thread model: single
gcc version 4.7.2 (GCC)

And also with trunk:

Using built-in specs.
COLLECT_GCC=/home/username/build/avr-gcc-master/bin/bin/avr-gcc
COLLECT_LTO_WRAPPER=/home/username/build/avr-gcc-master/bin/libexec/gcc/avr/4.9.0/lto-wrapper
Target: avr
Configured with: src/configure -v --enable-languages=c,c++
--prefix=/home/username/build/avr-gcc-master/bin --enable-shared
--with-system-zlib --enable-long-long --enable-nls --without-included-gettext
--disable-libssp --target=avr
Thread model: single
gcc version 4.9.0 20130614 (experimental) (GCC)
>From gcc-bugs-return-424503-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Sun Jun 16 22:34:30 2013
Return-Path: <gcc-bugs-return-424503-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 19265 invoked by alias); 16 Jun 2013 22:34:30 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 19242 invoked by uid 48); 16 Jun 2013 22:34:27 -0000
From: "dominiq at lps dot ens.fr" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug fortran/53296] Segfault on non-constant character array constructor containing kind spec
Date: Sun, 16 Jun 2013 22:34:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: fortran
X-Bugzilla-Version: 4.6.1
X-Bugzilla-Keywords: wrong-code
X-Bugzilla-Severity: normal
X-Bugzilla-Who: dominiq at lps dot ens.fr
X-Bugzilla-Status: NEW
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields: bug_status cf_reconfirmed_on everconfirmed
Message-ID: <bug-53296-4-cmLsqZPhOh@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-53296-4@http.gcc.gnu.org/bugzilla/>
References: <bug-53296-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2013-06/txt/msg00882.txt.bz2
Content-length: 515

http://gcc.gnu.org/bugzilla/show_bug.cgi?idS296

Dominique d'Humieres <dominiq at lps dot ens.fr> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2013-06-16
     Ever confirmed|0                           |1

--- Comment #3 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
Still present at revision 200133.

[Bug target/57631] spurious warning for avr interrupts with asm labels

[pebbles at riseup dot net]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57631

--- Comment #1 from pebbles at riseup dot net ---
Here is a more complete test:

// test.c ...
__attribute__((signal,used))
void __vector_1() { asm(""); } // should not trigger warning, should be placed
in __vectors

__attribute__((signal,used))
void pretendName() asm("__vector_2");
void pretendName() { asm(""); } // should not trigger warning, should be placed
in __vectors 

__attribute__((signal,used))
void __victor_3() { asm(""); } // should trigger warning, not be placed in
__vectors

__attribute__((signal,used))
void __vector_4() asm("__victor_4");
void __vector_4() { asm(""); } // should trigger warning, not be placed in
__vectors

int main() { return 0; }
// ... test.c

Really, I feel this should either be a disableable warning (if users are
expected to create interrupts and signals that will not be placed in the IV) or
an error.

[Bug target/57631] spurious warning for avr interrupts with asm labels

[pebbles at riseup dot net]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57631

--- Comment #2 from pebbles at riseup dot net ---
Created attachment 30314
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30314&action=edit
avr.c: avr_set_current_function: 'name' uses assembler name if set

[Bug target/57631] [patch] spurious warning for avr interrupts with asm labels

[gjl at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57631

Georg-Johann Lay <gjl at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |avr
           Priority|P3                          |P5
             Status|UNCONFIRMED                 |WAITING
   Last reconfirmed|                            |2013-06-21
                 CC|                            |gjl at gcc dot gnu.org
     Ever confirmed|0                           |1
           Severity|normal                      |enhancement

--- Comment #3 from Georg-Johann Lay <gjl at gcc dot gnu.org> ---
Would you explain what you are trying to achieve?

You can name the function __vectorFOO or __vector_my_ISR_function or whatever
without raising a warning.

[Bug target/57631] [patch] spurious warning for avr interrupts with asm labels

[pebbles at riseup dot net]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57631

--- Comment #4 from pebbles at riseup dot net ---
(In reply to Georg-Johann Lay from comment #3)
> Would you explain what you are trying to achieve?

For one thing, I'm coding in C++, so the handlers may be mangled to the wrong
symbols unless I tell the compiler what to name them.  The warning is
misleading at least in this case.

In C++ it's conventional to place symbols inside namespaces or classes over
prefixing them with underscores.  I'm writing a library and would like users to
be able to define the handlers with names and scopes that match the style
conventions of the project.

The __ prefix generally indicates that a symbol should not be used by a library
user, and I'm trying to indicate the opposite.

> You can name the function __vectorFOO or __vector_my_ISR_function or
> whatever without raising a warning.

But that requires reading the source of GCC, which I have begun doing but is
usually not a prerequisite for coding.  The warning should tell me that
straight out.

[Bug target/57631] [patch] spurious warning for avr interrupts with asm labels

[gjl at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57631

Georg-Johann Lay <gjl at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |pebbles at riseup dot net

--- Comment #7 from Georg-Johann Lay <gjl at gcc dot gnu.org> ---
(In reply to pebbles from comment #6)

> This seems like a very small patch that makes GCC more extendable and
> accurate.  I am not too attached to these things changing, although I am
> curious what the reason is for not considering the patch.

You can propose and explain your change in the gcc-patches@ mailing list, of
course.  That is the right place to get changes into the compiler.

[Bug target/57631] [patch] spurious warning for avr interrupts with asm labels

[pebbles at riseup dot net]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57631

--- Comment #8 from pebbles at riseup dot net ---
(In reply to Georg-Johann Lay from comment #7)
> (In reply to pebbles from comment #6)
> 
> > This seems like a very small patch that makes GCC more extendable and
> > accurate.  I am not too attached to these things changing, although I am
> > curious what the reason is for not considering the patch.
> 
> You can propose and explain your change in the gcc-patches@ mailing list, of
> course.  That is the right place to get changes into the compiler.

All right, thank you.  I only started trying to make a patch after noticing
what seemed like a bug(?)

I see from patch submission docs I should add testcases as well.  If you have
any more feedback or help, thank you.

[Bug target/57631] [patch] spurious warning for avr interrupts with asm labels

[pebbles at riseup dot net]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57631

pebbles at riseup dot net changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |NEW
           Assignee|pebbles at riseup dot net          |unassigned at gcc dot gnu.org

--- Comment #9 from pebbles at riseup dot net ---
I would like to learn to find out if this is a bug and contribute a fix to the
gcc sources.  At the moment building gcc loads my computer for a day, and it is
difficult to develop on.  I plan to pursue this when I am set up better, but if
anybody else ever cares about it they should look at it.

As far as I can tell the patch I attached is good but I haven't turned my test
into a testcase or read the gcc development docs yet.

[Bug target/57631] [patch] spurious warning for avr interrupts with asm labels

[gjl at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57631

--- Comment #10 from Georg-Johann Lay <gjl at gcc dot gnu.org> ---
I can have a look at it, but it might take a month or so until I get back to
this issue.

If I understand correctly, bottom line of what you need is that avr-gcc does
not warn for ISR names that are not of the form __vector*

Maybe a new command line option -W[no-]isr-name or similar is more general? 
The default of the option would be 'on' per default.

[Bug target/57631] [patch] spurious warning for avr interrupts with asm labels

[pebbles at riseup dot net]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57631

--- Comment #12 from pebbles at riseup dot net ---
(In reply to Georg-Johann Lay from comment #10)
> I can have a look at it, but it might take a month or so until I get back to
> this issue.
> 
> If I understand correctly, bottom line of what you need is that avr-gcc does
> not warn for ISR names that are not of the form __vector*
> 
> Maybe a new command line option -W[no-]isr-name or similar is more general? 
> The default of the option would be 'on' per default.

Thank you, yes!  Really, it seems this warning would be very useful almost all
the time if it only responded to the name the linker sees rather than the name
in the source.  This is the change I tried to make in the attached patch.

(In reply to Manuel López-Ibáñez from comment #11)
> (In reply to pebbles from comment #9)
> > I would like to learn to find out if this is a bug and contribute a fix to
> > the gcc sources.  At the moment building gcc loads my computer for a day,
> > and it is difficult to develop on.  I plan to pursue this when I am set up
> > better, but if anybody else ever cares about it they should look at it.
> 
> You can use the GCC Farm for gcc development for free.
> http://gcc.gnu.org/wiki/CompileFarm
> 
> There are scripts that simplify building and testing patches. See
> contrib/patch_test.sh and other scripts in that directory.
> 
> You can also use my own script
> http://gcc.gnu.org/wiki/
> ManuelL%C3%B3pezIb%C3%A1%C3%B1ez?action=AttachFile&do=view&target=gccfarming

Thank you.  I've submitted a compile farm application.
>From gcc-bugs-return-425154-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Wed Jun 26 03:23:09 2013
Return-Path: <gcc-bugs-return-425154-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 28846 invoked by alias); 26 Jun 2013 03:23:09 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 28797 invoked by uid 48); 26 Jun 2013 03:23:02 -0000
From: "allan at archlinux dot org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug c/57653] filename information discarded when using -imacros
Date: Wed, 26 Jun 2013 03:23:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: c
X-Bugzilla-Version: 4.8.2
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: allan at archlinux dot org
X-Bugzilla-Status: NEW
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields:
Message-ID: <bug-57653-4-D3F8yBaVih@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-57653-4@http.gcc.gnu.org/bugzilla/>
References: <bug-57653-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2013-06/txt/msg01533.txt.bz2
Content-length: 430

http://gcc.gnu.org/bugzilla/show_bug.cgi?idW653

--- Comment #19 from Allan McRae <allan at archlinux dot org> ---
That patch works.  With -imacros foo.h:

LC_ENTER  foo.c
LC_RENAME <command-line>
LC_ENTER  foo.h
LC_LEAVE  0x0
LC_ENTER  /usr/include/stdc-predef.h
LC_LEAVE  0x0
LC_RENAME foo.c
<- correct output
LC_LEAVE  0x0

All other combinations of -include/-imacros/-ffreestanding/no foo.h  all are
unchanged as expected.

[Bug target/57631] [patch] spurious warning for avr interrupts with asm labels

[gjl at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57631

Georg-Johann Lay <gjl at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
      Known to work|                            |4.7.4
         Resolution|---                         |FIXED
   Target Milestone|---                         |4.8.2
      Known to fail|                            |4.7.3

--- Comment #13 from Georg-Johann Lay <gjl at gcc dot gnu.org> ---
Fixed in r200901 (trunk), r200902 (4.8.2) , r200903 (4.7.4).

[Bug target/57631] Use assembler name for sanity checking of ISR names provided it is set

[mhlavink at redhat dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57631

Michal Hlavinka <mhlavink at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mhlavink at redhat dot com

--- Comment #14 from Michal Hlavinka <mhlavink at redhat dot com> ---
This causes regression:
protocols/pulses_avr.cpp:104:1: warning: '_vector_12' appears to be a
misspelled signal handler [enabled by default]
 ISR(TIMER1_COMPA_vect) //2MHz pulse generation (BLOCKING ISR)
 ^

the "+1" in the fix(gcc/config/avr/avr.c):
 name = DECL_ASSEMBLER_NAME_SET_P (decl)
        /* Remove the leading '*' added in set_user_assembler_name.  */
        ? 1 + IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl))
        : IDENTIFIER_POINTER (DECL_NAME (decl));

makes name == _vector_12 instead of __vector_12 and it triggers warning a few
lines later

tested version: gcc-4.8.2