diff mbox series

elf: fix handling of negative numbers in dl-printf

Message ID 20230503211641.5742-1-royeldar0@gmail.com
State Changes Requested
Headers
Series elf: fix handling of negative numbers in dl-printf |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

Roy Eldar May 3, 2023, 9:16 p.m. UTC 
  _dl_debug_vdprintf is a bare-bones printf implementation; currently
printing a signed integer (using "%d" format specifier) behaves
incorrectly when the number is negative, as it just prints the
corresponding unsigned integer, preceeded by a minus sign.

For example, _dl_printf("%d", -1) would print '-4294967295'.

Signed-off-by: Roy Eldar <royeldar0@gmail.com>
---
 elf/dl-printf.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Comments

Florian Weimer May 8, 2023, 1:42 p.m. UTC | #1 
* Roy Eldar via Libc-alpha:

> _dl_debug_vdprintf is a bare-bones printf implementation; currently
> printing a signed integer (using "%d" format specifier) behaves
> incorrectly when the number is negative, as it just prints the
> corresponding unsigned integer, preceeded by a minus sign.
>
> For example, _dl_printf("%d", -1) would print '-4294967295'.
>
> Signed-off-by: Roy Eldar <royeldar0@gmail.com>

We've discussed this during our patch review call.

Ideally we'd want a separate test case for this routine.  You could add
a statically-linked test case that calls _dl_debug_vdprintf directly
with a pipe descriptor.  Is this something you'd be interested in
working on?

Thanks,
Florian
Roy Eldar May 8, 2023, 6:08 p.m. UTC | #2 
Hi,

First of all, thanks for the quick response.

Florian Weimer <fweimer@redhat.com> wrote:

> Ideally we'd want a separate test case for this routine.  You could add
> a statically-linked test case that calls _dl_debug_vdprintf directly
> with a pipe descriptor.  Is this something you'd be interested in
> working on?

I'll be happy to work on that.

Thanks,
Roy
Roy Eldar May 24, 2023, 5:41 p.m. UTC | #3 
Hi,

Is there any update on that?

I've written a test case for _dl_debug_vdprintf in a different patch [1].

[1] https://public-inbox.org/libc-alpha/20230515202942.8307-1-royeldar0@gmail.com/
Florian Weimer May 25, 2023, 7:46 a.m. UTC | #4 
* Roy Eldar via Libc-alpha:

> _dl_debug_vdprintf is a bare-bones printf implementation; currently
> printing a signed integer (using "%d" format specifier) behaves
> incorrectly when the number is negative, as it just prints the
> corresponding unsigned integer, preceeded by a minus sign.
>
> For example, _dl_printf("%d", -1) would print '-4294967295'.
>
> Signed-off-by: Roy Eldar <royeldar0@gmail.com>
> ---
>  elf/dl-printf.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/elf/dl-printf.c b/elf/dl-printf.c
> index e8b9900370..977ac330b6 100644
> --- a/elf/dl-printf.c
> +++ b/elf/dl-printf.c

Please update the copyright staement with

  Copyright The GNU Toolchain Authors.

Then I'm going to apply it together with the test.

Thanks,
Florian
diff mbox series

Patch

diff --git a/elf/dl-printf.c b/elf/dl-printf.c
index e8b9900370..977ac330b6 100644
--- a/elf/dl-printf.c
+++ b/elf/dl-printf.c
@@ -150,19 +150,25 @@  _dl_debug_vdprintf (int fd, int tag_p, const char *fmt, va_list arg)
 		    if (long_mod)
 		      {
 			if ((long int) num < 0)
-			  negative = true;
+			  {
+			    num = -num;
+			    negative = true;
+			  }
 		      }
 		    else
 		      {
 			if ((int) num < 0)
 			  {
-			    num = (unsigned int) num;
+			    num = -(unsigned int) num;
 			    negative = true;
 			  }
 		      }
 #else
 		    if ((int) num < 0)
-		      negative = true;
+		      {
+			num = -num;
+			negative = true;
+		      }
 #endif
 		  }