* [PATCH] elf: fix handling of negative numbers in dl-printf
@ 2023-05-03 21:16 Roy Eldar
2023-05-08 13:42 ` Florian Weimer
2023-05-25 7:46 ` [PATCH] elf: fix handling of negative numbers in dl-printf Florian Weimer
0 siblings, 2 replies; 11+ messages in thread
From: Roy Eldar @ 2023-05-03 21:16 UTC (permalink / raw)
To: libc-alpha; +Cc: Roy Eldar, carlos
_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
@@ -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
}
--
2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] elf: fix handling of negative numbers in dl-printf
2023-05-03 21:16 [PATCH] elf: fix handling of negative numbers in dl-printf Roy Eldar
@ 2023-05-08 13:42 ` Florian Weimer
2023-05-08 18:08 ` Roy E
2023-05-15 20:29 ` [PATCH] elf: add test for dl-printf Roy Eldar
2023-05-25 7:46 ` [PATCH] elf: fix handling of negative numbers in dl-printf Florian Weimer
1 sibling, 2 replies; 11+ messages in thread
From: Florian Weimer @ 2023-05-08 13:42 UTC (permalink / raw)
To: Roy Eldar via Libc-alpha; +Cc: Roy Eldar, carlos
* 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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] elf: fix handling of negative numbers in dl-printf
2023-05-08 13:42 ` Florian Weimer
@ 2023-05-08 18:08 ` Roy E
2023-05-24 17:41 ` Roy E
2023-05-15 20:29 ` [PATCH] elf: add test for dl-printf Roy Eldar
1 sibling, 1 reply; 11+ messages in thread
From: Roy E @ 2023-05-08 18:08 UTC (permalink / raw)
To: Florian Weimer; +Cc: carlos, libc-alpha
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] elf: add test for dl-printf
2023-05-08 13:42 ` Florian Weimer
2023-05-08 18:08 ` Roy E
@ 2023-05-15 20:29 ` Roy Eldar
2023-05-25 7:44 ` Florian Weimer
1 sibling, 1 reply; 11+ messages in thread
From: Roy Eldar @ 2023-05-15 20:29 UTC (permalink / raw)
To: fweimer; +Cc: Roy Eldar, libc-alpha, carlos
This patch checks _dl_debug_vdprintf, by passing various inputs to
_dl_dprintf and comparing the output with invocations of snprintf.
Signed-off-by: Roy Eldar <royeldar0@gmail.com>
---
elf/Makefile | 1 +
elf/tst-dl-printf-static.c | 75 ++++++++++++++++++++++++++++++++++++++
2 files changed, 76 insertions(+)
create mode 100644 elf/tst-dl-printf-static.c
diff --git a/elf/Makefile b/elf/Makefile
index 396ec51424..90ccd65db1 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -275,6 +275,7 @@ tests-static-normal := \
tests-static-internal := \
tst-dl_find_object-static \
+ tst-dl-printf-static \
tst-ptrguard1-static \
tst-stackguard1-static \
tst-tls1-static \
diff --git a/elf/tst-dl-printf-static.c b/elf/tst-dl-printf-static.c
new file mode 100644
index 0000000000..a31759a006
--- /dev/null
+++ b/elf/tst-dl-printf-static.c
@@ -0,0 +1,75 @@
+/* Check _dl_debug_vdprintf.
+ Copyright (C) 2016-2023 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <ldsodefs.h>
+#include <limits.h>
+#include <stdarg.h>
+#include <stdio.h>
+#include <support/check.h>
+#include <support/xunistd.h>
+
+#define BUFSZ 64
+
+#define TEST(fmt, ...) do { \
+ char str1[BUFSZ], str2[BUFSZ]; \
+ int len1 = snprintf (str1, BUFSZ, fmt, __VA_ARGS__); \
+ TEST_VERIFY_EXIT (len1 >= 0); \
+ TEST_VERIFY_EXIT (len1 < BUFSZ); \
+ _dl_dprintf (fds[1], fmt, __VA_ARGS__); \
+ ssize_t len2 = read (fds[0], str2, BUFSZ); \
+ TEST_VERIFY_EXIT (len2 >= 0); \
+ TEST_VERIFY_EXIT (len2 < BUFSZ); \
+ str2[len2] = '\0'; \
+ TEST_COMPARE_STRING(str1, str2); \
+ } while (0)
+
+static int
+do_test (void)
+{
+ int fds[2];
+ xpipe (fds);
+ TEST ("%d", 0);
+ TEST ("%d", 1);
+ TEST ("%d", INT_MAX);
+ TEST ("%d", -1);
+ TEST ("%d", INT_MIN + 1);
+ TEST ("%d", INT_MIN);
+ TEST ("%u", 0U);
+ TEST ("%u", 1U);
+ TEST ("%u", UINT_MAX);
+ TEST ("%x", 0);
+ TEST ("%x", 1);
+ TEST ("%x", UINT_MAX);
+ TEST ("%ld", 0L);
+ TEST ("%ld", 1L);
+ TEST ("%ld", LONG_MAX);
+ TEST ("%ld", -1L);
+ TEST ("%ld", LONG_MIN + 1);
+ TEST ("%ld", LONG_MIN);
+ TEST ("%lu", 0UL);
+ TEST ("%lu", 1UL);
+ TEST ("%lu", ULONG_MAX);
+ TEST ("%lx", 0UL);
+ TEST ("%lx", 1UL);
+ TEST ("%lx", ULONG_MAX);
+ xclose (fds[0]);
+ xclose (fds[1]);
+ return 0;
+}
+
+#include <support/test-driver.c>
--
2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] elf: fix handling of negative numbers in dl-printf
2023-05-08 18:08 ` Roy E
@ 2023-05-24 17:41 ` Roy E
0 siblings, 0 replies; 11+ messages in thread
From: Roy E @ 2023-05-24 17:41 UTC (permalink / raw)
To: Florian Weimer; +Cc: carlos, libc-alpha
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/
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] elf: add test for dl-printf
2023-05-15 20:29 ` [PATCH] elf: add test for dl-printf Roy Eldar
@ 2023-05-25 7:44 ` Florian Weimer
2023-05-25 14:41 ` [PATCH v2] " Roy Eldar
0 siblings, 1 reply; 11+ messages in thread
From: Florian Weimer @ 2023-05-25 7:44 UTC (permalink / raw)
To: Roy Eldar; +Cc: libc-alpha, carlos
* Roy Eldar:
> This patch checks _dl_debug_vdprintf, by passing various inputs to
> _dl_dprintf and comparing the output with invocations of snprintf.
>
> Signed-off-by: Roy Eldar <royeldar0@gmail.com>
Do you have a copyright assignment on file? If not, Signed-off-by: is
sufficient but we should say …
> diff --git a/elf/tst-dl-printf-static.c b/elf/tst-dl-printf-static.c
> new file mode 100644
> index 0000000000..a31759a006
> --- /dev/null
> +++ b/elf/tst-dl-printf-static.c
> @@ -0,0 +1,75 @@
> +/* Check _dl_debug_vdprintf.
> + Copyright (C) 2016-2023 Free Software Foundation, Inc.
… “Copyright The GNU Toolchain Authors.” here.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <https://www.gnu.org/licenses/>. */
> +
> +#include <ldsodefs.h>
> +#include <limits.h>
> +#include <stdarg.h>
> +#include <stdio.h>
> +#include <support/check.h>
> +#include <support/xunistd.h>
> +
> +#define BUFSZ 64
> +
> +#define TEST(fmt, ...) do { \
GNU style puts { and } on their own lines.
> + char str1[BUFSZ], str2[BUFSZ]; \
> + int len1 = snprintf (str1, BUFSZ, fmt, __VA_ARGS__); \
> + TEST_VERIFY_EXIT (len1 >= 0); \
> + TEST_VERIFY_EXIT (len1 < BUFSZ); \
> + _dl_dprintf (fds[1], fmt, __VA_ARGS__); \
> + ssize_t len2 = read (fds[0], str2, BUFSZ); \
> + TEST_VERIFY_EXIT (len2 >= 0); \
> + TEST_VERIFY_EXIT (len2 < BUFSZ); \
> + str2[len2] = '\0'; \
> + TEST_COMPARE_STRING(str1, str2); \
Missing space after TEST_COMPARE_STRING.
Rest looks good.
Thanks,
Florian
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] elf: fix handling of negative numbers in dl-printf
2023-05-03 21:16 [PATCH] elf: fix handling of negative numbers in dl-printf Roy Eldar
2023-05-08 13:42 ` Florian Weimer
@ 2023-05-25 7:46 ` Florian Weimer
2023-05-25 14:41 ` [PATCH v2] " Roy Eldar
1 sibling, 1 reply; 11+ messages in thread
From: Florian Weimer @ 2023-05-25 7:46 UTC (permalink / raw)
To: Roy Eldar via Libc-alpha; +Cc: Roy Eldar, carlos
* 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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2] elf: fix handling of negative numbers in dl-printf
2023-05-25 7:46 ` [PATCH] elf: fix handling of negative numbers in dl-printf Florian Weimer
@ 2023-05-25 14:41 ` Roy Eldar
2023-05-25 17:10 ` Florian Weimer
0 siblings, 1 reply; 11+ messages in thread
From: Roy Eldar @ 2023-05-25 14:41 UTC (permalink / raw)
To: fweimer; +Cc: Roy Eldar, libc-alpha, carlos
_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 | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/elf/dl-printf.c b/elf/dl-printf.c
index e8b9900370..6efb4c019a 100644
--- a/elf/dl-printf.c
+++ b/elf/dl-printf.c
@@ -1,5 +1,6 @@
/* printf implementation for the dynamic loader.
Copyright (C) 1997-2023 Free Software Foundation, Inc.
+ Copyright The GNU Toolchain Authors.
This file is part of the GNU C Library.
The GNU C Library is free software; you can redistribute it and/or
@@ -150,19 +151,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
}
--
2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2] elf: add test for dl-printf
2023-05-25 7:44 ` Florian Weimer
@ 2023-05-25 14:41 ` Roy Eldar
2023-05-25 17:10 ` Florian Weimer
0 siblings, 1 reply; 11+ messages in thread
From: Roy Eldar @ 2023-05-25 14:41 UTC (permalink / raw)
To: fweimer; +Cc: Roy Eldar, libc-alpha, carlos
This patch checks _dl_debug_vdprintf, by passing various inputs to
_dl_dprintf and comparing the output with invocations of snprintf.
Signed-off-by: Roy Eldar <royeldar0@gmail.com>
---
elf/Makefile | 1 +
elf/tst-dl-printf-static.c | 78 ++++++++++++++++++++++++++++++++++++++
2 files changed, 79 insertions(+)
create mode 100644 elf/tst-dl-printf-static.c
diff --git a/elf/Makefile b/elf/Makefile
index 396ec51424..90ccd65db1 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -275,6 +275,7 @@ tests-static-normal := \
tests-static-internal := \
tst-dl_find_object-static \
+ tst-dl-printf-static \
tst-ptrguard1-static \
tst-stackguard1-static \
tst-tls1-static \
diff --git a/elf/tst-dl-printf-static.c b/elf/tst-dl-printf-static.c
new file mode 100644
index 0000000000..da89c44183
--- /dev/null
+++ b/elf/tst-dl-printf-static.c
@@ -0,0 +1,78 @@
+/* Check _dl_debug_vdprintf.
+ Copyright The GNU Toolchain Authors.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <ldsodefs.h>
+#include <limits.h>
+#include <stdarg.h>
+#include <stdio.h>
+#include <support/check.h>
+#include <support/xunistd.h>
+
+#define BUFSZ 64
+
+#define TEST(fmt, ...) \
+ do \
+ { \
+ char str1[BUFSZ], str2[BUFSZ]; \
+ int len1 = snprintf (str1, BUFSZ, fmt, __VA_ARGS__); \
+ TEST_VERIFY_EXIT (len1 >= 0); \
+ TEST_VERIFY_EXIT (len1 < BUFSZ); \
+ _dl_dprintf (fds[1], fmt, __VA_ARGS__); \
+ ssize_t len2 = read (fds[0], str2, BUFSZ); \
+ TEST_VERIFY_EXIT (len2 >= 0); \
+ TEST_VERIFY_EXIT (len2 < BUFSZ); \
+ str2[len2] = '\0'; \
+ TEST_COMPARE_STRING (str1, str2); \
+ } \
+ while (0)
+
+static int
+do_test (void)
+{
+ int fds[2];
+ xpipe (fds);
+ TEST ("%d", 0);
+ TEST ("%d", 1);
+ TEST ("%d", INT_MAX);
+ TEST ("%d", -1);
+ TEST ("%d", INT_MIN + 1);
+ TEST ("%d", INT_MIN);
+ TEST ("%u", 0U);
+ TEST ("%u", 1U);
+ TEST ("%u", UINT_MAX);
+ TEST ("%x", 0);
+ TEST ("%x", 1);
+ TEST ("%x", UINT_MAX);
+ TEST ("%ld", 0L);
+ TEST ("%ld", 1L);
+ TEST ("%ld", LONG_MAX);
+ TEST ("%ld", -1L);
+ TEST ("%ld", LONG_MIN + 1);
+ TEST ("%ld", LONG_MIN);
+ TEST ("%lu", 0UL);
+ TEST ("%lu", 1UL);
+ TEST ("%lu", ULONG_MAX);
+ TEST ("%lx", 0UL);
+ TEST ("%lx", 1UL);
+ TEST ("%lx", ULONG_MAX);
+ xclose (fds[0]);
+ xclose (fds[1]);
+ return 0;
+}
+
+#include <support/test-driver.c>
--
2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] elf: fix handling of negative numbers in dl-printf
2023-05-25 14:41 ` [PATCH v2] " Roy Eldar
@ 2023-05-25 17:10 ` Florian Weimer
0 siblings, 0 replies; 11+ messages in thread
From: Florian Weimer @ 2023-05-25 17:10 UTC (permalink / raw)
To: Roy Eldar; +Cc: libc-alpha, carlos
* Roy Eldar:
> _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>
Reviewed-by: Florian Weimer <fweimer@redhat.com>
And pushed.
Thanks,
Florian
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] elf: add test for dl-printf
2023-05-25 14:41 ` [PATCH v2] " Roy Eldar
@ 2023-05-25 17:10 ` Florian Weimer
0 siblings, 0 replies; 11+ messages in thread
From: Florian Weimer @ 2023-05-25 17:10 UTC (permalink / raw)
To: Roy Eldar; +Cc: libc-alpha, carlos
* Roy Eldar:
> This patch checks _dl_debug_vdprintf, by passing various inputs to
> _dl_dprintf and comparing the output with invocations of snprintf.
>
> Signed-off-by: Roy Eldar <royeldar0@gmail.com>
Reviewed-by: Florian Weimer <fweimer@redhat.com>
And pushed.
Thanks,
Florian
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-05-25 17:10 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-03 21:16 [PATCH] elf: fix handling of negative numbers in dl-printf Roy Eldar
2023-05-08 13:42 ` Florian Weimer
2023-05-08 18:08 ` Roy E
2023-05-24 17:41 ` Roy E
2023-05-15 20:29 ` [PATCH] elf: add test for dl-printf Roy Eldar
2023-05-25 7:44 ` Florian Weimer
2023-05-25 14:41 ` [PATCH v2] " Roy Eldar
2023-05-25 17:10 ` Florian Weimer
2023-05-25 7:46 ` [PATCH] elf: fix handling of negative numbers in dl-printf Florian Weimer
2023-05-25 14:41 ` [PATCH v2] " Roy Eldar
2023-05-25 17:10 ` Florian Weimer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).