GNU bug report logs - #78880
od Heap-buffer overflow

Previous Next

Full log

View this message in rfc822 format

From: Pádraig Brady <P <at> draigBrady.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 78880 <at> debbugs.gnu.org
Subject: bug#78880: od Heap-buffer overflow
Date: Sun, 29 Jun 2025 20:59:14 +0100

On 29/06/2025 13:17, Pádraig Brady wrote:
> On 29/06/2025 05:25, Paul Eggert wrote:
> 
>   > od: omit some duplicate code
>   > On x86-64 (for example) print_long, print_long_long, and
>   > print_intmax all behave identically, so give GCC enough info so
>   > that it generates code for just one of these functions.
>   > * src/od.c (enum size_spec): Arrange for enum values to
>   > be the same if they represent types that behave the same.
>   > (width_bytes, ISPEC_TO_FORMAT, decode_one_format):
>   > Match the enum size_spec changes.
> 
> The patch above causes a warning on some systems:
> 
>     src/od.c: In function 'decode_one_format':
>     src/od.c:900:28: error: duplicated 'if' condition [-Werror=duplicated-cond]
>       900 |         else if (size_spec == FLOAT_HALF)
>           |                  ~~~~~~~~~~^~~~~~~~~~~~~
>     src/od.c:895:28: note: previously used here
>       895 |         else if (size_spec == FLOAT_SINGLE)
>           |                  ~~~~~~~~~~^~~~~~~~~~~~~~~
> 
> If we want the compiler to just apply Dead Code Elimination here,
> then it may be best to push/pop ignoring that warning ?

FYI, related to this I see our coverity instance is complaining about deadcode
in the following block, which might be suppressed with this change:

diff --git a/src/od.c b/src/od.cindex 50319aa83..c621ef227 100644
--- a/src/od.c
+++ b/src/od.c
@@ -802,12 +802,17 @@ decode_one_format (char const *s_orig, char const *s, char const **next,
          and prefer INTMAX to LONG_LONG.  */
       print_function
         = (size_spec == INT ? print_int
+           /* coverity[DEADCODE] */
            : size_spec == SHORT ? (fmt == SIGNED_DECIMAL
                                    ? print_s_short : print_short)
+           /* coverity[DEADCODE] */
            : size_spec == CHAR ? (fmt == SIGNED_DECIMAL
                                   ? print_s_char : print_char)
+           /* coverity[DEADCODE] */
            : size_spec == LONG ? print_long
+           /* coverity[DEADCODE] */
            : size_spec == INTMAX ? print_intmax
+           /* coverity[DEADCODE] */
            : size_spec == LONG_LONG ? print_long_long
            : (affirm (false), nullptr));
       break;


That's a bit messy to me though (the existing comment should
suffice for humans to know the deadcode is intentional).
So I've manually suppressed that error instance in our coverity instance.

cheers,
Padraig

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.