[Bug translator/12137] New: SDT fails -pedantic with too-long strings

[Bug translator/12137] New: SDT fails -pedantic with too-long strings

[jistone at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=12137

           Summary: SDT fails -pedantic with too-long strings
           Product: systemtap
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: translator
        AssignedTo: systemtap@sources.redhat.com
        ReportedBy: jistone@redhat.com


While running sdt.exp:

Executing on host: gcc /home/jistone/systemtap/testsuite/systemtap.base/sdt.c 
-g -isystem/home/jistone/systemtap/testsuite -isystem/usr/local/include -Wall
-Wextra -Werror -pedantic  -lm   -o sdt.c.exe.3    (timeout = 300)
cc1: warnings being treated as errors
/home/jistone/systemtap/testsuite/systemtap.base/sdt.c: In function ‘call4’:
/home/jistone/systemtap/testsuite/systemtap.base/sdt.c:20: error: string length
‘526’ is greater than the length ‘509’ ISO C90 compilers are required to
support
/home/jistone/systemtap/testsuite/systemtap.base/sdt.c: In function ‘call5’:
/home/jistone/systemtap/testsuite/systemtap.base/sdt.c:25: error: string length
‘549’ is greater than the length ‘509’ ISO C90 compilers are required to
support
/home/jistone/systemtap/testsuite/systemtap.base/sdt.c: In function ‘call6’:
/home/jistone/systemtap/testsuite/systemtap.base/sdt.c:30: error: string length
‘572’ is greater than the length ‘509’ ISO C90 compilers are required to
support
/home/jistone/systemtap/testsuite/systemtap.base/sdt.c: In function ‘call7’:
/home/jistone/systemtap/testsuite/systemtap.base/sdt.c:35: error: string length
‘595’ is greater than the length ‘509’ ISO C90 compilers are required to
support
/home/jistone/systemtap/testsuite/systemtap.base/sdt.c: In function ‘call8’:
/home/jistone/systemtap/testsuite/systemtap.base/sdt.c:40: error: string length
‘618’ is greater than the length ‘509’ ISO C90 compilers are required to
support
/home/jistone/systemtap/testsuite/systemtap.base/sdt.c: In function ‘call9’:
/home/jistone/systemtap/testsuite/systemtap.base/sdt.c:45: error: string length
‘641’ is greater than the length ‘509’ ISO C90 compilers are required to
support
/home/jistone/systemtap/testsuite/systemtap.base/sdt.c: In function ‘call10’:
/home/jistone/systemtap/testsuite/systemtap.base/sdt.c:50: error: string length
‘666’ is greater than the length ‘509’ ISO C90 compilers are required to
support
compiler exited with status 1
output is:
[repeat of above]
FAIL: compiling sdt.c additional_flags=-pedantic uprobe

The same errors come up for:
FAIL: compiling sdt.c additional_flags=-ansi additional_flags=-pedantic uprobe

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug translator/12137] SDT fails -pedantic with too-long strings

[roland at gnu dot org]

http://sourceware.org/bugzilla/show_bug.cgi?id=12137

Roland McGrath <roland at gnu dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |roland at gnu dot org

--- Comment #1 from Roland McGrath <roland at gnu dot org> 2010-10-20 00:00:17 UTC ---
And that's with the file installed as /usr/local/include/sys/sdt.h?

I thought -isystem defeated that particular lossage--it certainly did in the
tests I did before.  It's only with -pedantic (generally considered useless,
though some nutballs do put it in their builds), by the way.

Any such report needs full details about the gcc version.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug translator/12137] SDT fails -pedantic with too-long strings

