bugGNU troff - Bugs: bug #52335, src: Some warnings from the...

 
 

bug #52335: src: Some warnings from the compiler that need fixing or explanations in the code

Submitted by:  Bjarni Ingi Gislason <bjarniig>
Submitted on:  Sat 04 Nov 2017 12:25:21 AM UTC  
 
Category:  Core Severity:  3 - Normal
Item Group:  Build/Installation Status:  Fixed
Privacy:  Public Assigned to:  G. Branden Robinson <gbranden>
Open/Closed:  Closed Planned Release:  1.22.4

Add a New Comment (Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

Mon 18 Jun 2018 07:45:45 AM UTC, comment #7:

commit c0d8b9e264bc2657fbf78f1c7855fc1be534b265
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Wed Nov 15 22:02:08 2017 -0500

    * src/libs/libgroff/new.cpp: Fix compiler warning.
   
    Define sized version of overloaded delete operator.  Duplicate code from
    unsized delete operator because simply calling through to it provokes
    another warning, -Wdelete-incomplete.

commit 001e6c139f2ecc6ea8a0476fbbf4fdf5be8f301c
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Sun Nov 12 10:14:35 2017 -0500

    src: Document intentional case fallthrough.
   
    Per discussion with Bjarni Ingi Gislason, this should resolve compiler
    warnings from GCC 7 with -Wimplicit-fallthrough.  He had to do the
    testing for me since I'm still on GCC 6.3 (which doesn't emit these
    warnings at all).

G. Branden Robinson <gbranden>
Project MemberIn charge of this item.
Sun 12 Nov 2017 03:20:39 PM UTC, comment #6:

I've pushed fixes for all of these except the sized destructor problem, for which I'm turning to the mailing list for help.

G. Branden Robinson <gbranden>
Project MemberIn charge of this item.
Sat 11 Nov 2017 03:42:29 AM UTC, comment #5:

  Comment to comment #2

  I added

break; /* "fatal" aborts the excution */

  after each "fatal(...)" line, even in the "default" case.

  No diagnostic.

  gcc (Debian 7.2.0-12) 7.2.1 20171025 C

  The used options are:

-Wall -Wextra -Wformat=2  -fstack-protector-strong -fno-common -funsigned-char -gdwarf-2 -O0
-fvar-tracking-assignments

Bjarni Ingi Gislason <bjarniig>
Sat 11 Nov 2017 03:19:11 AM UTC, comment #4:

  Comment to comment #1

  The patch silences the warning (gcc (Debian 7.2.0-12) 7.2.1 20171025 C)

  The used options are:

-Wall -Wextra -Wformat=2  -fstack-protector-strong -fno-common -funsigned-char -gdwarf-2 -O0
-fvar-tracking-assignments

Bjarni Ingi Gislason <bjarniig>
Fri 10 Nov 2017 01:11:29 PM UTC, comment #3:

The following quiets the sized-deallocation warning, but the most obvious solution (to me) causes a different warning.

Comments welcome.

diff --git a/src/libs/libgroff/new.cpp b/src/libs/libgroff/new.cpp
index 7a1a4730..dc2580f4 100644
--- a/src/libs/libgroff/new.cpp
+++ b/src/libs/libgroff/new.cpp
@@ -67,3 +67,22 @@ void operator delete(void *p)
     free(p);
#endif /* COOKIE_BUG */
}
+
+void operator delete(void *p,
+                    __attribute__((__unused__)) long unsigned int size)
+{
+  // It's ugly to duplicate the code from delete(void *) above, but if
+  // we don't, g++ 6.3 can't figure out we're calling through it to
+  // free().
+  //
+  // In function 'void operator delete(void*, long unsigned int)':
+  //   warning: deleting 'void*' is undefined [-Wdelete-incomplete]
+  //delete p;
+#ifdef COOKIE_BUG
+  if (p)
+    free((void *)((char *)p - 8));
+#else
+  if (p)
+    free(p);
+#endif /* COOKIE_BUG */
+}

G. Branden Robinson <gbranden>
Project MemberIn charge of this item.
Fri 10 Nov 2017 11:44:11 AM UTC, comment #2:

I think the instances in
src/preproc/grn/hdb.cpp are false positives as well.

We might be able to silence this warning by decorating fatal_error_exit() in src/libs/libgroff/fatal.cpp with _attribute_(noreturn), but I'm not positive.

G. Branden Robinson <gbranden>
Project MemberIn charge of this item.
Fri 10 Nov 2017 11:36:49 AM UTC, comment #1:

Hi Bjarni,

I believe fall-through is the correct behavior for groff.cpp, line 295.

However, I don't have GCC 7 handy. Can you test the following patch, please?

diff --git a/src/roff/groff/groff.cpp b/src/roff/groff/groff.cpp
index 46cbf018..ef654f6d 100644
--- a/src/roff/groff/groff.cpp
+++ b/src/roff/groff/groff.cpp
@@ -293,6 +293,7 @@ int main(int argc, char **argv)
       break;
     case 'o':
       oflag = 1;
+      __attribute__ ((fallthrough));
     case 'f':
     case 'm':
     case 'r':

G. Branden Robinson <gbranden>
Project MemberIn charge of this item.
Sat 04 Nov 2017 12:25:21 AM UTC, original submission:

  These need fixing, or if correct, an explanation should be added to the
code.

###
src/libs/libgroff/new.cpp:60:6: warning: the program should also define 'void operator delete(void*, long unsigned int)' [-Wsized-deallocation]
void operator delete(void *p)

###

src/roff/groff/groff.cpp: In function 'int main(int, char**)':
src/roff/groff/groff.cpp:295:13: warning: this statement may fall through [-Wimplicit-fallthrough=]
       oflag = 1;
       ~~~~~~^~~
src/roff/groff/groff.cpp:296:5: note: here
     case 'f':

###

src/preproc/grn/hdb.cpp: In function 'int DBGetType(char*)':
src/preproc/grn/hdb.cpp:247:12: warning: this statement may fall through [-Wimplicit-fallthrough=]
       fatal("unknown element type");
       ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
src/preproc/grn/hdb.cpp:249:3: note: here
   case 'B':
   ^~~~
src/preproc/grn/hdb.cpp:258:12: warning: this statement may fall through [-Wimplicit-fallthrough=]
       fatal("unknown element type");
       ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
src/preproc/grn/hdb.cpp:260:3: note: here
   case 'T':
   ^~~~
src/preproc/grn/hdb.cpp:269:12: warning: this statement may fall through [-Wimplicit-fallthrough=]
       fatal("unknown element type");
       ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
src/preproc/grn/hdb.cpp:271:3: note: here
   default:
###

Bjarni Ingi Gislason <bjarniig>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by gbranden (Posted a comment)
  • -email is unavailable- added by bjarniig (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-06-18 gbranden Open/ClosedOpen => Closed
    2018-06-18 gbranden CategoryNone => Core
        Item GroupNone => Build/Installation
        Planned ReleaseNone => 1.22.4
    2017-11-16 gbranden StatusNeed Info => Fixed
        Assigned toNone => gbranden
    2017-11-10 gbranden StatusNone => Need Info

    Back to the top


    Powered by Savane 3.4