[jistone at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=12137

--- Comment #2 from Josh Stone <jistone at redhat dot com> 2010-10-20 00:27:30 UTC ---
(In reply to comment #1)
> And that's with the file installed as /usr/local/include/sys/sdt.h?

Yes, I confirmed this with -E.  For this test, it's via testsuite/sys/sdt.h,
which does get to /usr/local correctly in the end.

> I thought -isystem defeated that particular lossage--it certainly did in the
> tests I did before.  It's only with -pedantic (generally considered useless,
> though some nutballs do put it in their builds), by the way.

I agree, a compiler message of "I'm not required to support this" is a bit
lame, but it is still a regression.  SDT V2 and V1 work fine with -pedantic.

> Any such report needs full details about the gcc version.

Sorry - this is on Fedora 13, both i686 and x86_64.
gcc (GCC) 4.4.4 20100630 (Red Hat 4.4.4-10)

An example from -E:

static void call4(int a, int b, int c, int d)
{
  __asm__ __volatile__ ("990: nop""\n" ".pushsection
.note.stapsdt"",""\"\""",""\"note\"""\n" ".balign 4""\n" ".4byte
992f-991f"",""994f-993f"",""3""\n" "991: .asciz \"stapsdt\"""\n" "992: .balign
4""\n" "993: .8byte 990b""\n" ".8byte _.stapsdt.base""\n" ".8byte 0""\n"
".asciz \"test\"""\n" ".asciz \"mark_d\"""\n" ".asciz \"%n[_SDT_S1]@%[_SDT_A1]
%n[_SDT_S2]@%[_SDT_A2] %n[_SDT_S3]@%[_SDT_A3] %n[_SDT_S4]@%[_SDT_A4]\"""\n"
"994: .balign 4""\n" ".popsection""\n" ".ifndef _.stapsdt.base""\n"
".pushsection .stapsdt.base"",""\"aG\""",""\"progbits\""","
".stapsdt.base"",""comdat""\n" ".weak _.stapsdt.base""\n" ".hidden
_.stapsdt.base""\n" "_.stapsdt.base: .space 1""\n" ".size
_.stapsdt.base"",""1""\n" ".popsection""\n" ".endif""\n" :: [_SDT_S1] "n"
(((__typeof((a) + 0)) -1L > (__typeof((a) + 0)) 0 ? -1 : 1) * (int) sizeof ((a)
+ 0)), [_SDT_A1] "nor" ((a) + 0), [_SDT_S2] "n" (((__typeof((b) + 0)) -1L >
(__typeof((b) + 0)) 0 ? -1 : 1) * (int) sizeof ((b) + 0)), [_SDT_A2] "nor" ((b)
+ 0), [_SDT_S3] "n" (((__typeof((c) + 0)) -1L > (__typeof((c) + 0)) 0 ? -1 : 1)
* (int) sizeof ((c) + 0)), [_SDT_A3] "nor" ((c) + 0), [_SDT_S4] "n"
(((__typeof((d) + 0)) -1L > (__typeof((d) + 0)) 0 ? -1 : 1) * (int) sizeof ((d)
+ 0)), [_SDT_A4] "nor" ((d) + 0));
}

Is it possible to split that into multiple __asm__ statements?

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug translator/12137] SDT fails -pedantic with too-long strings

[roland at gnu dot org]

http://sourceware.org/bugzilla/show_bug.cgi?id=12137

--- Comment #3 from Roland McGrath <roland at gnu dot org> 2010-10-20 00:36:56 UTC ---
This is somewhat mysterious.  F13/x86-64 is the system where I tested this when
I did it in the first place, though perhaps it was a slightly earlier gcc rpm
(I don't recall).  I suppose it's possible I didn't test something I thought I
had.

When I first committed the testsuite cleanups associated with sys/sdt.h, I used
a space after -isystem and then Stan changed that later for reasons I don't
recall ever understanding.  Just in case, try the same file with -isystem /...
instead.

It's certainly feasible to rejigger the macro magic to wind up with more
separate asm statements, though you'll have to take quite some care to make
sure that all the permutations of the macros (the ones for embedding inside
asm, e.g.) continue to work.  I really dislike that "solution" however, since
it also introduces the opportunity for the compiler to emit other code in
between them, which, though it seems unlikely to happen, could very well louse
things up thoroughly in nonobvious ways if it did.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug translator/12137] SDT fails -pedantic with too-long strings

[jistone at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=12137

--- Comment #4 from Josh Stone <jistone at redhat dot com> 2010-10-20 02:04:05 UTC ---
(In reply to comment #3)
> Just in case, try the same file with -isystem /... instead.

Adding a space doesn't help, nor does removing -isystem altogether to pick up
/usr/local/include naturally.  Is the compiler supposed to ignore these
warnings when the macro *expansion* is happening in the user's file?

If splitting the macros might introduce subtle problems, then let's not.  I'd
rather disavow -pedantic and keep robust behavior.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug translator/12137] SDT fails -pedantic with too-long strings

[mjw at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=12137

Mark Wielaard <mjw at redhat dot com> changed:

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

--- Comment #5 from Mark Wielaard <mjw at redhat dot com> 2010-10-20 09:04:09 UTC ---
Note that in previous versions of sdt.h we used __extension__ to suppress any
-pedantic warnings for "weird" constructs:

http://gcc.gnu.org/onlinedocs/gcc/Alternate-Keywords.html

   `-pedantic' and other options cause warnings for many GNU C
extensions.  You can prevent such warnings within one expression by
writing `__extension__' before the expression.  `__extension__' has no
effect aside from this.

But I have some trouble figuring out where the expression starts in this case.
Apparently __asm__ isn't an expression. hmmm.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug translator/12137] SDT fails -pedantic with too-long strings

[jistone at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=12137

--- Comment #6 from Josh Stone <jistone at redhat dot com> 2010-10-20 15:55:48 UTC ---
(In reply to comment #5)
> But I have some trouble figuring out where the expression starts in this case.
> Apparently __asm__ isn't an expression. hmmm.

It's a statement.  You can do:  __extension__ ({ __asm__ __volatile__ (...);
});
But that doesn't fix this warning. :(

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug translator/12137] SDT fails -pedantic with too-long strings

[jistone at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=12137

--- Comment #7 from Josh Stone <jistone at redhat dot com> 2010-10-20 16:32:29 UTC ---
The ".ifndef _.stapsdt.base [...] .endif" block is 198 bytes, and I think it
wouldn't matter if the compiler did anything to split that from the rest,
right?  So if we can make that a separate __asm__ string, that would take
call10's 666 bytes down to 468, within the pedantic 509 limit.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug translator/12137] SDT fails -pedantic with too-long strings

[jistone at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=12137

--- Comment #8 from Josh Stone <jistone at redhat dot com> 2010-10-20 17:52:32 UTC ---
Created attachment 5073
  --> http://sourceware.org/bugzilla/attachment.cgi?id=5073
proposed fix, split out the asm for _.stapsdt.base

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug translator/12137] SDT fails -pedantic with too-long strings

[jistone at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=12137

Josh Stone <jistone at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #9 from Josh Stone <jistone at redhat dot com> 2010-10-20 23:56:05 UTC ---
commit 5470765

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug translator/12137] SDT fails -pedantic with too-long strings

[fche at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=12137

Frank Ch. Eigler <fche at redhat dot com> changed:

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

--- Comment #10 from Frank Ch. Eigler <fche at redhat dot com> 2011-01-15 00:14:55 UTC ---
Note that this still occurs on powerpc (f12 tofu.yyz) for the 9 and 10 cases.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug translator/12137] SDT fails -pedantic with too-long strings

[tromey at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=12137

Tom Tromey <tromey at redhat dot com> changed:

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

--- Comment #11 from Tom Tromey <tromey at redhat dot com> 2011-01-17 14:57:11 UTC ---
(In reply to comment #4)
> Is the compiler supposed to ignore these
> warnings when the macro *expansion* is happening in the user's file?

This is a known gcc bug: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=7263

I think the particular case of __extension__ "overlong string" can be handled
more simply.  This GCC patch seems to fix it for me.

diff --git a/gcc/c-parser.c b/gcc/c-parser.c
index 51df188..b0ef249 100644
--- a/gcc/c-parser.c
+++ b/gcc/c-parser.c
@@ -1045,13 +1045,15 @@ disable_extension_diagnostics (void)
          | (warn_traditional << 2)
          | (flag_iso << 3)
          | (warn_long_long << 4)
-         | (warn_cxx_compat << 5));
+         | (warn_cxx_compat << 5)
+         | (warn_overlength_strings << 6));
   cpp_opts->cpp_pedantic = pedantic = 0;
   warn_pointer_arith = 0;
   cpp_opts->cpp_warn_traditional = warn_traditional = 0;
   flag_iso = 0;
   cpp_opts->cpp_warn_long_long = warn_long_long = 0;
   warn_cxx_compat = 0;
+  warn_overlength_strings = 0;
   return ret;
 }

@@ -1067,6 +1069,7 @@ restore_extension_diagnostics (int flags)
   flag_iso = (flags >> 3) & 1;
   cpp_opts->cpp_warn_long_long = warn_long_long = (flags >> 4) & 1;
   warn_cxx_compat = (flags >> 5) & 1;
+  warn_overlength_strings = (flags >> 6) & 1;
 }

 /* Possibly kinds of declarator to parse.  */

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug translator/12137] SDT fails -pedantic with too-long strings

[tromey at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=12137

--- Comment #12 from Tom Tromey <tromey at redhat dot com> 2011-01-18 18:21:41 UTC ---
I sent a gcc patch:

http://gcc.gnu.org/ml/gcc-patches/2011-01/msg01230.html

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug translator/12137] SDT fails -pedantic with too-long strings

[tromey at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=12137

--- Comment #13 from Tom Tromey <tromey at redhat dot com> 2011-01-27 15:46:25 UTC ---
(In reply to comment #12)
> I sent a gcc patch:
> 
> http://gcc.gnu.org/ml/gcc-patches/2011-01/msg01230.html

This was approved.
I am not going to check it in until the next stage 1, but I think
Jakub may include it in the F15 gcc (I asked him to).
In any case, it would not hurt to prefix the long strings with
"__extension__" now.  It won't break anything, even if gcc does
not have the patch.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.